Skip to content

fix(doctor): emit tool-policy profile audit at debug level#87865

Open
Kailigithub wants to merge 1 commit into
openclaw:mainfrom
Kailigithub:fix/doctor-tool-policy-audit-level
Open

fix(doctor): emit tool-policy profile audit at debug level#87865
Kailigithub wants to merge 1 commit into
openclaw:mainfrom
Kailigithub:fix/doctor-tool-policy-audit-level

Conversation

@Kailigithub

Copy link
Copy Markdown

Summary

openclaw doctor emits repeated audit lines at info/console level for expected tool-profile filtering:

[agents/tool-policy] tool policy removed 5 tool(s) via tools.profile (coding): agents_list, gateway, message, nodes, tts

These are expected and non-actionable — the tools are correctly removed by the profile filter. Emitting them at info level makes normal doctor output look noisy.

Root Cause

auditToolPolicyFilter in tool-policy-audit.ts always uses info level regardless of step type. Profile-filter steps (whose stepLabel starts with tools.profile) are routine housekeeping, not doctor findings.

Fix

When stepLabel starts with tools.profile, emit the audit log at debug level instead of info. This preserves the diagnostic path for verbose/debug logging while cleaning up normal doctor output.

All other audit paths (agents.allow, tools.deny, sandbox policies, etc.) continue to log at info level unchanged.

Verification

  • TypeScript compiles without errors
  • Logic change is isolated to a single conditional in auditToolPolicyFilter
  • No behavioral change for non-profile audit paths
  • No changes to function signatures or public APIs

Closes #87798

When tools.profile is set to a named profile (e.g. "coding"), the tool-policy
 pipeline removes tools that don't belong to that profile. The resulting audit
 lines ("tool policy removed N tool(s) via tools.profile (coding): ...") are
 expected behavior, not actionable doctor findings.

 These lines were emitted at info level, causing normal doctor output to look
 noisy when a profile filter is active. Now they are emitted at debug level so
 they appear in verbose/debug logs but not in regular doctor output.

 The fix is isolated to auditToolPolicyFilter in tool-policy-audit.ts. Only
 steps whose label starts with "tools.profile" are affected; other audit paths
 (agents.allow, tools.deny, etc.) continue to log at info level.
@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: XS triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 29, 2026
@clawsweeper

clawsweeper Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed May 28, 2026, 11:21 PM ET / 03:21 UTC.

Summary
This PR changes auditToolPolicyFilter so removed-tool audits for tools.profile* steps use debug logging while other policy-removal audits continue using info logging.

PR surface: Source +2. Total +2 across 1 file.

Reproducibility: yes. from source inspection: current main routes profile-filter tool removals through an info-level audit call, and the pipeline labels profile filtering as tools.profile (...). I did not run openclaw doctor because this review had to keep the checkout read-only.

Review metrics: none identified.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦐 gold shrimp
Result: blocked until real behavior proof is added.

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

Rank-up moves:

  • [P1] Add a debug logger mock and focused test expectation for tools.profile audit removals.
  • [P1] Post redacted after-fix openclaw doctor terminal output, screenshot, or logs showing normal output no longer includes the repeated audit line.

Proof guidance:

  • [P1] Needs real behavior proof before merge: No after-fix real behavior proof is present; the contributor should add redacted terminal output, a terminal screenshot, copied live output, or logs from a real openclaw doctor run, and updating the PR body should trigger a fresh ClawSweeper review or a maintainer can comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] The external PR has no after-fix real openclaw doctor proof, so the user-visible console cleanup has not been demonstrated in a real setup.
  • [P1] The changed debug path is not covered by the existing test mock, so focused tool-policy tests can fail or miss the new behavior until the mock and expectations are updated.

Maintainer options:

  1. Decide the mitigation before merge
    Keep the narrow log-level approach, add focused regression coverage for profile-filter debug audits, and require a redacted real openclaw doctor proof before merge.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] This external PR needs contributor proof and a small test/mock update before normal maintainer review can merge it; ClawSweeper repair is not the right lane while proof is missing.

Security
Cleared: The diff only changes a log level in existing tool-policy audit code and does not add dependency, workflow, credential, permission, install, or execution-path changes.

Review findings

  • [P2] Update the tool-policy logger mock for debug audits — src/agents/tool-policy-audit.ts:181
Review details

Best possible solution:

Keep the narrow log-level approach, add focused regression coverage for profile-filter debug audits, and require a redacted real openclaw doctor proof before merge.

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

