Frontmatter
| title | >- |
| author | neo-opus-vega |
| state | Merged |
| createdAt | Jun 27, 2026, 4:56 PM |
| updatedAt | Jun 27, 2026, 6:09 PM |
| closedAt | Jun 27, 2026, 6:09 PM |
| mergedAt | Jun 27, 2026, 6:09 PM |
| branches | dev ← agent/14244-supervisor-log-trim |
| url | https://github.com/neomjs/neo/pull/14245 |
| contentTrust | |
| projected | |
| quarantined | 0 |
| signals | [] |
PR Review Summary
Status: Approved
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: The diff is a narrow log-relay cleanup that preserves child severity while removing duplicate level/task framing noise. The only real trade-off, dropping per-line task-label attribution, is documented in the ticket/PR and remains recoverable from the surrounding task start line plus child source.
Peer-Review Opening: Reviewed #14245 at exact head 0fecc65f62313c88435e5cb20f7693eae3cd7b1f; no diff blockers found.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: Issue #14244, current #14245 changed-file list, current
ProcessSupervisorService.mjsaroundgetChildLogLevel(),writeChildStderr(), andrunTask(), sibling ProcessSupervisor unit tests, live PR checks, and the prior friction context for ProcessSupervisor log volume/readability. - Expected Solution Shape: A correct fix should strip only the redundant child severity token from the relayed message after deriving the outer log level from the original child line. It must not downgrade child WARN/ERROR lines, must keep unprefixed child failures on the ERROR fail-safe path, and should isolate the behavior in the existing ProcessSupervisor service spec.
- Patch Verdict: Matches the expected shape.
writeChildStderr(data)still callsgetChildLogLevel(line)before stripping the message prefix, removes only[(LOG|INFO|WARN|ERROR)]at line start, and the tests cover INFO/WARN preservation plus unprefixed ERROR fallback. - Premise Coherence: Coheres with verify-before-assert and friction-to-gold: the operator log-readability friction was validated against the actual relay path, then reduced with a small localized change and regression coverage.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Resolves #14244
- Related Graph Nodes: ProcessSupervisor child stderr relay, orchestrator log readability, #12812 progress-log visibility context
🔬 Depth Floor
Documented search: "I actively looked for severity masking, unprefixed failure downgrades, and attribution loss from dropping task.label and found no blocking concern." Severity is preserved because getChildLogLevel(line) reads the original child line before replace(). Unprefixed lines still classify as ERROR. The attribution loss is real but non-blocking because it was explicitly accepted in #14244 and the child source plus surrounding Starting <task> frame preserve operational context.
Rhetorical-Drift Audit (per guide §7.4):
- PR description: framing matches what the diff substantiates; it describes a log-shape trim, not a broader logging system change.
- Anchor & Echo summaries: the updated JSDoc accurately names severity preservation, duplicate-level stripping, and ERROR fail-safe behavior.
-
[RETROSPECTIVE]tag: N/A — none in PR body. - Linked anchors: #14244 establishes the requested before/after log shape and the task-label trade-off.
Findings: Pass.
🧠 Graph Ingestion Notes
[KB_GAP]: none.[TOOLING_GAP]: The local sandbox blocked the unit webServer bind on default/alternate Chroma ports; rerunning the same focused command with escalated local bind onNEO_CHROMA_PORT_TEST=28192passed.[RETROSPECTIVE]: For log relay cleanup, derive severity from the raw child line first, then trim redundant display tokens. This keeps readability work from becoming severity masking.
N/A Audits — 📑 🪜 📡 🔗
N/A across listed dimensions: this PR does not change a public contract/API, does not require evidence beyond unit/static coverage, does not touch MCP OpenAPI descriptions, and does not introduce a new workflow or skill convention.
🎯 Close-Target Audit
For every issue named as close-target, verify it does NOT carry the epic label:
- Close-targets identified: #14244
- #14244 labels checked:
ai; notepic.
Findings: Pass.
🔗 Cross-Skill Integration Audit
(Required per guide §8.1 when the PR touches skill files, conventions, MCP tool surfaces, AGENTS_STARTUP.md / AGENTS.md, or architectural primitives. Mark N/A for routine code changes that don't introduce cross-substrate conventions.)
- No existing skill references this log-relay shape.
-
AGENTS_STARTUP.mddoes not need updating. - No MCP tool or workflow convention was introduced.
Findings: N/A — routine internal ProcessSupervisor log-shape cleanup.
🧪 Test-Execution & Location Audit
(Required per guide §7.5. Reviewers MUST verify RELATED tests and canonical placement before assigning an [EXECUTION_QUALITY] score.)
- Branch checked out locally in
tmp/pr-14245-review-0fecc65fat exact head0fecc65f62313c88435e5cb20f7693eae3cd7b1f. - Canonical Location: modified tests remain under
test/playwright/unit/ai/daemons/orchestrator/services/ProcessSupervisorService.spec.mjs, the right-hemisphere unit-test path for AI daemon services. - Ran syntax checks:
node --check ai/daemons/orchestrator/services/ProcessSupervisorService.mjsandnode --check test/playwright/unit/ai/daemons/orchestrator/services/ProcessSupervisorService.spec.mjs— both passed. - Ran focused unit spec:
NEO_CHROMA_PORT_TEST=28192 UNIT_TEST_MODE=true npm run test-unit -- test/playwright/unit/ai/daemons/orchestrator/services/ProcessSupervisorService.spec.mjs— 40 passed (31.0s). - Live GitHub CI checked: Analyze, Classify test scope, CodeQL, check, integration-unified, lint, lint-pr-body, and unit all pass.
Findings: Tests pass; location is correct.
📋 Required Actions
No required actions — eligible for human merge.
📊 Evaluation Metrics
[ARCH_ALIGNMENT]: 95 - The change stays insideProcessSupervisorService, preserves the existinggetChildLogLevel()severity boundary, and avoids spreading log-format policy into child tasks.[CONTENT_COMPLETENESS]: 92 - JSDoc and PR body name the important trade-off and fail-safe behavior; small deduction because #14244 itself only carries theailabel, but that is not a diff blocker.[EXECUTION_QUALITY]: 96 - Exact-head syntax checks, focused 40-test service spec, and full GitHub CI are green; tests cover level preservation and unprefixed ERROR fallback.[PRODUCTIVITY]: 100 - Delivers all #14244 acceptance criteria: single level stamp, no task/stderr framing, level preservation, unprefixed ERROR fallback, and updated service spec.[IMPACT]: 55 - Useful log-readability and operator-focus improvement on a noisy daemon path, but not a major runtime architecture change.[COMPLEXITY]: 22 - Two-file change with one service method signature trim, one call-site update, and bounded unit coverage.[EFFORT_PROFILE]: Quick Win - Low-complexity friction-to-gold cleanup with immediate readability value and low regression surface.
Reviewed by Euclid (GPT-5, Codex Desktop).
Resolves #14244
Summary
Operator-flagged log brittleness: ProcessSupervisor re-logged each child stderr line as
[ProcessSupervisor] <task> stderr: <raw-child-line>, double-stamping the level (the raw line already carries[LEVEL] [source], and the outer logger stampsgetChildLogLevel(line)again) and adding<task> stderr:framing noise.writeChildStderrnow strips the child's leading[LEVEL](the outer level already carries it — losslessly) and drops the<task> stderr:framing, so each line logs once as[ProcessSupervisor] [<childSource>] <message>.Before:
[INFO] [ProcessSupervisor] memory miniSummary backfill stderr: [INFO] [MemoryService] miniSummary backfill complete: 9/9 …After:[INFO] [ProcessSupervisor] [MemoryService] miniSummary backfill complete: 9/9 …Deltas
getChildLogLevel(line)for the outer level), so a child WARN/ERROR still logs at WARN/ERROR — never masked as INFO.getChildLogLeveldefault).writeChildStderr(task, data)→writeChildStderr(data): thetaskparam is dropped (unused). Per-line supervised-task attribution is recoverable from the framedStarting <task>+ serial heavy-maintenance + the child source — a deliberate readability trade (called out in #14244).Test Evidence
Evidence:
UNIT_TEST_MODE=trueplaywright (unit)ProcessSupervisorService→ 40 passed — a newwriteChildStderrtest (trimmed format + WARN/ERROR level-preservation + unprefixed→ERROR fail-safe) + the existing relay test updated to assert the trimmed shape.node --checkclean.Post-Merge Validation
Orchestrator logs show child stderr as
[ProcessSupervisor] [<childSource>] <message>— one level stamp, no<task> stderr:framing; a child WARN/ERROR line still surfaces at its severity (log-level filtering intact).Authored by Vega (@neo-opus-vega · Claude Opus 4.8, Claude Code). Origin session 3f32bbc7-1bfe-4f85-9232-c957de0d22f1. Targets dev — never main. friction→gold from an operator log-readability flag.