Skip to content

Doctor: expose tool result cap findings#97500

Merged
giodl73-repo merged 3 commits into
openclaw:mainfrom
giodl73-repo:doctor-tool-result-cap-lint-findings
Jul 2, 2026
Merged

Doctor: expose tool result cap findings#97500
giodl73-repo merged 3 commits into
openclaw:mainfrom
giodl73-repo:doctor-tool-result-cap-lint-findings

Conversation

@giodl73-repo

@giodl73-repo giodl73-repo commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add default-disabled structured lint findings for core/doctor/tool-result-cap
  • map existing toolResultMaxChars advice into warning findings with stable path/target/fixHint metadata
  • keep legacy openclaw doctor note output as the authority for normal Doctor runs

Public contract

  • No public SDK, plugin manifest, config schema, or persisted data-model contract changes.
  • The structured check is opt-in via defaultEnabled: false; default doctor --lint skips it while --all and --only core/doctor/tool-result-cap include it.
  • No repair path is added.

Real behavior proof

Source CLI proof with isolated OPENCLAW_CONFIG_PATH containing agents.defaults.contextLimits.toolResultMaxChars = 16000:

  • default pnpm --silent openclaw doctor --lint --json: checksRun: 25, checksSkipped: 8, toolResultCapFindings: 0
  • explicit pnpm --silent openclaw doctor --lint --json --only core/doctor/tool-result-cap: checksRun: 1, checksSkipped: 32, finding path agents.defaults.contextLimits.toolResultMaxChars, target agents.defaults, severity warning

Review updates

  • Addressed ClawSweeper's inherited-default target enumeration P2: inherited defaults now produce per-agent findings where appropriate.
  • Addressed ClawSweeper's per-agent path-shape P2: explicit per-agent findings use agents.list.<id>.contextLimits.toolResultMaxChars and target: agents.list.<id>; inherited-default findings remain anchored to agents.defaults.contextLimits.toolResultMaxChars with per-agent targets.
  • Galin reviewed the PR and found no blocking findings.
  • ClawSweeper rated the prior head diamond lobster with proof sufficient / ready for maintainer look; re-review requested for the refreshed head.

Rebase refresh - 2026-07-01 after #97496

Rebased onto current origin/main after #97496 merged and refreshed the PR head to a9d3dfd9f58d087abb8bc13414ecd44534a1516e.

Fresh validation after rebase:

  • ./node_modules/.bin/oxfmt --check src/flows/doctor-health-contributions.ts src/flows/doctor-health-contributions.test.ts src/flows/doctor-tool-result-cap-advice.ts src/flows/doctor-tool-result-cap-advice.test.ts
  • ./node_modules/.bin/oxlint src/flows/doctor-health-contributions.ts src/flows/doctor-health-contributions.test.ts src/flows/doctor-tool-result-cap-advice.ts src/flows/doctor-tool-result-cap-advice.test.ts
  • node scripts/run-vitest.mjs src/flows/doctor-tool-result-cap-advice.test.ts src/flows/doctor-health-contributions.test.ts (63 tests)
  • node scripts/run-tsgo.mjs -p tsconfig.core.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/core.tsbuildinfo
  • node scripts/run-tsgo.mjs -p test/tsconfig/tsconfig.core.test.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/core-test.tsbuildinfo
  • node scripts/plugin-sdk-surface-report.mjs --check
  • node scripts/sync-plugin-sdk-exports.mjs --check
  • git diff --check

Notes

pnpm check:changed first tried to delegate to Blacksmith Testbox and was blocked by the local Crabbox binary version (0.15.0, requires >=0.22.0), so the prior validation used the documented local child path directly with OPENCLAW_CHECK_CHANGED_REMOTE_CHILD=1.

@giodl73-repo

Copy link
Copy Markdown
Contributor Author

@clawsweeper review

@openclaw-barnacle openclaw-barnacle Bot added the maintainer Maintainer-authored PR label Jun 28, 2026
@clawsweeper

clawsweeper Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper

clawsweeper Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 2, 2026, 12:41 AM ET / 04:41 UTC.

Summary
The branch adds a default-disabled structured Doctor lint check for toolResultMaxChars advice and tests opt-in/default-skip plus inherited per-agent findings.

Reproducibility: not applicable. this is a feature PR rather than a bug report. The changed behavior is checkable with openclaw doctor --lint --json --only core/doctor/tool-result-cap, and the PR body includes isolated CLI output for that path.

