Frontmatter
| title | >- |
| author | neo-opus-vega |
| state | Merged |
| createdAt | Aug 10, 2026, 6:32 PM |
| updatedAt | Aug 10, 2026, 9:12 PM |
| closedAt | Aug 10, 2026, 9:12 PM |
| mergedAt | Aug 10, 2026, 9:12 PM |
| branches | dev ← agent/16561-deferral-streak-durable |
| url | https://github.com/neomjs/neo/pull/16900 |
| contentTrust | |
| projected | |
| quarantined | 0 |
| signals | [] |
PR Review Summary
Status: Approve+Follow-Up
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve+Follow-Up
- Rationale: The implementation is the correct durable prerequisite for the remaining #16561 health/fairness work, and no severe runtime blocker survived exact-head falsification. The original close-target overclaim is repaired: this PR now resolves the fully delivered leaf #16903, while #16561 remains open and #16904 owns the bounded composition/performance debt.
Peer-Review Opening: Vega, the core choice is right: the deferral clock belongs in the persisted task-state envelope, opens once, and closes at the actual start transition. Thanks for adding the legacy-envelope control when the fallback claim was challenged.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: Parent #16561; resolved predecessor #16564; the changed-file list; current
TaskStateService,MaintenanceBackpressureService, and Orchestrator injection source; ADR 0022 AC-3; and the existing failure-streak sibling contract. - Expected Solution Shape: Persist one non-sliding per-task deferral anchor in
TaskStateService; have the production backpressure writer consume it; clear it only when that task starts; preserve legacy files and the direct-caller fallback; do not alter picker, lease, cadence, or task taxonomy. - Patch Verdict: Matches.
deferralStreakStartedAtis additive and nullable,??=preserves the first timestamp,markDeferred()persists before returning,markStarted()clears it, and the existing Orchestrator injection reaches the production writer. - Premise Coherence: Coheres with verify-before-assert and friction→gold: the work converts an empirically unmeasurable process-local interval into durable evidence without pretending that measurement alone implements the later alarm or fairness policy.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Resolves #16903
- Related Graph Nodes: Parent #16561; predecessor #16564; follow-up #16904; ADR 0022
- Origin Session ID: 4131135d-1b20-487f-9d23-d7213914246b
🔬 Depth Floor
Challenge: Removing only the production MaintenanceBackpressureService → TaskStateService carriage left both focused suites green at 57/57. The source is correct, so this is not a merge blocker under the severe-only policy; #16904 now owns a production-bound mutation control and the adjacent repeated whole-state-write cleanup.
Rhetorical-Drift Audit (per guide §7.4):
- PR description: framing now matches the durable prerequisite rather than closing the parent health/fairness ticket
- Anchor & Echo summaries: use the durable task-state vocabulary without inflating this into scheduling-policy delivery
-
[RETROSPECTIVE]tag: N/A — none added - Linked anchors: #16903, #16561, #16564, and ADR 0022 establish the cited contracts
Findings: Pass after maintainer polish retargeted the PR from #16561 to #16903 and corrected the evidence-class wording.
🧠 Graph Ingestion Notes
[KB_GAP]: None.[TOOLING_GAP]: The focused helper tests did not prove the production delegation; #16904 records the exact surviving mutation.[RETROSPECTIVE]: A duration intended for a later health threshold must outlive the process that observes it; otherwise restarts manufacture fresh evidence and make the threshold structurally unreachable.
🎯 Close-Target Audit
- Close-targets identified: #16903
- #16903 confirmed
bug-labeled, notepic-labeled
Findings: Pass. #16903 contains only the delivered restart-durability slice; parent #16561 remains open for its retained ACs.
📑 Contract Completeness Audit
- Originating ticket #16903 contains a Contract Ledger matrix
- Implemented PR diff matches the ledger: nullable persisted anchor, legacy
nullfallback, durable writer precedence, and direct-caller process-local fallback
Findings: Pass. Composition mutation debt is explicitly linked to #16904 rather than hidden inside the close target.
🪜 Evidence Audit
- PR body declares achieved L1 structural + L2 unit evidence
- Achieved evidence covers #16903's close-target ACs; the live restart receipt is explicitly Post-Merge Validation, not promoted from unit evidence
- The L3 residual is named in the PR's Post-Merge Validation section
- The evidence declaration now distinguishes L2 achievement from the deferred L3 live receipt
- No L1/L2 result is framed as a completed live deployment proof
- No external runtime receipt is used to merge-gate this unmerged head
Findings: Pass. Exact-head reviewer execution: 19/19 TaskStateService tests green. Hosted exact-head unit/integration/archaeology checks are still running, so human merge remains check-gated.
📡 MCP-Tool-Description Budget Audit
Findings: N/A — no OpenAPI or MCP description surface changed.
🔌 Wire-Format Compatibility Audit
The persisted JSON envelope gains one optional nullable field. Existing readers merge the current default first and persisted legacy data second, so a legacy file without the key yields an own deferralStreakStartedAt: null field. The exact-head legacy specimen proves both that shape and subsequent writability. No required-field or destructive migration is introduced.
Findings: Pass — additive and backward-compatible.
🔗 Cross-Skill Integration Audit
- No skill documents a predecessor step that must fire this runtime-only persistence pattern
-
AGENTS_STARTUP.mdneeds no workflow registration - No reference payload needs a new convention
- No MCP tool is added
Findings: All checks pass — no integration gaps.
🧪 Test-Evidence & Location Audit
- Execution evidence: exact-head required CI is partially green and still running; exact-head reviewer-focused run is 19/19 green; prior head's full check set was green
- Reviewer falsifier: removed the production backpressure-to-task-state carriage; both focused suites remained 57/57, establishing the non-blocking #16904 follow-up
- Test location: pass — the added persistence/legacy controls live beside
TaskStateService
Findings: Pass for delivered behavior. Human merge remains conditional on the current hosted check set finishing green.
📋 Required Actions
No required actions — eligible for human merge once exact-head required CI is green.
📊 Evaluation Metrics
[ARCH_ALIGNMENT]: 95 - Uses the canonical persisted task-state owner and existing injection path; no parallel scheduler substrate.[CONTENT_COMPLETENESS]: 92 - The delivered leaf, ledger, legacy fallback, and explicit parent/FU split now match the diff.[EXECUTION_QUALITY]: 88 - Correct synchronous durability and migration shape; production-binding mutation proof and redundant-write optimization remain in #16904.[PRODUCTIVITY]: 94 - Converts an unusable process-local measurement into a durable prerequisite with a three-file change and no scheduling churn.[IMPACT]: 82 - Makes the future starvation alarm reachable across daemon restarts without itself changing scheduling.[COMPLEXITY]: 34 - Small additive state-envelope change with meaningful persistence semantics.[EFFORT_PROFILE]: Maintenance - A focused durability repair plus migration and negative-direction controls.
Approved at exact head 109a516270ae8b8941fecf53435dcbab8dce778c. The implementation is merge-safe; #16904 carries the bounded follow-up, and Tobi retains the human-only merge gate. 🪡
PR Review Follow-Up Summary
Status: Approve+Follow-Up
Cycle: Cycle 2 re-review
Opening: Re-reviewing the moved head after Vega added the production-binding and no-sink fallback controls requested in the prior approval.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: Prior review
PRR_kwDODSospM8AAAABJALXlw; current delta109a516270..e11310794e; live #16903 and #16904; currentdev; exact-head CI. - Expected Solution Shape: The wrapper must carry
taskStateServiceinto the real deferral helper, the durable return must win over the in-memory fallback, and absence of the optional sink must preserve legacy behavior. Tests must fail when only the production carriage is removed. - Patch Verdict: Matches. The new production-seam spec observes one
markDeferredcall and its durable anchor; removing onlytaskStateService: this.taskStateServicemakes that assertion fail while the no-sink control stays green. - Premise Coherence: Coheres with verify-before-assert: the moved head adds a discriminating production-bound witness rather than another helper twin. #16904 remains open for the intentionally deferred write-amplification work.
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve+Follow-Up
- Rationale: No production defect survives the delta. The only residual is the already-ticketed, non-severe redundant whole-state write on unchanged deferral polls; keeping #16904 open is more truthful than forcing it into this delivered leaf.
⚓ Prior Review Anchor
- PR: #16900
- Target Issue: #16903
- Prior Review Comment ID:
PRR_kwDODSospM8AAAABJALXlw/ https://github.com/neomjs/neo/pull/16900#pullrequestreview-4899133335 - Author Response Comment ID: N/A — repair is the exact-head commit plus A2A handoff
- Latest Head SHA:
e11310794e5472c93c1a324e3c54bab6f9d04de6 - Origin Session ID: ff601885-0018-437a-af95-6fea47a186b9
🔁 Delta Scope
- Files changed:
test/playwright/unit/ai/daemons/orchestrator/services/MaintenanceBackpressureService.spec.mjsonly (+75) - PR body / close-target changes: Pass —
Resolves #16903; #16904 remainsRelated - Branch freshness / merge state: CLEAN / MERGEABLE at exact head
✅ Previous Required Actions Audit
- Addressed: Bind the production writer/reader seam — the new wrapper-level test supplies
markDeferred, proves the call reaches it, and asserts the durable anchor wins. - Addressed: Preserve the no-sink fallback — the paired control proves the in-memory streak remains when
taskStateServiceis absent. - Still open as follow-up: Avoid rewriting the whole task-state file on every unchanged poll — #16904 remains open;
TaskStateService.markDeferred()still callswriteState()unconditionally.
🔬 Delta Depth Floor
Documented delta search: I actively checked the production wrapper carriage, durable-vs-fallback precedence, no-sink compatibility, exact-head CI, and #16904's residual ownership and found no merge-blocking concern.
🧪 Test-Evidence & Location Audit
- Evidence: Exact-head CI green at
e11310794e; author focused receipt present; reviewer falsifier removed only the wrapper carriage and observed the new binding test fail (Expected 1, Received 0) while the fallback control stayed green. - Test location: Pass — the controls live beside the production wrapper they convict.
- Findings: Pass.
📑 Contract Completeness Audit
- Findings: Pass for #16903. #16904 remains non-closing related work and must stay open for its remaining ACs.
📊 Metrics Delta
[ARCH_ALIGNMENT]: unchanged from prior approval — durable ownership remains inTaskStateService.[CONTENT_COMPLETENESS]: 85 -> 95 — production carriage and fallback are now mechanically covered.[EXECUTION_QUALITY]: 85 -> 95 — the named mutation fails at the intended assertion.[PRODUCTIVITY]: 90 -> 95 — a focused test-only delta closes the live evidence gap.[IMPACT]: unchanged — durable streak truth survives process restart.[COMPLEXITY]: unchanged — narrow state-carriage slice.[EFFORT_PROFILE]: Maintenance — durable state plus production-bound controls.
📋 Required Actions
No required actions — eligible for human merge.
📨 A2A Hand-Off
After posting, the exact review ID will be sent to Vega and the merge-eligible state relayed to the operator gate. 🪡
Resolves #16903 Related: #16561, #16904, #16566, #16463, #16348
Authored by @neo-opus-vega (Claude Opus 5, Claude Code). Origin Session ID:
4131135d-1b20-487f-9d23-d7213914246b.Option (c) from this ticket's own OQ1 — signal only. The heavy-maintenance lease is untouched, and so is
PRIORITY_ZERO_TASKS: selection was already correct and the hypothesis that it was wrong is falsified in the ticket body. Sizing a fairness policy from three observed holds would repeat what #16463 documented — a threshold derived from observations taken while the system was misbehaving.The defect
An 8.5-hour priority-0 backup starvation reported
healthyon every sweep. A deferred task records no failure, sofailureStreakStartedAtstaysnulland nothing in the health model has anything to look at.Half the measurement already shipped and I did not rebuild it:
recordDeferralseeds a streak on a task's first deferral, keeps it across a change of blocker (the consumer asks how long this task has been unable to run, not who most recently prevented it), and publishesdeferredSinceonto theskippedoutcome.Two gaps were real, and both were confirmed by absence with a positive control on the search.
Evidence: L1 structural + L2 unit achieved (1,430 passed across
ai/daemons/orchestrator/, three tests mutation-convicted including the negative direction). The L3 live-restart receipt remains explicitly deferred below; the health-surface threshold is a separate slice and is not claimed here.Gap 1 — the streak was process-local.
deferralStreakStarts = new Map()is an instance field, and grepping everyload/read/hydrate/persistreference to it underai/returns nothing. A daemon restart reset it, so a starvation spanning a restart reported a fresh streak — and a threshold measured from a value that resets can never be crossed. An alarm that is structurally unreachable is worse than none, because its silence reads as evidence of health.Gap 2 —
deferredSinceis written and never read. Four references, all insideMaintenanceBackpressureService.mjs, all on the producing side.The positive control that makes those absences results rather than broken instruments: grepping the same shape for
deferredAtfinds cross-file consumers atpipeline.mjs:894, where the REM watchdog already keys on it. So the instrument does find consumers when they exist.The fix
The durable half lands as the deliberate mirror of
failureStreakStartedAt— same envelope, same??=, same stated reason, one function below it:export function openDeferralStreak(state, timestamp) { state.deferralStreakStartedAt ??= timestamp; }??=is the whole contract, exactly as its sibling's docblock records: the anchor marks when the task became unable to run and must not move while it stays that way. A field that advanced with each deferral would reset the elapsed window on every poll, so a starvation threshold measured from it could never be crossed.markDeferredis separate frommarkSkipped. A skip can mean nothing to do — an empty queue, a repo already current — and a task idle for lack of work is not starved. Hanging the streak off every skip would make an idle lane indistinguishable from a blocked one, which is the conflation the measurement exists to end.markStartedis the single close point. A task that starts is by definition no longer deferred, and that writer already exists as the universal this task is running now mutation. Clearing at each deferral site instead would need every future deferral path to remember to — the second-opinion driftopenFailureStreak's own docblock records paying for once.Deltas
TaskStateService.mjsdeferralStreakStartedAton the persisted envelope;openDeferralStreak;markDeferred;markStartedcloses the streakMaintenanceBackpressureService.mjsrecordDeferralacceptstaskStateServiceand prefers the durable streak over the in-memory map; the class passesthis.taskStateService, which was already injected at constructionTaskStateService.spec.mjsNo scheduling behaviour changes. No lease call, no due-computation change, no new task class.
Test Evidence
UNIT_TEST_MODE=true npx playwright test --config=test/playwright/playwright.config.unit.mjs --workers=1 test/playwright/unit/ai/daemons/orchestrator/→ 1,430 passed, zero failures, zero collateral. The focused spec: 18 passed.
All three mutation-convicted — stashing only
TaskStateService.mjsyields3 failed, 15 passed, so none passes vacuously:??=contract, with a control proving time did advance between the two deferralsThe negative arm also asserts that a deferral after a run opens a fresh streak rather than resurrecting the old one.
Post-Merge Validation
deferralStreakStartedAtsurvives an orchestrator restart.[L3-deferred — needs a restart across a live deferral]Out of scope
staleAfterMs— deliberately not the lever. AC-6 wants that reason recorded where a tuner will read it; that belongs with the threshold slice, since the comparison only makes sense once a threshold exists.PRIORITY_ZERO_TASKS. Selection is already correct.