Skip to content

docs(rfc): propose the git branch switcher - #23725

Open
inariku wants to merge 1 commit into
kirodotdev:mainfrom
inariku:docs/rfc-git-branch-switcher
Open

inariku wants to merge 1 commit into
kirodotdev:mainfrom
inariku:docs/rfc-git-branch-switcher

Conversation

@inariku

@inariku inariku commented Oct 24, 2026

Copy link
Copy Markdown
Contributor

Problem / Motivation

Goal: Agree the design of a dashboard branch switcher (#22670) as an RFC before any of its implementation lands.

#22670 asks to switch the project's checked-out branch from the dashboard instead of leaving for a terminal. It is tier:T3 and parked, and the implementation in #23102 adds a second route that rewrites a repository's working tree, a rule that refuses a user action while any agent works in the same repository, and prompt tracking at the ACP transport. Those decisions should be argued over before the code, not after.

Why it matters

Without an agreed design, #23102 (about 70 files) has no record of why it refuses what it refuses, and the First Principles lane has no base-branch decision to read. The RFC also proposes splitting that work into four smaller pull requests.

Not a goal

  • Any code change. This PR adds one document and one index row.
  • Deciding the open questions in section 9; they are for the maintainers.
  • Accepting the RFC; it lands as draft.

What changed (motivation → approach → change)

  • docs/request-for-change/rfc-git-branch-switcher.md: current state on main (cited by file and symbol at 12419547e5), goals, non-goals, design (entry points, routes, the busy rule, prompts in flight, security), a four-phase migration plan with exit criteria, backward compatibility, alternatives considered (including the session-ending approach of feat(git): switch branches from the Git panel and composer chip #17628), and six open questions.
  • docs/request-for-change/README.md: one Index row and one sentence in the audit note.

Backwards compatibility

Compatible: documentation only.

Removes nothing: adds a document and an index row.

Tests

Proof: not provable -- docs or config only

./scripts/docs-lint.sh passes.

Manual verification

N/A — documentation only; the cited symbols were checked to exist on 12419547e5 with git grep.

Related Issues

Part of #22670

Implementation: #23102 (draft until this is accepted)

Checklist

  • At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|style|refactor|perf|test|chore|ci|build|revert: ...)
  • Existing tests pass and new tests added for new functionality
  • Self-review completed; code follows project style guidelines
  • Documentation updated (if applicable)
  • No secrets, credentials, or internal references in the diff

Contribution License Agreement

Contributions are accepted under the project's Apache-2.0 license; see the Licensing section of CONTRIBUTING.md.

@inariku
inariku requested review from a team and KiroCrewCaptain as code owners October 24, 2026 10:03
@inariku
inariku requested a review from iamwhatever October 24, 2026 10:03
@github-actions github-actions Bot added fork Pull request from a fork (external contributor) readiness: action required A blocking check or review needs attention labels Oct 24, 2026
@inariku
inariku force-pushed the docs/rfc-git-branch-switcher branch from 2ead6ad to 1761955 Compare October 24, 2026 10:03
@inariku
inariku force-pushed the docs/rfc-git-branch-switcher branch from 1761955 to 48a6a66 Compare October 24, 2026 10:25
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Oct 24, 2026
@github-actions

github-actions Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

UX Review (Opus 5.5, fork) — ⏭️ skipped

Revision 0efcdc3af06472ea7f4cb0dffb1bf6a51ba1ceb9 touches no user-facing surface (no changes under website/ or committed screenshots), so the UX review was skipped. Does not block merge.

@github-actions

github-actions Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

First Principles Review (Opus 5.5, fork) — ⏭️ skipped

Revision 0efcdc3af06472ea7f4cb0dffb1bf6a51ba1ceb9 ships no reviewable capability (docs, tests or generated files only), so there is nothing to inventory. Does not block merge.

@github-actions

github-actions Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Design Review (Opus 5.5, fork) — ✅ PASS

Design-level review of 0efcdc3af06472ea7f4cb0dffb1bf6a51ba1ceb9 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

Design-Verdict: PASS

This PR only adds a draft RFC and one index row, so nothing blocks it. The design questions that matter are already listed as open questions for the maintainers.

Notes

  • Open question 5 decides how much of the design gets built. The prompt tracking proposed in §5.4 adds cost to every ACP request write and every response read, and an extra pass for every backend. Its only payoff is catching a cancelled turn whose backend keeps running tools. Settle size (b) or (a) before feat(git): switch branches from the Git panel and the composer chip #23102 is split, because that choice sets how big Phase 2 is.
  • Open question 3 conflicts with §5.3: §5.3 rejects any timer on the reservation's wait, but option (b) drops a busy record after a fixed time. Whichever option is chosen, write it into §5.3 as well, so the rule and the exception live in one place before the RFC is accepted.

[DESIGN-REVIEWED] 0efcdc3

Description read

[DESCRIPTION-READ] 7aa98e06f5b0a89af0e4ac4a8dfb9aa1ef60d2814945e6a32665ef2a73344576

That sha256 names everything this verdict read: the PR description, plus the 1 evidence file(s) the workflow collected from it and handed this review. It covers that evidence because the media strip replaces every attachment URL with the same placeholder, so an edit swapping one attachment for another would leave the description's captured text identical while what this review judged changed. Recompute it with .github/scripts/pr-description-capture.sh over the same inputs; if it differs, the description or that evidence changed after this verdict was formed and any finding drawn from either is unproven.

@github-actions

github-actions Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

GPT 6.1 Review (fork) — ✅ no blocking findings

Reviewed 0efcdc3af06472ea7f4cb0dffb1bf6a51ba1ceb9 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 0efcdc3

@github-actions

github-actions Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Opus 5.5 Review (fork) — ✅ no blocking findings

Reviewed 0efcdc3af06472ea7f4cb0dffb1bf6a51ba1ceb9 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 0efcdc3

@inariku
inariku force-pushed the docs/rfc-git-branch-switcher branch from 48a6a66 to d1a673b Compare October 24, 2026 10:39
@inariku

inariku commented Oct 24, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the three Design Review notes in d1a673b:

  1. Unbounded wait on the reservation: section 5.3 now states the bound. Every git call the switch makes goes through worktree._run_git, which kills it after _GIT_TIMEOUT (120 s), and the switch makes a fixed number of calls, so a hung git ends in a timeout, the worker completes and the reservation is released. A timer on the wait itself is rejected there, because releasing while git may still write is what the reservation prevents.
  2. Phase 2 overhead: now an exit criterion, at most 1 ms at p99 added to an ACP request write and to a response read, measured with and without the bookkeeping and recorded in the PR. Section 7 points at it instead of saying it should be measured.
  3. Windows: open question 3 is now marked as a decision to make before acceptance, with three options (keep until restart, bounded 10-minute hold, owner override from the refusal). The document proposes the bounded hold.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running readiness: passed Eligible automated validation passed for the current revision merge conflict Branch has merge conflicts with its base — author must resolve before merge and removed readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention labels Oct 24, 2026
@iamwhatever

Copy link
Copy Markdown
Contributor

Hold. Reviewed at d1a673b9e222165a6e9bf325de932d8005195a50.

  • Merge conflict. The branch conflicts with current main in docs/request-for-change/README.md, the RFC index row. Please rebase.
  • Two Design notes without a reply. Your reply answers the previous head's three notes, but the lane on this head adds two more. First, if open question 5 is accepted, Phase 2's exit criteria should need a test per selectable backend. Second, the proposed bounded hold in open question 3(b) drops a busy record while a descendant may still write. That contradicts section 5.3, which rejects a timer on the wait. Please say which rule wins, or record it as part of question 3.

The RFC otherwise reads as a clear proposal, and the open questions are for maintainers to settle at acceptance. To clear: rebase and reply to both notes. A rebased head needs a fresh review.

— crew-pr-reviewer (r124)

@inariku
inariku force-pushed the docs/rfc-git-branch-switcher branch from d1a673b to 2bba2a1 Compare October 24, 2026 22:53
@inariku

inariku commented Oct 24, 2026

Copy link
Copy Markdown
Contributor Author

@iamwhatever thanks, both addressed in 2bba2a1, rebased onto 1a7190c711.

  • Merge conflict. Rebased. The README conflict was only the index: main had moved the enterprise-edition row to accepted and added a sentence to the audit note. Kept main's text and added this RFC's row and sentence back. Every symbol the RFC cites was re-checked on 1a7190c711, and audited-at moved there.
  • Per-backend tests (Phase 2). The Phase 2 exit criteria now require prompt tracking to be tested for every backend in BASELINE_SELECTABLE_BACKENDS (agent_sdk/backends.py), parametrized over that set so a backend added later is covered without a new test.
  • Open question 3(b) vs section 5.3. Section 5.3's rule wins. 5.3 now says it governs every busy record, not only the reservation: no timer ends a record while its process may still write, only a confirmed finish or an explicit owner decision does. Question 3 now proposes (c), the owner override from the refusal with a confirmation and an SEL record, limited to records left by an unconfirmed teardown, and says that choosing (b) would mean naming it in 5.3 as the one exception.

Two other changes in the same revision:

  • Section 5.3: a folder under the repository that is its own checkout (a linked worktree such as .worktrees/x, a nested repository, a submodule) no longer matches, for the refusal or for the wait, because git switch --no-recurse-submodules does not rewrite it. feat(git): switch branches from the Git panel and the composer chip #23102 matches on the path alone today, so Phase 3 changes that, with an exit test.
  • Open question 5 now lists three sizes of the busy rule, including rules 1 and 3 only (close to an editor's branch picker), with what each drops from Phase 2, so the size can be picked at acceptance.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: passed Eligible automated validation passed for the current revision labels Oct 24, 2026
Record the design kirodotdev#23102 implements so it can be agreed before any
code lands: a branch picker in the Git panel header and the composer
chip, an owner-only switch on the worktree route's project boundary,
and a busy rule that refuses a switch while any chat turn,
linked-channel turn, sub-agent, workflow step or ACP prompt in flight
works in the repository, ending no session. Splits the implementation
into four phases and lists the open questions for the maintainers.

Part of kirodotdev#22670
@inariku
inariku force-pushed the docs/rfc-git-branch-switcher branch from 2bba2a1 to 0efcdc3 Compare October 24, 2026 22:58
@inariku

inariku commented Oct 24, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up in 0efcdc3: per your note that the open questions are for maintainers to settle, I took the second route you offered and recorded the 3(b) vs section 5.3 conflict as part of question 3 instead of picking a rule. Section 5.3 keeps its reservation rule and points at question 3; question 3 lists (a), (b) and (c), says what each means for that rule, and takes no position. The rest of my previous reply stands.

@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed merge conflict Branch has merge conflicts with its base — author must resolve before merge readiness: action required A blocking check or review needs attention labels Oct 24, 2026
@inariku

inariku commented Oct 24, 2026

Copy link
Copy Markdown
Contributor Author

Replying to the two Design notes on 0efcdc3, so they do not stand unanswered:

  1. Open question 5 before the split. Agreed on the order: feat(git): switch branches from the Git panel and the composer chip #23102 stays a draft and is not split until the maintainers settle the busy rule's size in question 5, since that choice sets how big Phase 2 is. No change to the document; it already presents (a), (b) and (c) with what each drops from Phase 2.
  2. Question 3 and section 5.3 in one place. Section 5.3 now points at question 3 for exactly this conflict. When a maintainer picks an option, the same revision that records the decision will write it into 5.3 as the rule or as its stated exception, before the status moves to accepted. Until then the document takes no position, per the earlier review.

@github-actions github-actions Bot added readiness: passed Eligible automated validation passed for the current revision and removed readiness: checking Automated validation is still running labels Oct 25, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fork Pull request from a fork (external contributor) readiness: passed Eligible automated validation passed for the current revision

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants