Repository navigation
Conversation
UX Review (Opus 5.5) — ⏭️ skippedRevision |
First Principles Review (Opus 5.5) — ✅ human override accepted@buluoray overrode this lane for |
Design Review (Opus 5.5) — ✅ PASSDesign-level review of Design-Verdict: PASS Nothing blocks: the change adds worktrees and a cleanup step, takes nothing away, and reuses the existing sandboxed git runner and the deferred reset that Notes
[DESIGN-REVIEWED] 9953d01 Description read[DESCRIPTION-READ] 2f8baee1a015cf5544319980ec238f452745594291913944e0f7813fd576d9c9 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 |
Opus 5.5 Review — 🔴 changes requested (blocking)Reviewed Blocks: one path in a worker's brief that sits under a directory the gateway can't search makes the worker run in the shared checkout. The first candidate path raises BLOCKING — src/kiro_crew/dashboard/crewmate_worktree_scope.py:118 Trigger: a worker turn's message names a path inside a directory the gateway user can't search, such as [BLOCK-MERGE] 9953d01 Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
GPT 6.1 Review — 🔴 changes requested (blocking)GPT 6.1 found at least one blocking issue that must be resolved before merging This comment is updated in place on each push. BLOCKING -- src/kiro_crew/crewmate_worktree.py:970 -- Index cleanup can delete a governance profile through a directory swap
BLOCKING -- src/kiro_crew/context_assembly/turn.py:398 -- Repository config can forge trusted prompt boundaries
BLOCKING -- src/kiro_crew/crewmate_worktree.py:604 -- An unknown default branch makes synchronization follow the shared checkout
[BLOCK-MERGE] 9953d01 Adjudication (Opus 5.5) — is blocking on each finding proportionate?F3 — MEDIUM. Three things have to be true. The repository has no [ADJUDICATION] 9953d01 total=1 uphold=1 downgrade=0 [ADJUDICATION-FENCED] 9953d01 fenced=2 flagged=1 🏷️ Fenced finding(s) machine-flagged as likely edge caseThe security fence keeps these findings blocking regardless of adjudication; the only clearance path is a human override. The annotate-only pass judged the condition combination each one requires extreme, and pre-drafted the override rationale below. A repository writer must independently verify a rationale before posting it — it is machine-authored, and a wrong override on a security-class finding ships exactly the class the fence exists to stop.
Replace False positive or not applicable? A repository writer can comment: |
682cfa7 to
4cea3db
Compare
4cea3db to
963e809
Compare
|
/ai-review override first-principles 963e809: Product decision by the maintainer (buluoray): every crewmate worker runs in its own git worktree by default, so parallel workers never edit the same checkout. |
Human judgment recorded@buluoray marked the first-principles AI finding as false positive, not applicable, or explicitly accepted for
This decision applies only to this commit. A new push requires a new judgment. |
963e809 to
dc4f0ff
Compare
|
/ai-review override first-principles dc4f0ff: Product decision by the maintainer (buluoray): every crewmate worker runs in its own git worktree by default; an unpushed, clean branch with no PR is the only one rebased automatically. |
Human judgment recorded@buluoray recorded a legacy release (class
This decision applies only to this commit. A new push requires a new judgment. |
|
This override used the old command without a class. It was recorded as class |
d8f3fc4 to
a0ba86b
Compare
|
/ai-review override first-principles a0ba86b: Product decision by the maintainer (buluoray): every crewmate worker runs in its own git worktree by default; an unpushed, clean branch with no PR is the only one rebased automatically. |
|
AI-review override not recorded: |
Human judgment recorded@buluoray recorded a legacy release (class
This decision applies only to this commit. A new push requires a new judgment. |
|
This override used the old command without a class. It was recorded as class |
a0ba86b to
d99aee3
Compare
|
/ai-review override first-principles d99aee3: Product decision by the maintainer (buluoray): every crewmate worker runs in its own git worktree by default; an unpushed, clean branch with no PR is the only one rebased automatically. |
Human judgment recorded@buluoray recorded a legacy release (class
This decision applies only to this commit. A new push requires a new judgment. |
|
This override used the old command without a class. It was recorded as class |
d99aee3 to
0d6c8c0
Compare
|
/ai-review override first-principles 0d6c8c0: Product decision by the maintainer (buluoray): every crewmate worker runs in its own git worktree by default; an unpushed, clean branch with no PR is the only one rebased automatically. |
Human judgment recorded@buluoray recorded a legacy release (class
This decision applies only to this commit. A new push requires a new judgment. |
|
This override used the old command without a class. It was recorded as class |
Two crewmates handed one repository shared its checkout and branch. A crewmate's worker now runs in the crewmate's own worktree of the repository its turn is about, beside the repository on a crew/<slug>/ branch, cut from the default branch once and reused by every worker the crewmate opens. - Turn admission moves a worker turn's project into that worktree after the crewmate's Permissions are seeded, from the paths the message names, then the slot's project, then the crewmate's goals. - Each worker turn first rebases a clean worktree onto the repository's default branch; a dirty one is left as it is, and the [PROJECT] line says which happened once. - The [PROJECT] line names the repository and the branch, and the conductor and worker prompts say where the worker commits. - Deleting the crewmate removes its worktrees, keeping any with uncommitted changes and never touching branches or other checkouts.
|
/ai-review override first-principles 9953d01: Product decision by the maintainer (buluoray): every crewmate worker runs in its own git worktree by default; an unpushed, clean branch with no PR is the only one rebased automatically. |
Human judgment recorded@buluoray recorded a legacy release (class
This decision applies only to this commit. A new push requires a new judgment. |
|
This override used the old command without a class. It was recorded as class |
Problem / Motivation
Goal: Each crewmate's worker works in the crewmate's own git worktree and branch, so two crewmates handed the same repository never edit one checkout or one branch.
With Crewmate 2.0 (#23010) every crewmate hands its work to a worker, but the worker runs wherever its session's project points, usually the repository's shared checkout. Two crewmates working in one repository then edit the same working tree and commit on whatever branch happens to be checked out, so their changes mix and one can move the branch under the other.
Why it matters
Anyone running more than one crewmate on a repository gets tangled commits and lost or overwritten work, and has to keep each crewmate out of the others' way by hand. A worktree per crewmate keeps each one's changes on its own branch, and the user's own checkout stays as they left it.
Not a goal
crew/<slug>/prefix is asked for in the prompt, not refused by the gateway.What changed (motivation → approach → change)
Goal → give each crewmate one worktree per repository and point its workers there before the turn starts → reuse the follow-up card's sandboxed git runner (
handlers/worktree._run_git) and the existing project/binding re-capture, so nothing else in the turn changes.kiro_crew/crewmate_worktree.py: makes or reuses the crewmate's worktree beside the repository,<parent>/<repo>.worktrees/<crewmate slug>/oncrew/<slug>/<repo>, cut once from the repository's default branch (origin/HEAD, else a localmain, never the shared checkout'sHEAD). A name the cleaning changed (app+tools) or one over 40 characters keeps a digest of the whole, so two names never share a path or branch; every segment is a legal git ref component (no.., no leading., no.lockending), soapp.lockandapp..toolsstill get a branch. A repository whose parent is not writable (or a filesystem root) falls back to<crewmate workspace>/worktrees/<crewmate slug>/<repo>-<hash>. Every worktree is listed inworktrees/index.jsonwith the crewmate's slug (read throughhooks.safe_read_prefix, so a link to a sensitive path reads as no index; written with no-follow descriptors and a rename relative to the pinnedworktreesfolder, so aworktreesswapped for a link, to governanceprofiles/say, gets nothing written); two crewmates may share a workspace, so lookup, reuse and removal read only the crewmate's own entries, under a per-index lock. A listed worktree is reused on any of the crewmate's branches, a topic branch a worker cut there included; one whose folder was deleted by hand is pruned from git and built again (a locked one still refuses). Aworktree addthat times out or fails is unwound (branch and folder removed) before the error is raised.sync_with_baserebases the branch onto the same default branch only when it was never pushed (no upstream, and no remote-tracking ref holding any of its own commits, so a commit made after a push does not hide it and a fresh branch with no commits of its own is brought up to date; this covers any branch with a PR), the worktree is clean (no uncommitted or untracked changes), it holds no merge commit and it is behind. The rebase moves only this branch (rebase.updateRefsoff) and signs nothing (--no-gpg-sign,commit.gpgSign=false, everygpg.*programpinned), so a signing program the repository names never runs. One that stops, fails or times out is aborted and checked back on its old commit; when that cannot be shown, the turn ends with the reason instead of running on a half-rebased tree, and a later turn there is refused the same way until the rebase is finished or aborted. Beforestatusor a rebase the worktree is checked again for a program the repository's config makes git run (a checkout filter or a merge driver); a repository that has one is left untouched and the worker told. One sync runs at a time per worktree, and a rebase it did not start is never aborted (the turn is refused instead). The dirty check does not enter submodules, whose config is not screened. A pushed branch is never rebased automatically: the worker runsgit merge-tree --write-tree <base> HEADbefore it pushes and rebases explicitly only on a conflict, saying so in its report. Each outcome leaves a one-line note.dashboard/crewmate_worktree_scope.py: for a worker turn (akirocrew-mate-workerrecord with the crewmate's id), turn admission picks the repository from the message's paths (absolute,~or Windows drive paths; a quoted one is read whole, spaces included), then the slot's project, then the crewmate's goals. A path in another repository (another of the crewmate's own worktrees included) sends the turn there; a path in the worktree it already runs in, or in the shared checkout of that same repository, is passed over, so the session is not reset. A worktree left mid-rebase refuses the turn whatever the message names. It movesslot.projectinto the worktree and re-captures the binding under the dispatch lock (chat_turn/turn_admission.py). A running session takes the same deferred resetset_projectuses.context_assembly/turn.py: the[PROJECT]line names the repository and branch (the.gitmarker,commondirandHEADare read throughhooks.safe_read_prefix, so a marker linked to a credential file is refused;HEADmust be one line naming a plain, git-legal branch and the repository path must hold no control character or bracket, checked again at the rail, so a forged HEAD adds no line), says to commit there, and carries the base-sync note once.crewmate_prompts.json:worktree.*notes, plus one sentence each in the conductor's crew-log section and the worker's principles.dashboard/agent_admin/crew_removal.py: deleting a crewmate removes its listed worktrees withgit worktree remove(never--force). It keeps one with uncommitted changes, or one whose repository configures a checkout filter, and leaves branches, the repositories' checkouts and other crewmates' worktrees alone.Spec:
docs/system-specs/modules/crew-mode.md("Each crewmate works in a git worktree of its own", and the conductor/worker prompt lines).Backwards compatibility
Compatible: a worker turn whose paths are in no git repository runs where it did before, and a git failure leaves the turn on its current project, except a rebase this change started that cannot be put back, which ends that worker's turn with the reason. Conductor chats and non-crewmate sessions are untouched.
Removes nothing: the change adds worktrees, prompt lines and a cleanup step on crewmate deletion; the cleanup removes only worktrees this change created and listed in the crewmate's own index.
Tests
test/test_crewmate_worktree.py: a real worker turn throughchat_runner._run_chatruns with its cwd in the crewmate's worktree. Two crewmates get separate worktrees and branches, and a worktree is reused. The[PROJECT]line names the branch. Every worker turn syncs with the default branch first, never the shared checkout's branch: a clean worktree is rebased, a dirty one is left with its note, a pushed branch (with an upstream, pushed under another name, or with a commit made after the push) is never rebased, a fresh branch cut fromoriginis brought up to date, the rebase moves no other branch underrebase.updateRefs, an untracked file keeps it from being rebased, a branch holding a merge commit is left for the worker, a rebase that stops or times out is undone, one that cannot be undone ends the turn and keeps refusing the next one, a filter or merge driver the repository names stops the sync without running, a rebase the sync did not start is left alone, two syncs of one worktree run one at a time, a brief naming another repository moves the turn there while one naming the same repository's shared checkout does not, a mid-rebase worktree refuses a turn that names a file in it, the dirty check skips submodules, the rebase never runs a repository-named signing program, a.gitlinked to a credential file is refused unread, a timed-outworktree addleaves no branch or folder, a~path into its own worktree moves nothing,app.lockandapp..toolsget valid branches, a brief naming a not-yet-created file in another repository goes to that repository, anindex.jsonlinked to a credential file is refused unread, aworktreesfolder linked to a profiles folder gets nothing written and reads as no index, a forged or multiline HEAD names no worktree and adds no rail line, and removal keeps a worktree whose repository names a filter. A worktree folder deleted by hand is built again; a locked one still refuses. The admission hook's own branches (conductor skipped, candidate order, refusals, the session reset) run without git. Deleting the crewmate removes its worktrees but keeps a dirty one.test/test_crewmate_worktree_layout.py: the beside-the-repository layout, sanitised names, two long slugs kept apart, names that clean alike kept apart, the branch cut frommainwhile the checkout sits on a feature branch, the fallback when the parent is not writable or the repository is a root, the index (a listed worktree used where it is, two concurrent turns writing it one at a time, two crewmates sharing a workspace never touching each other's), a topic branch keeping the worktree the crewmate's, quoted paths with spaces and Windows paths, and paths on different drives.test/test_agent_refactor_spec_bytes.py(the worker spec's prompt) and theall-metprompt preview golden (the two new sentences only).Proof: not provable -- other: new feature, the new tests exercise code this pull request adds
Manual verification
N/A — unit coverage drives real git in temporary repositories, through the sandboxed runner where the host has a backend and plain git otherwise (the Linux CI runners have none, so the tests no longer skip there; Windows keeps the skip). Run locally both ways, with
TMPDIRoutside the crew data home.Related Issues
Follows #23010.
Checklist
feat|fix|docs|style|refactor|perf|test|chore|ci|build|revert: ...)Contribution License Agreement
Contributions are accepted under the project's Apache-2.0 license.