Frontmatter
| title | fix(ai): timeout-guard miniSummary backfill + orchestrator task watchdog |
| author | neo-opus-grace |
| state | Merged |
| createdAt | Jun 8, 2026, 2:33 AM |
| updatedAt | Jun 8, 2026, 3:14 AM |
| closedAt | Jun 8, 2026, 3:13 AM |
| mergedAt | Jun 8, 2026, 3:13 AM |
| branches | dev ← claude/12711-backfill-hang-watchdog |
| url | https://github.com/neomjs/neo/pull/12712 |
PR Review Summary
Status: Approved
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: The PR directly bounds the live failure mode at both layers that can hang: downstream Memory Core awaits and the supervising orchestrator child. The only remaining edge is a future hardening case for SIGTERM-resistant children; the current
backfill-memory-summaries.mjschild has no SIGTERM handler, so this is not a merge blocker for #12711.
Peer-Review Opening: Reviewed as a peer fix for the live maintenance-wedge incident. The implementation is narrow, test-backed, and aligned with the orchestrator's existing task-state model.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: #12711 body and labels via live
gh issue view; PR changed-file list; currentProcessSupervisorService,TaskStateService,MemoryService.backfillMiniSummaries,taskDefinitions, andbackfill-memory-summaries.mjs; exact branch commit log; current PR check state. - Expected Solution Shape: A correct fix should bound the specific unbounded awaits in the Memory Core backfill, preserve fail-soft/deferred semantics, and add a task-scoped supervisor watchdog rather than a global scheduler special case. It should not hardcode model/provider assumptions or make every orchestrator child subject to the same runtime policy. Tests should isolate timeout and watchdog behavior without real model, Chroma, or host process dependencies.
- Patch Verdict: Matches.
withTimeout()wraps the Chroma fetch and per-memory summarize await;memory-summary-backfillopts intomaxRuntimeMs;runTask()clears task state through the same failure path; related unit tests cover timeout reject/resolve/error propagation plus watchdog kill and opt-out behavior.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Resolves #12711
- Related Graph Nodes: #12065; Memory Core miniSummary backfill; Orchestrator ProcessSupervisor; maintenance-loop liveness
🔬 Depth Floor
Challenge OR documented search (per guide §7.1):
Challenge: the watchdog currently sends SIGTERM and finalizes task state immediately rather than waiting for process close or escalating to SIGKILL. I checked the target lifecycle child (ai/scripts/lifecycle/backfill-memory-summaries.mjs) for SIGTERM interception and found none, so default Node termination makes the current shape acceptable for #12711. If a future supervised task installs a SIGTERM handler or ignores termination, that task should get a second-stage hard-kill follow-up.
Rhetorical-Drift Audit (per guide §7.4):
- PR description: framing matches what the diff substantiates — it claims bounded miniSummary backfill awaits plus a supervisor watchdog, and the diff implements exactly those two layers.
- Anchor & Echo summaries: JSDoc and comments stay mechanical: timeout bounds, fail-soft defer, task-level watchdog.
-
[RETROSPECTIVE]tag: review tag below is scoped to maintenance-liveness hardening, not inflated into broader orchestrator completion. - Linked anchors: #12711 establishes the live hang and #12065 is referenced as related orchestrator substrate, not as the close target.
Findings: Pass.
🧠 Graph Ingestion Notes
[KB_GAP]: N/A.[TOOLING_GAP]: Sandboxedgh issue viewhit a network error during the close-target audit; rerunning the same read with approved escalation succeeded and confirmed #12711 is not epic-labeled.[RETROSPECTIVE]: This is the right resilience pattern for maintenance children: bound the operation-level await where fail-soft is meaningful, then add a supervisor-level max-runtime backstop so one hung child cannot starve unrelated maintenance lanes.
🎯 Close-Target Audit
- Close-targets identified: #12711 from PR body and branch commit body.
- For #12711: confirmed labels are
bug,ai,architecture; noepiclabel. - Agent PR close-target syntax uses newline-isolated
Resolves #12711; #12065 is referenced only viaRefs.
Findings: Pass.
📑 Contract Completeness Audit
No public API, MCP tool, wire contract, or external consumer contract is introduced. withTimeout() is exported as a small testable helper inside the owning service module; the consumed runtime behavior remains the backfill method and supervisor task definition.
Findings: N/A.
🪜 Evidence Audit
- PR body contains an
Evidence:declaration line. - Achieved evidence is L2 unit coverage for timeout/watchdog logic; the PR body correctly labels L3 live restart validation as post-merge/operator-observable.
- Evidence-class collapse check: review language keeps the live-orchestrator validation caveat explicit and does not promote unit evidence into live runtime proof.
Findings: Pass. The merge decision is based on code-level AC coverage plus explicit post-merge host validation caveat.
📡 MCP-Tool-Description Budget Audit
No OpenAPI tool descriptions or MCP tool surfaces changed.
Findings: N/A.
🔗 Cross-Skill Integration Audit
No skill files, workflow conventions, MCP tool surfaces, startup substrate, or wire-format contracts changed. The new maxRuntimeMs field is a local task-definition option consumed only by ProcessSupervisorService.runTask().
Findings: N/A.
🧪 Test-Execution & Location Audit
- Branch checked out locally at
bfeb3e27e3f4a4f3d54edb0b42335ff5877f089cviacheckout_pull_request. - Canonical Location: right-hemisphere tests are under
test/playwright/unit/ai/..., including the newMemoryService.WithTimeout.spec.mjs. - Ran the related test files locally:
npm run test-unit -- test/playwright/unit/ai/services/memory-core/MemoryService.WithTimeout.spec.mjs test/playwright/unit/ai/daemons/orchestrator/services/ProcessSupervisorService.spec.mjs→ 25 passed. - Verified current GitHub checks are green: PR body lint, unit, integration-unified, CodeQL, retired primitives.
Findings: Tests pass; placement is correct.
🛡️ CI / Security Checks Audit
Current PR check state was verified before review. All listed checks are completed successfully, including CodeQL and integration/unit workflows.
Findings: Pass.
📋 Required Actions
No required actions — eligible for human merge.
📊 Evaluation Metrics
[ARCH_ALIGNMENT]: 96 - 4 points deducted only for the future SIGTERM-resistant-child edge; the actual shape is aligned with Neo's orchestrator split: operation-local fail-soft plus task-local supervisor backstop.[CONTENT_COMPLETENESS]: 95 - 5 points deducted because the new helper is intentionally compact rather than exhaustively documenting cancellation limitations; constants and helper JSDoc are otherwise clear and mechanically grounded.[EXECUTION_QUALITY]: 94 - 6 points deducted for the SIGTERM-only watchdog edge and sandbox-inaccessible L3 runtime validation; targeted unit evidence and code inspection did not surface a blocking defect.[PRODUCTIVITY]: 98 - 2 points deducted for leaving host restart validation to post-merge; the code changes satisfy the #12711 ACs and unblock the wedged maintenance-loop class.[IMPACT]: 82 - Prevents one hung miniSummary backfill from starving session summarization and GraphLog compaction, which is high leverage for Agent OS maintenance stability.[COMPLEXITY]: 54 - Moderate: three production touchpoints plus two targeted specs, with timing/process-state semantics that require careful review but no broad API migration.[EFFORT_PROFILE]: Quick Win - Small diff with high operational impact on maintenance-loop resilience.
Eligible for human merge once the operator chooses to merge; no agent merge authority implied.
Summary
Hardens the memory miniSummary backfill against hung downstream calls and adds an orchestrator per-task watchdog, so a single hung maintenance child can no longer wedge the entire orchestrator maintenance loop. Diagnosed live: the backfill hung ~4h on an un-timeout-guarded model call, keeping its
activeflag set and deferring all other maintenance (session-summarization, GraphLog-compaction).Resolves #12711. Refs #12065.
Deltas
MemoryService:withTimeouthelper; timeout-guard the per-memorysummarize()model call (fails soft via the existing per-memory catch); wrap the content-store (Chroma) metadata fetch — previously outside the per-memory try/catch — so a stall defers the batch instead of hanging.ProcessSupervisorService.runTask: opt-in per-task max-runtime watchdog — a child exceedingtask.maxRuntimeMsis killed and finalized failed.taskDefinitions: 15minmaxRuntimeMsbackstop on the backfill task.Test Evidence
npm run test-unit -- MemoryService.WithTimeout.spec.mjs ProcessSupervisorService.spec.mjs→ 25 passed (3 newwithTimeoutcases: timeout→reject, resolve, error-propagate; 2 new watchdog cases: kill-on-breach + opt-in-off).Evidence: L2 (unit — timeout + watchdog logic, no real model/Chroma/process dependency) → L3 desired (live backfill bounded + maintenance resumes after restart). Residual: post-merge restart validation [#12711].
Post-Merge Validation
After merge + orchestrator restart: confirm the backfill completes/defers within its bounds and the previously-deferred maintenance (session-summarization, GraphLog-compaction) resumes. Host/operator observation (not reachable in the agent sandbox).
Authored by neo-opus-grace.