fix: lower successful agent stop completion logs#101703
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 7, 2026, 10:08 AM ET / 14:08 UTC. Summary PR surface: Source +34, Tests +57. Total +91 across 2 files. Reproducibility: yes. Current main has a source-level reproduction path: a successful attempt with meta.stopReason set to stop reaches emitLifecycleEnd, which currently logs every non-end_turn stop reason through error severity. Review metrics: 1 noteworthy metric.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land one narrow implementation that aligns lifecycle-end log severity with the existing terminal outcome classification, preserves lifecycle payload shape, and closes the overlapping PR once a canonical branch is selected or merged. Do we have a high-confidence way to reproduce the issue? Yes. Current main has a source-level reproduction path: a successful attempt with meta.stopReason set to stop reaches emitLifecycleEnd, which currently logs every non-end_turn stop reason through error severity. Is this the best way to solve the issue? Yes. Reusing buildAgentRunTerminalOutcome is the narrowest maintainable fix because it follows the existing terminal-state contract instead of adding a separate stop-reason map; the overlapping PR is not a safer supersession target. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against c8d95da14c5b. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +34, Tests +57. Total +91 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
maintainer review is complete and this is land-ready at exact head what was validated:
proof gaps/noise:
no docs or changelog change is required. the reported raw/announce run-id duplication is deliberately left out because the evidence does not prove one lifecycle event is emitted twice. |
|
Merged via squash.
|
Closes #101678
What Problem This Solves
Fixes an issue where successful agent runs could emit lifecycle completion logs at error severity when the run ended with normal provider metadata such as
stopReason=stop.This was noisy for operators because clean completions could look like failures in error-level monitoring.
Why This Change Was Made
Agent lifecycle end logging now uses the existing normalized terminal outcome classification before choosing a log level: completed outcomes log at info, timeout outcomes log at warn, and cancelled/failed outcomes remain error-level. The lifecycle event payload is unchanged; this PR only changes the severity used for the completion log line.
Risk note: this touches agent-run observability, so the regression coverage keeps cancelled, failed, and timeout endings out of the successful-completion path.
User Impact
Successful agent completions no longer pollute error-level logs just because the provider reports
stopReason=stop. Operators should see fewer false-positive error alerts while real failed, cancelled, aborted, and timeout outcomes remain visible at elevated severity.Evidence
Live behavior proof with local DeepSeek (
deepseek/deepseek-v4-pro) after the fix:The JSON output contained the expected assistant reply and successful completion metadata:
Focused validation:
AI-assisted: AI-assisted implementation and validation; proof run manually.