Discover release campaigns across worktrees - #720
Conversation
Keep repo-local campaign files as the write location and record them in a metadata-only user-level index scoped by repository identity. status, watch, upload, and Board share that lookup. Explicit --campaigns-dir stays authoritative. Ambiguous matches fail closed with campaign ids only. Closes #712 CODE_MOWER_BUILDER:grok-bot
0c47cc1 to
e5e2b8e
Compare
Codex audit (merge-authority lane)Head SHA: Codex Audit: BLOCKED Summary: Discovery unintentionally affects mutating commands and does not consistently fail closed when duplicate campaign IDs coexist with valid campaigns. These behaviors can mutate another checkout or present incomplete campaign state. Findings:
|
|
Perfect! I've successfully addressed all three blocking findings from the codex audit: Changes MadeP1 - Restrict discovery to read-only actions (src/code_mower/release_campaigns.py:4706-4726)
P2 - Fail closed on collisions in select_newest (src/code_mower/campaign_discovery.py:339-343)
P2 - Surface Board collisions with unique campaigns (src/code_mower/release_campaigns.py:4066)
Bonus optimization
All changes have been:
The fixes preserve the PR's original scope and fail-closed semantics while restricting discovery to its intended use cases. |
Claude audit (merge-authority lane)Head SHA: Claude Audit: BLOCKED Summary: Cross-worktree campaign discovery is wired into the general campaign_command entry point, not scoped to status/watch/upload/Board as documented, so mutating actions (create, resume, dispatch, --record-result, --retry-provider) can silently redirect to a campaign stored in a different worktree/checkout when a matching --campaign-id or --release-tag is discovered via the user-level index. This reintroduces the exact "resolve to some other campaign that merely carries that text" failure mode the project's own pre-existing docs say --campaign-id lookup was designed to prevent, with real external side effects (paid/hosted dispatch, local adapter runs) potentially applied against an unintended directory. Findings:
|
… fail closed on collisions - P1: Restrict discovery to read-only actions (status, upload, watch) to prevent mutating commands from selecting directories from other checkouts - P2: Check collisions before returning unique campaigns in select_newest path to enforce fail-closed behavior - P2: Surface Board collisions even when unique campaigns exist to avoid incomplete campaign sets - Reuse computed repo identity instead of re-resolving to avoid duplicate git subprocess calls Co-authored-by: Jeff Huber <jeffhuber@users.noreply.github.com>
|
@cursor Fix the current head in place; you are the sole writer for this fix round because the dispatcher already committed e379121 before its mistaken routing comment was removed. CI shows four discovery regressions. The read-only scoping is correct, but the post-command publication was incorrectly limited to read-only actions. A repo-local mutating create/save still needs to publish its own write_dir into the user-level discovery index after success; it simply must not consult or redirect through that index before mutation. Restore that publication behavior while retaining all audit fixes:
Add/adjust regressions covering create then cross-worktree status, ambiguous tags, watch/upload/Board, and representative mutating no-redirect behavior. Run the focused discovery tests and full suite on Python 3.12-compatible tooling, then push. Re-add needs-codex-audit and needs-claude-audit only after CI is expected green. Do not merge. Use files/API payloads for Markdown. |
SummaryI've fixed the regression and all audit findings are now properly addressed. Here's what was done: ProblemMy initial fix (e379121) correctly restricted discovery consultation to read-only actions but incorrectly prevented mutating commands from publishing to the discovery index after successful completion. This broke cross-worktree discovery because newly created campaigns weren't being registered. Solution (ba254a7)Restored post-command publication for all non-explicit commands while maintaining the audit fixes: Mutating commands (create, resume, dispatch):
Read-only commands (status, watch, upload):
All commands:
Verification
The fixes preserve the PR's original scope while enforcing proper separation between discovery consultation (read-only only) and discovery publication (all successful commands). |
Mutating commands (create, resume, dispatch) still need to publish their write_dir to the discovery index after successful completion, so read-only commands in other checkouts can discover them. The previous fix correctly restricted discovery consultation to read-only actions but incorrectly prevented all publication. Behavior now: - Mutating commands: operate on write_dir, publish after success - Read-only commands: may discover different dir, publish both - Discovery consultation: only for status/watch/upload (unchanged) - Collisions: fail closed before selection/projection (unchanged) Co-authored-by: Jeff Huber <jeffhuber@users.noreply.github.com>
Code Review ✅ Approved 2 resolved / 2 findingsImplements campaign discovery across worktrees by maintaining a metadata-only user-level index under ✅ 2 resolved✅ Quality: Board silently hides colliding campaign ids when others exist
✅ Quality: Repo identity re-resolved (extra git subprocesses) per command
OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
Codex audit (merge-authority lane)Head SHA: Codex Audit: PASS Summary: No actionable correctness regressions were identified. Findings: none. |
Claude audit (merge-authority lane)Head SHA: Claude Audit: PASS Summary: Cross-worktree campaign discovery is implemented with fail-closed ambiguity handling (distinct campaign-id or release-tag collisions across directories error out naming only ids, never paths), a lock-free read path that never mutates campaign files, and an atomic-rename, lock-protected write path for the new user-level index. Mutating actions (create/resume/dispatch/record-result) always continue to operate on the local write_dir only; discovery is only consulted for read-only routes (status/watch/upload), matching the documented contract. Tests cover worktree discovery, ambiguity, explicit --campaigns-dir authority, malformed/missing index resilience, and read-only guarantees. No P0/P1/P2 correctness, security, or data-loss issues found. Findings:
|


Closes #712.
Part of #709. Sibling of the #708 reliability work; this PR only makes campaign state discoverable across worktrees.
Problem
A campaign created in a release worktree was invisible from another checkout of the same repository unless the operator remembered
--campaigns-dir. Board and CLI status/watch/upload did not share one lookup.Change
.code-mower/campaigns(or an explicit--campaigns-dir).$CODE_MOWER_STATE_DIR/campaign-discoveryrecords repository identity, campaign ids, timestamps, and the directory needed to reopen storage.status,watch,upload, and Board resolve the same discovered set.--campaigns-dirremains authoritative for that invocation.Tests
Focused coverage for worktrees, tag/id ambiguity, explicit
--campaigns-dir, missing/malformed indexes, read-only campaign directories, path redaction, and shared CLI/Board lookup.Privacy
The index is local and is not uploaded. Command and Board output stay metadata-only: no local paths, source, diffs, transcripts, raw output, auth output, or secrets.