Ship shared plugin hooks (merge after release N) - #550
Conversation
github-actions
Bot
added
commands
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
a480e3e to
6bc3e87
Compare
One hooks/hooks.json at the plugin root serves both agents: SessionStart reports Basecamp auth status as model context, and the paired PreToolUse/PostToolUse Bash hooks drive the HEAD-snapshot commit nudge via the hidden agent-hook command. PostToolUseFailure routes to the same post-commit check so a `git commit && git push` with a failed push still nudges on Claude; Codex ignores unknown event names inside hooks, and its PostToolUse already fires regardless of exit status. No other top-level keys — Codex hard-errors on them. The tool hooks carry no statusMessage: they run on every Bash call and stay visually silent. Commands are plain `basecamp agent-hook <sub>` strings, valid under all four hook shells (sh -lc, cmd.exe /C, Git Bash, PowerShell) with no wrapper or per-agent split. The dormant TestHooksFileCommandsInvokeBasecamp gate activates with this file. Restore the hooks language deferred in cb13972: README and install.md regain hooks in the plugin feature lists and the Codex /hooks trust step (Codex lists untrusted hooks but does not run them until trusted; Claude's install consent covers it), the AGENTS.md tree regains hooks/, the Codex manifest regains the connect-commits clause, and setup codex again points at /hooks. Hooks require a CLI release containing agent-hook. On an older CLI a refreshed plugin payload produces non-blocking unknown-command hook errors on either agent; remediation is `basecamp upgrade`.
6bc3e87 to
9c1e036
Compare
There was a problem hiding this comment.
Pull request overview
This PR ships a shared hooks/hooks.json at the plugin root so both Claude Code and Codex can run the same hook configuration, enabling session-start status context and Git commit-reference nudges via basecamp agent-hook …. It also restores/updates setup guidance and docs so users can install the plugins and (for Codex) trust hooks via /hooks.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Changes:
- Add shared agent hook configuration in
hooks/hooks.json(SessionStart + Bash PreToolUse/PostToolUse/PostToolUseFailure). - Update Codex setup wizard output + tests to include
/hookstrust guidance before “start a new thread”. - Refresh documentation/manifests to mention hooks and commit-to-todo linking.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates AI agent integration section to include hooks and Codex /hooks trust guidance. |
| internal/commands/wizard_codex.go | Prints Codex post-setup next steps including trusting hooks before starting a new thread. |
| internal/commands/wizard_codex_test.go | Asserts /hooks trust guidance is present and ordered before “start a new thread”. |
| install.md | Restores hooks language and adds Codex /hooks trust guidance in install steps. |
| hooks/hooks.json | Introduces shared hook definitions invoking basecamp agent-hook … for session context and commit nudges. |
| AGENTS.md | Adds hooks/ to the repository tree overview. |
| .codex-plugin/plugin.json | Updates the Codex plugin longDescription to include commit-to-todo linking. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c1e036141
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "hooks": [ | ||
| { | ||
| "type": "command", | ||
| "command": "basecamp agent-hook session-start", |
There was a problem hiding this comment.
If this lands while v0.7.2 is still the published CLI, this hook will fail for users who install the plugin from the marketplace and then trust hooks: the marketplace entry points at basecamp/basecamp-cli's default branch, but the latest v0.7.2 release (f087e6e, checked July 22, 2026) does not contain the hidden agent-hook command, which was added later in 340b502. Please either hold this until a CLI release that includes agent-hook is available, or wrap the hook command with a command/version-exists fallback so stable CLI installs stay silent instead of reporting hook failures.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Flagged for human review — the finding is correct, but both remedies are maintainer calls, so leaving this unresolved.
Option 1 (hold) is already in force. The PR title gates it — "merge after release N" — and release N is independently held on the locked-keychain acceptance test. So the failure mode described here is not currently reachable by merging.
Option 2 (wrapper) needs a decision I should not make unilaterally, and it is not the one-liner it looks like. An old CLI writes its error to stdout, not stderr, and exits 7:
$ basecamp not-a-real-command 2>/dev/null
{ "ok": false, "error": "unknown command ...", "code": "api_error" }
So ... 2>/dev/null || true would not stay silent — it would push a JSON error blob onto stdout, which for SessionStart is exactly the channel the hook uses to inject context. A correct wrapper has to guard before running, e.g.
basecamp agent-hook --help >/dev/null 2>&1 && basecamp agent-hook <sub> || true
which costs an extra process spawn on every hook fire — including PreToolUse on every Bash call. That hot-path cost is the trade-off worth a maintainer opinion. (Shell syntax itself is fine here: TestHooksFileCommandsInvokeBasecamp already accepts an exec basecamp form, and exec is a shell builtin.)
Worth noting the underlying exposure outlives release N: anyone still on an older CLI who refreshes the plugin hits this regardless of when it merges. That is now addressed on the documentation side — README and install.md name the CLI floor, the remediation (basecamp upgrade), and the check (basecamp agent-hook --help replying "unknown command"), which covers the "no clear remediation" half of this comment.
…I floor The hook timeouts were set below what the commands can actually take. gitOutput gives every git invocation its own 2s deadline, and the two git-touching subcommands make several in sequence: pre-commit-snapshot agentHookHead up to 4 calls ~8s post-commit agentHookCommitReference up to 5 calls ~10s Both sat under a 5s outer timeout, so on a slow disk or filesystem the hook could be killed mid-run — dropping the snapshot, and with it the nudge that depends on it. Raised to 10s and 12s respectively, covering the worst case with margin. PostToolUseFailure runs the same post-commit command and gets the same 12s. SessionStart makes no git calls and stays at 5s. The docs now state the CLI floor the hooks imply. They call `agent-hook`, which older installs do not have, and the failure mode gave no hint: hook errors after a plugin refresh, with nothing pointing at the CLI version. README and install.md now name the requirement, give the remediation (`basecamp upgrade`, then a new session), and name the check — `basecamp agent-hook --help` answers "unknown command" on a CLI too old, which is exactly what an install in that state reports.
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
feat: ship shared plugin hooks (
hooks/hooks.json) + docsDO NOT MERGE until release N (the first CLI release containing
agent-hook) has shipped. Review now; merge after. From that moment, any refreshed plugin payload references a command that exists in the released CLI.What
One
hooks/hooks.jsonat the plugin root serves both agents (both read the same location, same matcher semantics, samehookSpecificOutputwire):startup|resume|clear|compact) →basecamp agent-hook session-start— auth status as model context, with a spinner label.Bash) →pre-commit-snapshot/post-commit— the paired HEAD-snapshot commit nudge. NostatusMessageon these: they run on every Bash call and stay visually silent.Bash) →post-commit— closes the old accepted asymmetry: on Claude,git commit && git pushwith a failed push still nudges (the commit is proven from repo state, not the tool result). Codex ignores unknown event names insidehooks(verified:HookEventsTomllacksdeny_unknown_fields), so this key is inert there; Codex's own PostToolUse fires regardless of exit status. No other top-level keys — Codex hard-errors on unknown top-level keys in hooks.json.Commands are plain
basecamp agent-hook <sub>strings — valid under all four hook shells (sh -lc,cmd.exe /C, Git Bash, PowerShell); no wrapper, nocommandWindows, no per-agent split.Docs/setup restoration (reverses the Phase 1 deferrals from
cb139726):/hooks" (Codex lists untrusted hooks but silently does not run them until trusted). Claude needs no trust step — install consent covers it.runCodexSetup: re-adds the muted "Review and trust the plugin hooks with /hooks." next-step line; the wizard test flips from proving/hooksabsent to proving it present.hooks/entry back (agent-neutral wording)..codex-plugin/plugin.jsonlongDescription: restores the connect-commits clause.TestHooksFileCommandsInvokeBasecamp(dormant since #534) activates automatically now thathooks/hooks.jsonexists.Compatibility floor
Hooks require CLI ≥ release N. Users of either agent with an older CLI may see non-blocking hook errors after their plugin payload refreshes (Codex: on
marketplace upgrade; Claude: at the release N+1 manifest version bump, which can auto-update the plugin while the user still runs CLI < N). Remediation:basecamp upgrade. Codex additionally keeps new hooks inert until/hookstrust.Rollout
codex plugin marketplace upgrade 37signals(orbasecamp setup codex), then must trust via/hooks.Staging verification (pre-merge, candidate payload + dev-build CLI)
Both agents were verified against this branch's SHA (
4f28a810) with production plugins isolated first (Codex: throwawayCODEX_HOME+ staging marketplace37signals-stagingpinned to the SHA, install positively attributed; Claude: productionbasecamp@37signalsdisabled, candidate loaded via--plugin-dir), then production state restored.codex exec --dangerously-bypass-hook-trust, dev-buildbasecampon PATH): SessionStart context injected verbatim ("Basecamp is active and OAuth is ready…"); scratch repo commit withBC-123→ nudge delivered as developer context with the short hash; failed commit (nothing staged) → NONE./hookstrust flow itself not exercisable non-interactively — the bypass flag exercises the same execution path post-trust.PostToolUsenudge ontodo-456commit quoted back by the model;PostToolUseFailureon commit-then-failed-push quoted theBC-777nudge verbatim — the asymmetry this event closes, verified live. SessionStart verified at the transcript level: hook executed, stdout parsed, context attached ashook_additional_context(models don't always attribute it when asked, but injection is proven).TestHooksFileCommandsInvokeBasecampnow runs (not skipped) and passes; fullbin/cigreen on this branch.Summary by cubic
Ships shared plugin hooks for Codex and Claude via
hooks/hooks.json, adding session-start context and Git commit-to-todo nudges. Widens hook timeouts to avoid premature exits and updates docs to state the CLI floor foragent-hook.New Features
hooks/hooks.jsonfor both agents:SessionStartadds auth status; BashPreToolUse/PostToolUsepower commit nudges;PostToolUseFailureon Claude also nudges after a failed push; Bash hooks run silently.basecamp agent-hook <sub>and work across shells; Codex ignores unknown hook event names; no unknown top-level keys..codex-plugin/plugin.jsonlongDescription mentions commit linking;AGENTS.mdlistshooks/; tests enforce trust-before-thread order.SessionStart5s;pre-commit-snapshot10s;post-commit12s (including failure path) to cover git calls.Migration
agent-hook(release N+). If hooks error after install/refresh, runbasecamp upgrade, start a new session, and verify withbasecamp agent-hook --help(an "unknown command" reply means the CLI is too old)./hooks, then start a new thread. Claude needs no trust step.Written for commit db3aad5. Summary will update on new commits.