fix(backlog): stop orchestrator-inferred DONE from forcing premature review#222
Draft
tstapler wants to merge 1 commit into
Draft
fix(backlog): stop orchestrator-inferred DONE from forcing premature review#222tstapler wants to merge 1 commit into
tstapler wants to merge 1 commit into
Conversation
… forcing premature review
Root cause (confirmed live, two independent items in one session):
AutonomousDriver.run() (session/autonomous_driver.go) polls a separate,
minimal orchestrator LLM every time the driven session goes idle, asking it
to judge whether "the goal is complete" from nothing but a raw terminal-tail
snapshot (buildOrchestrationPrompt) — no visibility into acceptance
criteria, committed diff state, or whether the work session's own agent
ever called request_review.
onAutonomousDriverComplete (server/services/autonomous_orchestration_service.go)
trusted that orchestrator's Done=true signal to force the backlog item's
in_progress→review transition and spawn a review session — a second,
much weaker path racing the legitimate one (the request_review MCP tool
in server/mcp/tools_backlog.go, which rejects uncommitted changes and
requires the work session's own agent to explicitly decide the goal is met).
Live evidence: two SDD-pipeline backlog items each had a review session
spawn within 1-9 minutes of their work session starting, right after
nothing but a requirements.md commit — the orchestrator hallucinated DONE
during a natural pause between SDD phases. Both reviews correctly returned
FAIL ("no implementation exists"), but the underlying work sessions kept
running under their own steam and landed real implementation commits
40+ minutes later, unaware a stale FAIL verdict had already been recorded.
Fix: for SessionRoleWork, no longer let the orchestrator's Done signal set
toStatus=BacklogStatusReview. If request_review already fired, the item is
no longer in_progress and there's nothing to do. If it hasn't, leave the
item in_progress (request_review remains the sole path to review) and log
it for diagnosability. The orchestrator's Done reply is still real evidence
the driver itself isn't stuck (looping/hitting the turn cap), so the
autonomous_stuck row is still resolved directly, decoupled from the
now-removed status transition.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NnUqFpFJid7USqmi7CJQJ7
Contributor
✅ Registry ValidationTest Coverage: 19/179 features have
|
Contributor
Go Benchmarks (Tier 1) |
Contributor
E2E RPC Latency |
Contributor
📊 Feature E2E CoverageFeature coverage report unavailable
|
Contributor
Frontend Terminal Throughput |
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
1 participant
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.
Root cause (confirmed live in production, two independent items in the same session)
AutonomousDriver.run()(session/autonomous_driver.go) polls a separate, minimal orchestrator LLM every time the driven session goes idle, asking it to judge whether "the goal is complete" from nothing but a raw terminal-tail snapshot (buildOrchestrationPrompt). This orchestrator has zero visibility into acceptance criteria, committed diff state, or whether the work session's own agent ever calledrequest_review.onAutonomousDriverComplete(server/services/autonomous_orchestration_service.go) trusted that orchestrator'sDone=truesignal to force the backlog item'sin_progress→reviewtransition and spawn a review session — a second, much weaker path racing the legitimate one: therequest_reviewMCP tool (server/mcp/tools_backlog.go), which rejects uncommitted changes and requires the work session's own agent to explicitly decide the goal is met.Live evidence: two SDD-pipeline backlog items each had a review session spawn within 1-9 minutes of their work session starting, right after nothing but a
requirements.mdcommit — the orchestrator hallucinated DONE during a natural pause between SDD phases. Both reviews correctly returned FAIL ("no implementation exists"), but the underlying work sessions kept running under their own steam and landed real implementation commits 40+ minutes later, unaware a stale FAIL verdict had already been recorded against them.Fix
For
SessionRoleWork, the orchestrator'sDonesignal no longer setstoStatus = BacklogStatusReview:request_reviewalready fired, the item is no longerin_progress— nothing to do, the legitimate mechanism already handled it.in_progress(request_reviewremains the sole path to review) and log it for diagnosability instead of silently forcing a wrong transition.The orchestrator's
Donereply is still real evidence the driver itself isn't stuck (looping on malformed responses, hitting the turn cap) — that's a narrower, still-useful claim independent of "ready for review" — so theautonomous_stuckrow is still resolved, just called directly rather than gated behind the now-removed status transition.Test plan
go build ./server/services/... ./session/...go test ./server/services/... ./session/...TestAutonomousOrchestrationService_OnAutonomousDriverComplete_DoesNotForceReview_When_OrchestratorClaimsDoneWithoutRequestReview— reproduces the exact failure shape (SessionRoleWork,Done=true, item stillin_progress) and asserts the item staysin_progresswith no review spawnedexpected: in_progress, actual: review, review trigger called)TestRemoveHooksConfig_should_StripOnlyTheNamedHook_When_MultipleHooksPresent, confirmed to fail/pass identically with and without this change — pre-existing test-ordering flakiness, not a regression)Claude-Session: https://claude.ai/code/session_01NnUqFpFJid7USqmi7CJQJ7