Yes from source inspection: current main routes profile-filter tool removals through an info-level audit call, and the pipeline labels profile filtering as tools.profile (...). I did not run openclaw doctor because this review had to keep the checkout read-only.

Is this the best way to solve the issue?

No, not merge-ready yet: the runtime change is a narrow fit, but the PR does not update the focused test mock/expectations and does not include real doctor-output proof. The safer path is to keep the code change small while proving the debug path and normal doctor output.

Full review comments:

  • [P2] Update the tool-policy logger mock for debug audits — src/agents/tool-policy-audit.ts:181
    This change makes profile-filter removals call toolPolicyAuditLogger.debug, but the existing tool-policy-pipeline test mock still returns only { info }. The unchanged warning-cache test exercises a tools.profile (coding) removal, so that path will call undefined(...) instead of proving the new debug behavior. Add a debug mock and a focused expectation for profile audits while keeping the non-profile info assertions.
    Confidence: 0.86

Overall correctness: patch is incorrect
Overall confidence: 0.86

AGENTS.md: found and applied where relevant.

Codex review notes: reasoning high; reviewed against 5a84869c061d.

Label changes

Label justifications:

  • P2: This is a normal-priority user-visible doctor diagnostic fix with a narrow agents/tool-policy surface and limited blast radius.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: No after-fix real behavior proof is present; the contributor should add redacted terminal output, a terminal screenshot, copied live output, or logs from a real openclaw doctor run, and updating the PR body should trigger a fresh ClawSweeper review or a maintainer can comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +2. Total +2 across 1 file.

View PR surface stats
Area Files Added Removed Net
Source 1 3 1 +2
Tests 0 0 0 0
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 1 3 1 +2

What I checked:

  • Current behavior: Current main still emits removed-tool policy audit entries through toolPolicyAuditLogger.info, matching the linked doctor-console noise report. (src/agents/tool-policy-audit.ts:179, 5a84869c061d)
  • Profile audit path: The shared tool-policy pipeline labels profile steps as tools.profile (...) and invokes auditToolPolicyFilter after filtering each expanded policy step. (src/agents/tool-policy-pipeline.ts:64, 5a84869c061d)
  • PR diff: The PR head changes the audit call to choose debug for labels starting with tools.profile and info otherwise. (src/agents/tool-policy-audit.ts:181, 8035b8645aa6)
  • Test gap: The PR head leaves the existing logger mock with only info, while the test file still has a tools.profile (coding) removal path that would now call debug. (src/agents/tool-policy-pipeline.test.ts:14, 8035b8645aa6)
  • Logger contract: The runtime subsystem logger includes both debug and info, so the production logging API supports the proposed level change. (src/logging/subsystem.ts:19, 5a84869c061d)
  • Related issue context: The linked issue already had a ClawSweeper source review keeping it open as a narrow P2 doctor diagnostic bug with no live doctor proof yet.

Likely related people:

  • steipete: Current blame in the checked-out source points the audit function and nearby pipeline code at Peter Steinberger's recent release-lane commit, and the related issue's prior review identifies Peter's earlier shared pipeline work as relevant history. (role: introduced shared pipeline and current-line owner signal; confidence: medium; commits: c0094a232d00, f97ad8f288d2; files: src/agents/tool-policy-audit.ts, src/agents/tool-policy-pipeline.ts)
  • vincentkoc: The related issue's prior review identifies recent profile allowlist-warning suppression work in the same tool-policy pipeline as adjacent to this audit-level behavior. (role: recent adjacent area contributor; confidence: medium; commits: 53504b366201; files: src/agents/tool-policy-pipeline.ts, src/agents/tool-policy-pipeline.test.ts)
  • Nyanako: The related issue's prior review identifies adjacent gated core tool/profile warning behavior changes in the same pipeline, making this a useful secondary routing signal. (role: adjacent warning behavior contributor; confidence: medium; commits: d72cc7a380e5; files: src/agents/tool-policy-pipeline.ts, src/agents/tool-policy-pipeline.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 rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. labels May 29, 2026
@clawsweeper

clawsweeper Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

@Kailigithub thanks for the PR. ClawSweeper is still waiting on real behavior proof before this can move forward.

Useful proof can be a screenshot, short video, terminal output, copied live output, linked artifact, or redacted logs that show the changed behavior after the fix. Please redact private tokens, phone numbers, private endpoints, customer data, and anything else sensitive.

Once proof is added to the PR body or a comment, ClawSweeper or a maintainer can re-check it.

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. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: doctor emits repeated tool-policy removal audit lines at normal console level

2 participants