Review metrics: 1 noteworthy metric.

  • Structured Doctor check: 1 added, default-disabled. This adds a stable opt-in JSON diagnostic surface, so maintainers should notice the path and target semantics before merge.

Stored data model
Persistent data-model change detected: migration/backfill/repair: apps/ios/README.md, migration/backfill/repair: src/flows/doctor-health-contributions.test.ts, migration/backfill/repair: src/flows/doctor-health-contributions.ts, migration/backfill/repair: src/flows/doctor-tool-result-cap-advice.test.ts, migration/backfill/repair: src/flows/doctor-tool-result-cap-advice.ts, persistent cache schema: .github/workflows/ci.yml, and 22 more. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

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

Risk before merge

  • [P1] Maintainer review still needs to accept the new default-disabled structured Doctor JSON path/target contract before merge.

Maintainer options:

  1. Decide the mitigation before merge
    Land the focused structured check after maintainer review and required checks, preserving legacy Doctor note output and the reviewed opt-in lint semantics.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P2] No narrow automated repair is needed; the protected maintainer label and new operator-facing Doctor JSON contract leave explicit maintainer review as the next action.

Security
Cleared: The live diff changes Doctor TypeScript logic and tests only; no workflow, dependency, secret, install, or package-resolution surface is changed.

Review details

Best possible solution:

Land the focused structured check after maintainer review and required checks, preserving legacy Doctor note output and the reviewed opt-in lint semantics.

Do we have a high-confidence way to reproduce the issue?

Not applicable; this is a feature PR rather than a bug report. The changed behavior is checkable with openclaw doctor --lint --json --only core/doctor/tool-result-cap, and the PR body includes isolated CLI output for that path.

Is this the best way to solve the issue?

Yes; this is the best observed shape because it reuses existing cap advice, model/context-window helpers, agent context-limit inheritance, and default-disabled Doctor lint selection instead of adding a parallel config interpretation.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 8c7ac9b9b9df.

Label changes

Label justifications:

  • P2: This is a bounded Doctor lint improvement with limited runtime blast radius and focused validation.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body and follow-up comments include after-change isolated CLI output showing default skip behavior and explicit structured finding emission.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body and follow-up comments include after-change isolated CLI output showing default skip behavior and explicit structured finding emission.
Evidence reviewed

What I checked:

