fix(workflow): preserve council delegation reliability#516
Merged
Conversation
… be built (#1) A read-only delegation (e.g. a council review) that deps on an implement leg is supposed to run in an integration worktree with the leg branch merged in. If integrationDepBranches resolved ZERO leg branches (the leg's branch was not readable — e.g. transient SQLITE_BUSY contention when several repo-scoped daemons share one ~/.gitmoot, or the branch not yet committed/visible), the delegation silently fell through to the BASE checkout and reviewed code WITHOUT the implemented change — producing false verdicts (observed: council reviewers split, some judging the base while siblings judged the merged work). Fix: when a read-only delegation declares a dep on a non-read-only (implement) sibling but no leg branch resolves, fail closed (block + a delegation_integration_unresolved event) instead of reviewing base. Also raise the SQLite busy_timeout 5s -> 15s so multi-daemon write bursts drain instead of erroring (the contention that triggered the stale read). New regression test: TestAllocateAndEnqueueDelegationBlocksWhenImplementLegUnresolved. Co-authored-by: gaijinjoe <crazyjerry543@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
) MaxDelegationDepth (8) and MaxDelegationTotalJobs (64) are hard caps. A council coordinator legitimately runs long continuation chains — 6 rounds plus up to 4 fix cycles, and EACH round/fix-cycle continuation increments DelegationDepth — so the default-8 depth is exhausted after ~1 fix cycle and the run dies with 'delegation depth limit of 8 reached' long before the 4-fix-cycle policy completes. Make both bounds env-overridable (GITMOOT_MAX_DELEGATION_DEPTH / GITMOOT_MAX_DELEGATION_TOTAL_JOBS, positive ints), defaulting to the existing safe consts when unset/invalid. The width, wall-clock, and token/cost backstops still bound runaway recursion, so the default is unchanged for everyone; only a daemon that opts in (e.g. a council daemon) raises them. Test: env override + invalid fallback. Co-authored-by: gaijinjoe <crazyjerry543@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(kimi): start a fresh session per job (don't resume a dir-bound session) Kimi Code sessions are DIRECTORY-SCOPED. The council-kimi seat pinned a single runtime_ref session and resumed it with -S, but review delegations run in per-delegation worktrees, so the resume failed every time with 'Session ... was created under a different directory' — and since kimi is a NATIVE voter, that failure forced a hard block, holding the gate on essentially every worktree review. KimiAdapter.Deliver now omits -S and starts a fresh session per job; gitmoot jobs carry full context in the prompt, so a fresh session works in any worktree and kimi stays a native seat. RuntimeRef is kept (validated) but not used to resume. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(kimi): update adapter tests for fresh-session-per-job (no -S resume) Deliver/Health no longer pass -S <session>; assertions now expect a fresh session command. Renamed TestKimiDeliverCommandResumesSession -> StartsFreshSession and TestKimiHealthUsesRegisteredSession -> RunsFreshSession to match the new behavior. --------- Co-authored-by: gaijinjoe <crazyjerry543@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
|
Review note from local PR check: I think this PR should preserve the existing “implement leg already on base” case before merge.
Could you add a regression test for this case?
If that case is intentionally impossible after this change, please document why; otherwise I think the guard should distinguish “no branch because already on base” from “no branch because unresolved/missing”. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR upstreams the council reliability fixes currently carried in gaijinjoe/gitmoot so downstream council users can run upstream Gitmoot without reintroducing known failures.
Fixes
GITMOOT_MAX_DELEGATION_DEPTHandGITMOOT_MAX_DELEGATION_TOTAL_JOBS, preserving existing defaults when unset/invalid.Verification
go build ./...go test ./internal/db ./internal/workflow ./internal/runtimego test ./internal/workflow ./internal/runtimeNotes
These changes are based on upstream
mainata06b6adand are already validated locally with the council stack.