Skip to content

fix(statusline): link PRs from tool.call so cc-plugin-sec-default can't skip it - #16

Merged
AnnatarHe merged 1 commit into
mainfrom
claude/fervent-keller-vl406t
Oct 21, 2026
Merged

AnnatarHe merged 1 commit into
mainfrom
claude/fervent-keller-vl406t

Conversation

@AnnatarHe

@AnnatarHe AnnatarHe commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Problem

On Team and Enterprise plans, Claude Code reports:

shelltime-statusline: its classic.PostToolUse hook was skipped. "cc-plugin-sec-default" runs first and sent the event on past it.

cc-plugin-sec-default (sec-default@builtin) is built into the Claude Code binary. It puts itself in the outermost prepend tier on Team/Enterprise orgs and on machines with managed settings. Its hooks module answers every classic.* event with next.to(e, "append"), which skips the whole user tier, where installed mods sit. A user plugin can't get around this: next.to never lets a lower tier skip a higher one, and only managed settings (prependPlugins) can reorder plugins.

So the mod's classic.PostToolUse hook never ran. PRs opened with gh pr create were never linked to the session, and the GitHub App never posted a cost comment. The statusline itself was not affected.

Fix

  • hooks/register.tsx: PR linking moves to a tool.call hook on { tool: 'Bash' }, which sec-default doesn't hook.
    • It awaits next(e) and reads e.command and ran.result.stdout.
    • Linking runs in the background, and the result is returned unchanged.
    • Only successful, unrefused calls count, matching PostToolUse, which never fired on a failed call.
    • The session id comes from $.session.id().
  • hooks/pullRequests.ts: createdPullRequestUrls(command, stdout). The matcher already limits it to Bash, so the tool-name check and loose field reads are gone.
  • hooks/pullRequests.test.ts: the tests now drive $.tool.call with a Bash stub beneath. New cases check that the Bash result reaches the model unchanged, and that a failed or refused gh pr create links nothing. git runs from the statusline refresh are left out of the recorded runs.
  • README: says which hook does the linking and why it isn't PostToolUse.

Testing

  • claude plugin test plugins/shelltime-statusline: 34 pass, 0 fail.
  • Mutation check: with the matcher changed so the hook never fires, the 3 linking tests fail.
  • claude plugin validate --strict plugins/shelltime-statusline and claude plugin validate --strict .: pass. The report lists tool.call{tool=Bash} and no classic.PostToolUse.
  • tsc against the engine's 2.1.306 API types: clean for the changed files. The one error left is in sessionEnd.test.ts (an http.fetch stub typed unknown), which this PR doesn't touch.
  • pnpm typecheck / pnpm test were not run locally because the container's pnpm is a placeholder binary. The UI specs and stories don't import the changed files, and CI's ui job runs them.

Generated by Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

…'t skip it

On Team and Enterprise plans Claude Code's built-in cc-plugin-sec-default
seats itself in the outermost (prepend) tier and answers every classic.*
event with next.to(e, "append"), skipping the user tier where installed
mods sit. The mod's classic.PostToolUse hook therefore never ran there,
and PRs opened with gh pr create were never linked to the session.

Read the Bash call from a tool.call hook instead, which sec-default does
not hook. Only successful, unrefused calls count, as PostToolUse did, and
the session id now comes from $.session.id().

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W1cER89rF43oNjGycbv9WL
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-10-21T06:50:15.681003Z
We couldn’t complete this review. Comment @codex review to try again.
7950d77 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@AnnatarHe

Copy link
Copy Markdown
Contributor Author

/claude review

@AnnatarHe

Copy link
Copy Markdown
Contributor Author

@claude review

@claude

claude Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Claude finished @AnnatarHe's task in 19s —— View job


Review of PR #16

Verdict: looks good, no blocking issues.

I read the full diff and checked that logOnce, linkPullRequests and $.session.id() exist in register.tsx with the signatures the new hook uses. I didn't run the tests or typecheck.

What I checked

  • Moving to tool.call is the right fix. It uses a hook that cc-plugin-sec-default doesn't touch, and the README explains why it isn't PostToolUse.
  • Result handling is correct. The hook awaits next(e), links in the background, and returns ran unchanged. The "result reaches the model unchanged" test covers this.
  • Failed and refused calls are skipped. The check ran.deny === undefined && ran.isError !== true matches the old PostToolUse behavior. Short-circuiting also means ran.result.stdout is only read when result is an object. The new failed/refused test covers it.
  • Errors can't escape. The session.id() rejection is caught and logged. linkPullRequests has its own catch (it calls logOnce).
  • Simplifying createdPullRequestUrls is safe. The { tool: 'Bash' } matcher makes the tool-name check and the loose field reads redundant. The git stub in the tests is needed so the statusline's refresh doesn't pollute the recorded runs.

Minor, optional

  • ran.result.stdout assumes a successful Bash result is always an object. If the engine ever returns a plain string result on success, .stdout would be undefined and extractPullRequestUrls would throw on .match. A typeof stdout === 'string' guard would make that case safe. The types you tested against presumably rule it out, so this is only defensive.
  • ran.deny and ran.isError have to be read as a union, so confirm the 2.1.306 types make that narrowing sound. You said tsc was clean on the changed files, so this should already be fine.
  • pnpm typecheck and pnpm test weren't run locally, as the description says. Check that CI's ui job is green before merging.

@AnnatarHe
AnnatarHe merged commit 28abb1f into main Oct 21, 2026
3 checks passed
@AnnatarHe
AnnatarHe deleted the claude/fervent-keller-vl406t branch October 21, 2026 07:53
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.

2 participants