Likely related people:

  • steipete: Authored and merged the auto-scaled live tool-result cap work and Doctor advice in Auto-scale live tool result caps #86857, which this PR exposes as structured lint. (role: adjacent feature owner; confidence: high; commits: 5b4943353551; files: src/flows/doctor-tool-result-cap-advice.ts, src/flows/doctor-health-contributions.ts, src/agents/embedded-agent-runner/tool-result-truncation.ts)
  • giodl73-repo: Beyond authoring this PR, authored the merged adjacent structured Doctor lint contribution in Doctor: expose channel plugin blocker findings #97496. (role: recent area contributor; confidence: high; commits: cdaafe198cb7; files: src/flows/doctor-health-contributions.ts, src/flows/doctor-health-contributions.test.ts, src/commands/doctor/shared/channel-plugin-blockers.ts)
  • galiniliev: Reviewed this Doctor lint/tool-result cap surface on the PR and reported no blocking findings, making them a likely follow-up reviewer. (role: reviewer; confidence: medium; files: src/flows/doctor-health-contributions.ts, src/flows/doctor-tool-result-cap-advice.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: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. labels Jun 28, 2026
@giodl73-repo

Copy link
Copy Markdown
Contributor Author

@clawsweeper review

Addressed the P2 inherited-default target enumeration finding in 2fe869c. Structured lint now includes agents inheriting defaults caps and has a regression for an agent-specific larger model.

Validation: focused Vitest 57 tests, changed-file oxlint, tsgo:core, local child check:changed, SDK surface, diff check.

@clawsweeper

clawsweeper Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jun 28, 2026
@giodl73-repo

Copy link
Copy Markdown
Contributor Author

@clawsweeper review

Addressed the P2 per-agent path-shape finding in 5f671c8. Explicit per-agent tool-result-cap findings now use agents.list..contextLimits.toolResultMaxChars / target agents.list.; inherited defaults stay on agents.defaults.contextLimits.toolResultMaxChars.

Validation: focused Vitest 57 tests, changed-file oxlint, tsgo:core, local child check:changed, SDK surface, diff check.

@clawsweeper

clawsweeper Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added 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. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. labels Jun 28, 2026
@galiniliev galiniliev self-assigned this Jun 29, 2026

@galiniliev galiniliev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR adds a default-disabled structured core/doctor/tool-result-cap lint check for explicit toolResultMaxChars settings, while keeping the existing human Doctor note path as the normal openclaw doctor behavior. That matters because the new JSON finding surface is now an operator-facing contract for path, target, and fix-hint metadata.

LOC: +366/-47 (4 files)

Findings: No blocking findings.

Bug/behavior: feature PR for Doctor structured lint output; no shipped regression being repaired. The affected surface is Doctor lint/contribution registration and tool-result cap advice formatting.

Best-fix verdict: best. The PR reuses the existing cap calculations and Doctor advice helper instead of adding a parallel interpretation of toolResultMaxChars, and the structured check is opt-in through the existing defaultEnabled: false lint-selection contract.

Alternatives considered:

  • Put the check in doctor-core-checks.ts: rejected because this behavior already belongs to the ordered doctor:tool-result-cap contribution and has a legacy Doctor run path there.
  • Make doctor --lint run it by default: rejected because this is advisory/no-repair guidance and would turn existing explicit caps into default warning exits.
  • Emit only a defaults-level finding: rejected because configured agents can inherit the default cap while resolving different models/context windows, and the current implementation keeps the legacy per-target enumeration.

Code read: src/flows/doctor-health-contributions.ts, src/flows/doctor-tool-result-cap-advice.ts, src/flows/doctor-lint-flow.ts, src/commands/doctor-lint.ts, src/config/schema.help.ts, src/agents/agent-scope-config.ts, src/agents/context-window-guard.ts, src/agents/model-selection.ts, src/agents/model-catalog.ts, src/agents/embedded-agent-runner/tool-result-truncation.ts, and adjacent tests.

Evidence map: changed surface is the Doctor contribution plus cap-advice helper; entry point is openclaw doctor --lint; owner boundary is core Doctor health contributions; caller checked through runDoctorLintCli and runDoctorLintChecks; callees checked through model selection/catalog, context-window resolution, agent context-limit merging, and runtime cap calculation; sibling invariant checked against existing default-disabled Doctor lint checks and config path help; current origin/main only has legacy note output for this contribution.

Validation run:

  • node scripts/run-vitest.mjs run src/flows/doctor-tool-result-cap-advice.test.ts src/flows/doctor-health-contributions.test.ts --reporter=verbose -> 57 passed.
  • Real CLI probe with isolated OPENCLAW_CONFIG_PATH and --only core/doctor/tool-result-cap emitted warning findings for agents.defaults.contextLimits.toolResultMaxChars and inherited target: agents.list.writer; exit 1 was expected because warnings meet the default severity threshold. The probe built stale local dist first and left the worktree clean.

Remaining uncertainty: I did not run the broader changed gate or remote/Testbox proof in this pass; the PR body already reports changed-gate and type/lint proof for this exact head.

@giodl73-repo
giodl73-repo marked this pull request as ready for review July 1, 2026 16:30
@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jul 1, 2026
@giodl73-repo
giodl73-repo force-pushed the doctor-tool-result-cap-lint-findings branch from 5f671c8 to 1033813 Compare July 1, 2026 16:37
@giodl73-repo

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@giodl73-repo
giodl73-repo force-pushed the doctor-tool-result-cap-lint-findings branch from 1033813 to 214ac9c Compare July 1, 2026 23:36
@giodl73-repo
giodl73-repo force-pushed the doctor-tool-result-cap-lint-findings branch from 1347ee5 to 1a533f7 Compare July 2, 2026 04:38
@openclaw-barnacle openclaw-barnacle Bot removed docs Improvements or additions to documentation channel: discord Channel integration: discord channel: imessage Channel integration: imessage channel: mattermost Channel integration: mattermost channel: nostr Channel integration: nostr channel: telegram Channel integration: telegram app: android App: android app: ios App: ios gateway Gateway runtime extensions: memory-core Extension: memory-core cli CLI command changes scripts Repository scripts agents Agent runtime and tooling extensions: nvidia extensions: vercel-ai-gateway extensions: fal extensions: qa-lab extensions: memory-wiki extensions: codex extensions: ollama labels Jul 2, 2026
@giodl73-repo
giodl73-repo merged commit b2c507c into openclaw:main Jul 2, 2026
110 of 113 checks passed
@giodl73-repo

Copy link
Copy Markdown
Contributor Author

Merged via squash.

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

Labels

maintainer Maintainer-authored PR P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. size: M 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.

2 participants