feat(agent): serve stale decision packs with freshness marker and background rebuild#26
Merged
JSONbored merged 9 commits intoMay 29, 2026
Conversation
…kground rebuild Closes #15. `GET /v1/contributors/:login/decision-pack`, the MCP decision-pack tools, and the agent orchestrator previously discarded a perfectly usable cached pack whenever it crossed the 6h freshness window and returned an empty `needs_snapshot_refresh` body. Every MCP `agent plan` call hitting a slightly stale pack therefore came back with no `topActions` or `repoDecisions`, exactly when the contributor was asking what to do next. This change centralizes the staleness policy in a new `loadContributorDecisionPackForServing` helper used by the REST API, the MCP server, and the agent orchestrator. The helper: - returns a `fresh` pack as-is when within the freshness window, - returns the same pack with `freshness: "rebuilding"` (or `"stale"` if the queue refused the job) plus `rebuildEnqueued: true` when the pack is past the window, and enqueues a single background rebuild, - returns a bounded `needs_snapshot_refresh` body with `freshness: "missing"` only when no snapshot exists. The REST routes now return HTTP 200 with the cached `topActions`/ `repoDecisions` when serving a stale-but-usable pack and reserve 202 for the truly-missing case. The agent orchestrator no longer returns an empty `needs_snapshot_refresh` bundle on staleness: it runs the normal decision/blocker path against the cached pack, marks the run `completed` with `dataQualityStatus: "degraded"`, attaches a freshness warning to the persisted context snapshot, and records the freshness + rebuild status on the run payload. MCP tools degrade the same way. OpenAPI schemas add the `freshness` and `rebuildEnqueued` fields on `ContributorDecisionPack`, `RepoDecisionResponse`, and `DecisionPackRefreshNeeded`. Worker budget safety is preserved: the request path still reads only the latest `signal_snapshots` row and never invokes broad issue/PR listers inline. Tests: - new unit coverage for `loadContributorDecisionPackForServing` across fresh, stale-with-enqueue, stale-without-enqueue, missing, and queue-failure paths, - updated stale-user integration test expects 200 + actionable payload, - new orchestrator test asserts stale-pack runs finish `completed` with a `decision pack is stale ...; background rebuild enqueued` freshness warning, - "interrupted run" test updated to assert the new graceful degradation (queue errors now yield `needs_snapshot_refresh` with `rebuildEnqueued: false` instead of failing the run).
…ild failures Maintainer-review fixups for #15: - tryEnqueueDecisionPackRebuild now records a decision_pack.rebuild_enqueue_failed audit event when JOBS.send throws, so a queue outage no longer silently disables agent.run_failed alerting. - executeDecisionPackRun emits distinct refreshReason values (queue_unavailable / missing_decision_pack / stale_decision_pack / stale_decision_pack_queue_unavailable) so an outage is distinguishable from "no snapshot yet". - DecisionPackRefreshNeeded and DecisionPackRefreshNeededSchema drop the unreachable reason: "stale_snapshot", freshness: "rebuilding", staleSnapshot, dataQuality, and enqueued fields. reason is now exactly "missing_snapshot" and freshness is exactly "missing". - OpenAPI 202 descriptions for both decision-pack routes updated to reflect the new contract; 200 description calls out freshness: "stale" | "rebuilding". - Unify on rebuildEnqueued across the orchestrator payload (dropped snapshotRefreshEnqueued alias) and across the refresh response (dropped enqueued alias). - Public GitHub bot comment (buildPublicAgentCommandComment) emits a one-line "snapshot is stale; rebuild requested" preface when the run's payload.freshness is not "fresh". - Drop unused loadFreshContributorDecisionPack (callers all moved to loadContributorDecisionPackForServing). Tests: - new perf-regression test asserts the serving path never invokes listContributorPullRequests/Issues/RepoStats/Repositories/SyncStates/ SyncSegments/LatestRepoGithubTotalsSnapshots. - new integration assertion covers the per-repo stale path (GET /v1/contributors/:login/repos/:owner/:repo/decision returns 200 with freshness: "rebuilding" and a decision body). - existing needs_snapshot_refresh integration assertions pinned to the new freshness + rebuildEnqueued fields. - interrupted-run test tightened: queue throw now asserts refreshReason: "queue_unavailable" AND a persisted decision_pack.rebuild_enqueue_failed audit row. - stale-orchestrator test pins dataQualityStatus: "degraded" against a fixture whose signalFidelity.status is "complete", so the assertion is no longer trivially satisfied by the fixture default.
9 tasks
JSONbored
requested changes
May 29, 2026
JSONbored
left a comment
Owner
There was a problem hiding this comment.
PR #26 - feat(agent): serve stale decision packs with freshness marker and background rebuild
Signal: REQUEST CHANGES
Required changes:
- Fix branch coverage drop: CI reports 94.86% branches, below 95%.
- Add branch tests for stale snapshot + enqueue failure, missing snapshot + enqueue failure, and repo-decision stale serving paths.
- Prevent repeated stale-pack requests from enqueueing unbounded duplicate rebuild jobs; acceptance asked for a single/background rebuild behavior.
- Confirm API/MCP responses remain bounded and never trigger broad issue/PR scans inline.
Repeated stale-pack requests for the same login no longer enqueue a fresh job; tryEnqueueDecisionPackRebuild checks audit_events for a decision_pack.rebuild_enqueued entry within DECISION_PACK_REBUILD_DEBOUNCE_MS (15s) and short-circuits when one exists. Success path now records a decision_pack.rebuild_enqueued audit event so the debounce has a signal to read. Adds hasRecentAuditEvent in repositories.ts (indexed lookup on audit_events_actor_created_idx) and uses it as the dedupe gate.
Adds error-path coverage for backfill-pr-details (no body, empty repoFullName) and the json-parse .catch arm on build-contributor-evidence and build-contributor-decision-packs.
Contributor
Author
|
Fixups pushed in Addressed the maintainer review findings:
Coverage recalculation: local branch coverage is now Validation run locally:
GitHub checks on the pushed head are passing: Contributor trust and Superagent Security Scan. |
Contributor
Author
|
@JSONbored could you please review the update again? |
Owner
Yes, i'm working on them. |
…stale-serving # Conflicts: # CHANGELOG.md # src/services/decision-pack.ts
…stale-serving # Conflicts: # test/unit/github-commands.test.ts
8 tasks
This was referenced Jul 2, 2026
This was referenced Jul 11, 2026
JSONbored
added a commit
that referenced
this pull request
Jul 11, 2026
…entry (#5046) (#5048) logSelfHostAiProviderFailed fired console.error on every subscription- CLI attempt, not just the final one -- contradicting ai-review.ts's own documented "per-attempt=warn, exhausted=error" policy (#26). GITTENSORY-K (claude_stalled_no_output) hit 2077+ events: pulled raw events sharing the same jobId/trace_id at attempt 0, 1, 2, confirming one review's 3x-retry-per-model loop was independently escalating each attempt to Sentry instead of just the exhausted outcome. Thread finalAttempt through AiRunOptions (mirroring the existing attempt/jobId correlation fields) so the provider's own log can tell a retried attempt (quiet) from a genuinely exhausted one (loud). runWorkersOpinion, the dual-AI tie-break judge, the issue planner, and the AI slop advisory all compute it as "last attempt of the last model" and pass it through. Two of those callers (planner.ts, ai-slop.ts) have no logging of their own -- their retry loops rely entirely on this log for visibility, so a blanket downgrade would have silenced them; unset stays loud by default for exactly that reason.
This was referenced Jul 12, 2026
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
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.
Closes #15.
Summary
freshness: "fresh" | "stale" | "rebuilding" | "missing"marker plusrebuildEnqueued: booleantoContributorDecisionPack, the per-repo decision response, andDecisionPackRefreshNeeded.loadContributorDecisionPackForServinghelper used by the REST API (GET /v1/contributors/:login/decision-pack,…/repos/:owner/:repo/decision), the MCP server (gittensory_get_decision_pack,gittensory_explain_repo_decision), and the agent orchestrator'sexecuteDecisionPackRun.topActions/repoDecisionswith HTTP 200,freshness: "rebuilding"and a backgrounded rebuild. 202 +needs_snapshot_refreshis reserved for the truly-missing case.needs_snapshot_refreshbundle on staleness — it runs the normal decision/blocker actions against the stale pack, finishes the runcompletedwithdataQualityStatus: "degraded", and writes a freshness warning ("decision pack is stale (age Ns); background rebuild enqueued") to the persisted context snapshot.signal_snapshotsrow;buildAndPersistContributorDecisionPackis never invoked inline. Queue-send failures are caught — the response degrades tofreshness: "stale"/rebuildEnqueued: falseinstead of failing the whole request.Acceptance criteria → coverage
topActions/repoDecisions) and orchestrator test (status: "completed"+ actions persisted).freshness,rebuildEnqueued, and existingsnapshotAgeSeconds/staleSnapshotfields, asserted in unit + integration.JOBS.sendper stale request and exactly-once even with multiple serving calls.signal_snapshotsvialoadContributorDecisionPack—buildAndPersistContributorDecisionPackremains the queue-side writer.Test plan
npm run typechecknpx vitest run test/unit— 250 tests passnpx vitest run test/integration— 16 tests passnpx vitest run --config vitest.workers.config.ts— workers suite greennpm run changelog— refreshedCHANGELOG.mdOpen questions for review
AgentRunStatus: kept the existing"completed" | "needs_snapshot_refresh"enum (no new value) and useddataQualityStatus: "degraded"+ a freshness warning to signal staleness on completed runs. Avoids breaking enum consumers; open to introducing acompleted_stalestatus if you'd rather make it explicit.rebuildingsemantics: there's no in-flight queue introspection today, sofreshness: "rebuilding"simply means "stale + a rebuild was just enqueued from this request"."stale"is reserved for when the queue refused the job (caught error). Doc'd insite/troubleshooting.md.Behavioral change to flag for reviewers
stale-userintegration assertion (needs_snapshot_refreshwith empty body and HTTP 202) is intentionally flipped to assert HTTP 200 + actionable payload — this is the contract change the issue is asking for. The legacy missing-snapshot 202 path is unchanged.needs_snapshot_refreshwithrebuildEnqueued: falseinstead of failing the run. The old behavior was an accidental side-effect ofJOBS.sendthrowing unhandled.