Skip to content

fix: lower successful agent stop completion logs#101703

Merged
vincentkoc merged 1 commit into
openclaw:mainfrom
ZengWen-DT:fix/101678-stop-completion-error-log
Jul 7, 2026
Merged

fix: lower successful agent stop completion logs#101703
vincentkoc merged 1 commit into
openclaw:mainfrom
ZengWen-DT:fix/101678-stop-completion-error-log

Conversation

@ZengWen-DT

Copy link
Copy Markdown
Contributor

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:

$ set -a; source <local-openclaw-env>; set +a; pnpm openclaw --log-level error agent --local --model deepseek/deepseek-v4-pro --session-key agent:main:issue-101678-proof-errorlevel --message "Reply exactly: OPENCLAW_STOP_ERRORLEVEL_OK" --json --timeout 180 >after.stdout 2>after.stderr
completion-log-matches=after.stdout:0
after.stderr:0
reply-match=4
stop-reason-match=2

The JSON output contained the expected assistant reply and successful completion metadata:

"finalAssistantVisibleText": "OPENCLAW_STOP_ERRORLEVEL_OK"
"stopReason": "stop"
"completion": {
  "stopReason": "stop",
  "finishReason": "stop"
}

Focused validation:

node scripts/run-vitest.mjs src/agents/agent-command.ingress-diagnostics.test.ts
Test Files  1 passed (1)
Tests  17 passed (17)

node scripts/run-vitest.mjs src/agents/agent-run-terminal-outcome.test.ts
Test Files  1 passed (1)
Tests  16 passed (16)

node scripts/run-tsgo.mjs -p tsconfig.core.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/core.tsbuildinfo
passed

node scripts/run-tsgo.mjs -p test/tsconfig/tsconfig.core.test.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/core-test.tsbuildinfo
passed

pnpm exec oxfmt --check --threads=1 src/agents/agent-command.ts src/agents/agent-command.ingress-diagnostics.test.ts
All matched files use the correct format.

git diff --check
passed

pnpm build
passed

AI-assisted: AI-assisted implementation and validation; proof run manually.

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: S labels Jul 7, 2026
@clawsweeper

clawsweeper Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 7, 2026, 10:08 AM ET / 14:08 UTC.

Summary
The PR changes agent lifecycle completion logging to choose info, warn, or error from normalized terminal outcome metadata and adds focused diagnostics tests for successful, timeout, cancelled, and failed endings.

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
Persistent data-model change detected: vector/embedding metadata: src/agents/agent-command.ingress-diagnostics.test.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #101678
Summary: This PR is a candidate fix for the canonical open issue about successful stopReason=stop completions logged at error severity, with one overlapping open PR for the same root cause.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • none.

Risk before merge

  • [P1] A second open PR targets the same canonical issue; maintainers should choose one implementation and avoid landing duplicate overlapping fixes.

Maintainer options:

  1. Decide the mitigation before merge
    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.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P2] No repair lane is needed: this active PR has sufficient proof and no concrete code finding; normal review should choose one overlapping PR and let exact-head merge gates decide readiness.

Security
Cleared: The diff only changes in-process agent logging severity and focused tests; no security or supply-chain-sensitive surface is touched.

Review details

Best 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 changes

Label changes:

  • add P2: This is a normal-priority agent observability fix with limited blast radius: it reduces false error alerts without changing run delivery or persisted state.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix live CLI output from a local DeepSeek run showing no error-level completion log and successful stop metadata, plus focused validation output.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes after-fix live CLI output from a local DeepSeek run showing no error-level completion log and successful stop metadata, plus focused validation output.

Label justifications:

  • P2: This is a normal-priority agent observability fix with limited blast radius: it reduces false error alerts without changing run delivery or persisted state.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes after-fix live CLI output from a local DeepSeek run showing no error-level completion log and successful stop metadata, plus focused validation output.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix live CLI output from a local DeepSeek run showing no error-level completion log and successful stop metadata, plus focused validation output.
Evidence reviewed

PR surface:

Source +34, Tests +57. Total +91 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 36 2 +34
Tests 1 57 0 +57
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 93 2 +91

