Skip to content

feat(cli): export a redacted diagnostics bundle (bsk diagnostics export) - #356

Open
dangzitou wants to merge 1 commit into
Tencent:mainfrom
dangzitou:feat/diagnostics-export
Open

dangzitou wants to merge 1 commit into
Tencent:mainfrom
dangzitou:feat/diagnostics-export

Conversation

@dangzitou

@dangzitou dangzitou commented Sep 27, 2026 •

Copy link
Copy Markdown

What problem this solves

Fixes #335.

An agent that sees BrowserSkill fail has no supported way to inspect
BrowserSkill's own state: the extension debug page is served under a
chrome-extension:// URL, which the Agent Window sandbox deliberately
refuses to observe, and borrowing the user's debug tab can fail when no
tab can show the confirmation. Recovery then depends on the user
manually reading debug output. This adds a CLI command that exports
what the CLI can see — daemon log tail, daemon status, the bsk doctor
checks, and the active session list — as a single zip an agent can
attach to an issue report.

How this fixes it

bsk diagnostics export [--out <path>] [--log-lines <n>] writes a zip:

  • metadata.json — CLI/daemon versions, platform, timestamp.
  • doctor.json — the checks bsk doctor runs, with repair hints.
  • status.json — daemon info, status and active sessions (null with
    a README note when no daemon is running).
  • daemon-log.txt — trailing log lines (default 500, capped at 4 MiB).
  • README.md — contents, missing parts, redaction note, scope limits.

Redaction is on by default: values of credential keys (token, cookie,
authorization, password, …) become [REDACTED] in both JSON and shell
spellings; credential headers are redacted to end of line so
Authorization: Bearer <token> never survives. Ordinary lines (URLs,
session ids, tab ids) pass through unchanged, and keys embedded in
larger identifiers (secretive, tokens_seen) are not touched. The
one residual gap — a raw credential with no key, e.g. a token inside a
URL — is called out in the README.

Every source is best-effort: a missing daemon or log file is recorded in
the README's "Missing parts" section instead of failing, so a bundle is
always writable. doctor::checks() is extracted so the export reuses
the exact daemon-state resolution bsk doctor runs.

User impact

bsk diagnostics export --out diag.zip

Agents and users get one file to attach when reporting problems, with
credential values removed. Scope is deliberately the CLI-visible
surface; the extension's own debug page remains out of reach by design
and is called out in the README.

Validation

  • cargo fmt --all -- --check, cargo clippy --workspace --all-targets --locked -- -D warnings, cargo test --workspace --locked (green;
    no new tests included in this PR).
  • Local end-to-end runs (macOS): against a live daemon with an active
    session (Chrome for Testing 154 + unpacked extension) —
    status.json contains the session and daemon-log.txt is redacted;
    with no daemon — all optional parts recorded as README notes.

@dangzitou
dangzitou force-pushed the feat/diagnostics-export branch from 88205b7 to a10492d Compare September 27, 2026 18:07
Agents cannot inspect BrowserSkill's own debug console: the extension
debug page is served under a chrome-extension:// URL that the Agent
Window sandbox refuses to observe, so a failed tool call has no
matching local diagnostics. Export what the CLI can see instead.

The bundle contains the daemon log tail, daemon status, the `bsk doctor`
checks and the active session list. Values of credential keys (token,
cookie, authorization, password, …) become `[REDACTED]` by default;
credential headers are redacted to end of line so `Bearer` tokens never
survive. Every source degrades to a README note when the daemon or log
file is absent, so a bundle is always writable.

doctor::checks() exposes the check list without rendering so the export
reuses the same daemon-state resolution `bsk doctor` does.
@dangzitou
dangzitou force-pushed the feat/diagnostics-export branch from a10492d to 658fd4e Compare September 27, 2026 18:13
@iuyo5678

Copy link
Copy Markdown
Collaborator

Thanks for adding this. A diagnostics bundle would make BrowserSkill failures much easier for users and agents to investigate, especially when the extension is disconnected. The CLI-visible scope is a useful first step,I like this PR.

The patch applies to main a358e75, builds successfully, and passes the 22 existing doctor tests. However, targeted checks reproduced several issues that should be addressed before merging.

1. Make diagnostics collection passive

diagnostics::collect() currently calls doctor::checks(), which inherits behavior beyond inspection:

  • resolve_daemon_state() can automatically start a missing daemon.
  • check_skill_up_to_date() synchronizes installed managed skills. This still happens with BSK_AUTO_START=0.
  • The home-directory check can create directories or change permissions.

Exporting diagnostics should preserve the failure state. Please separate passive collection and check evaluation from startup, synchronization, and repair behavior. A missing daemon should be recorded as missing, without starting it. Likewise, installed skills should remain unchanged.

Please reuse the check logic where possible, while keeping the existing doctor behavior intact.

2. Fix redaction and apply it to every exported file

The current log redactor misses common credential formats. These examples retain secrets:

{"token": "DEMO_SECRET"}
{"authorization": "Bearer DEMO_SECRET"}
token='DEMO_SECRET'
Cookie: a=DEMO_A; b=DEMO_B

The Cookie example only redacts the first value. Escaped quotes also cause partial redaction. Conversely, no token supplied; status=unhealthy incorrectly redacts the status because the parser searches too far ahead for a separator.

There is also a separate coverage gap: only daemon-log.txt passes through redaction. JSON files and dynamic README notes are written directly. For example, a synthetic update error containing token=DEMO_SECRET survives in doctor.json.

Please establish one sanitization boundary for all exported content. Structured JSON should be sanitized structurally, dynamic strings should receive text sanitization, and credential headers should have their complete values removed. The extension’s existing redaction implementation can inform the policy and shared test cases.

3. Keep the diagnostics command and document its distinct scope

Please retain bsk diagnostics export. Existing bsk debug commands investigate website/task evidence and already have their own export flow. Diagnostics should remain usable without an active session or connected extension.

For consistency with existing exports, please use:

bsk diagnostics export --output diagnostics.zip

Please add docs/diagnostics.md, link it from the README and website-debugging guide, and add concise agent-facing instructions to the bundled skill’s references/environment.md, with an appropriate entry point in SKILL.md.

The documentation should explain collection scope, offline behavior, missing sources, redaction limits, and that exporting does not upload anything. Please explicitly state that the current bundle includes daemon-wide session summaries and logs; adding session filtering is not required for this PR.

4. Make output handling and size limits reliable

File::create() currently truncates an existing destination. Please use temporary-file completion and refuse overwriting an existing file by default, following the existing bsk debug export pattern.

The advertised 4 MiB log limit is also exceeded: a test with long lines exported 4,259,839 bytes. Please enforce the documented limit on the final exported log, handle partial leading lines safely, and report truncation. Missing or failed sources should remain explicit in the bundle.

5. Add focused regression tests and clarify issue coverage

Please cover the redaction cases above, secrets in doctor/error output, export without a daemon, unchanged installed skills, missing/unreadable sources, oversized logs, and existing output files. Since the original report is from Windows, please include relevant Windows validation.

The current CLI/daemon bundle is useful, but it does not provide all extension-side diagnostics described in #335. Please clarify that this is a partial resolution and adjust the closing reference accordingly, keeping any remaining extension-side work tracked separately. There is no need to expand browser-page permissions for this PR.

Once passive collection, bundle-wide redaction, reliable file handling, and the corresponding tests are in place, this would be a valuable addition to merge.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

LLM agents cannot inspect the BrowserSkill debug console

2 participants