What I checked:

  • Root policy read: Root AGENTS.md was read fully; it specifically directs agent run terminal state to normalize through the terminal-outcome helper, which is the policy seam this PR uses. (AGENTS.md:98, c8d95da14c5b)
  • Scoped policy read: src/agents/AGENTS.md was read fully; its test/runtime import guidance was applied while checking the new agent-command import and test shape. (src/agents/AGENTS.md:1, c8d95da14c5b)
  • Current main behavior: Current main logs every non-end_turn stop reason with console.error inside emitLifecycleEnd, so a successful stopReason=stop completion still produces error-level output. (src/agents/agent-command.ts:1877, c8d95da14c5b)
  • PR implementation: The PR adds resolveAgentRunLifecycleEndLogLevel and uses the existing subsystem logger at the lifecycle-end call site without changing the lifecycle event payload. (src/agents/agent-command.ts:254, 12c5b0d9a189)
  • Terminal outcome contract: The canonical helper treats status=ok with stopReason=stop as completed and only treats stop/rpc as cancellation for non-success terminal inputs. (src/agents/agent-run-terminal-outcome.ts:122, c8d95da14c5b)
  • Regression coverage: The PR adds focused unit coverage for successful stop/tool-use, ordinary end_turn, timeout, cancelled, and failed lifecycle-end log-level decisions. (src/agents/agent-command.ingress-diagnostics.test.ts:66, 12c5b0d9a189)

Likely related people:

  • steipete: Current blame attributes the affected lifecycle logging and terminal-outcome helper to the grafted main snapshot authored by Peter Steinberger, and git log -S shows earlier agent runtime movement by the same author. (role: recent area contributor; confidence: medium; commits: 903a09ab858d, 00b57145ff01; files: src/agents/agent-command.ts, src/agents/agent-run-terminal-outcome.ts)
  • vincentkoc: The shipped v2026.6.11-era history contains the same affected logging branch and is useful release-boundary provenance, though it is low-confidence ownership of the original behavior. (role: release boundary contributor; confidence: low; commits: e085fa1a3ffd; files: src/agents/agent-command.ts)
  • openperf: Merged PR 58178 changed the same subagent command lifecycle area around live model-switch handling and lifecycle reset semantics. (role: adjacent feature contributor; confidence: medium; commits: 91340b23e030, 9c5316c2c0c8; files: src/agents/agent-command.ts, src/agents/agent-command.live-model-switch.test.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal backlog priority with limited blast radius. labels Jul 7, 2026
@vincentkoc vincentkoc self-assigned this Jul 7, 2026
@vincentkoc

Copy link
Copy Markdown
Member

maintainer review is complete and this is land-ready at exact head 12c5b0d9a18951cf11b550cebbdd7cf46b5e78a2.

what was validated:

  • issue Successful run completions (stopReason=stop) logged at error severity, sometimes twice per run #101678 reproduces on shipped v2026.6.11: successful stopReason=stop completions are emitted at error severity.
  • the patch uses the canonical terminal-outcome classifier; successful stops/tool use log at info, timeout-attributed endings at warn, failed/cancelled endings remain error, and end_turn/missing reasons remain silent.
  • node scripts/run-vitest.mjs src/agents/agent-command.ingress-diagnostics.test.ts src/agents/agent-run-terminal-outcome.test.ts: 33/33 passed.
  • native prepare gates: pnpm build and pnpm check passed.
  • exact-head hosted CI run 28871915379: all relevant checks passed.
  • two fresh Codex autoreviews, including the final review against current origin/main: no actionable findings.

proof gaps/noise:

  • direct secretless AWS Crabbox capacity did not allocate after 5m30s; no lease was created.
  • the stale-base local full suite reported 24 failures in unrelated extension tests outside this two-file agent diff. native hosted-evidence mode accepted the green exact-head CI instead.

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.

@vincentkoc
vincentkoc merged commit 6db0506 into openclaw:main Jul 7, 2026
146 of 152 checks passed
@vincentkoc

Copy link
Copy Markdown
Member

Merged via squash.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Successful run completions (stopReason=stop) logged at error severity, sometimes twice per run

2 participants