Uh oh!
There was an error while loading. Please reload this page.
Ship shared plugin hooks - #550
Conversation
a480e3e to
6bc3e87Compare6bc3e87 to
9c1e036CompareThere 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.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
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".
Uh oh!
There was an error while loading. Please reload this page.
jeremy
commented
Aug 1, 2026
@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". |
jeremy
commented
Aug 1, 2026
Landing sequence — read before marking this readyThis PR is now a draft so the gate is enforced rather than implied. The hooks call The order matters, and step 4 is the one thats easy to miss:
On (4): merging the hooks after v0.8.0 while leaving the plugin manifest at The wrapper alternative raised in the P1 thread was considered and rejected on purpose: an old CLI writes its error to stdout (exit 7), so a |
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`.
…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.
db3aad5 to
7e605ddComparejeremy
commented
Aug 3, 2026
@codex review Rebased onto |
Codex Review: Didn't find any major issues. Hooray! 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) + docsMerge gate cleared — release N shipped as stable
v0.8.0on 2026-08-03, and it is Latest.This PR was held because a plugin installed from the default branch would have called
agent-hook, a command the then-publishedv0.7.2did not have. Verified against the shipped artifact rather than the source tree, since that is what users actually download:basecamp_0.8.0_darwin_arm64.tar.gzreportsbasecamp version 0.8.0and exposes exactly the three subcommandshooks/hooks.jsoninvokes (session-start,pre-commit-snapshot,post-commit). The full flow was then exercised end to end against that binary: snapshot → commitBC-4455→ nudge emitted with the reference and short hash.Rebased onto
main(was 45 commits behind);bin/ciexit 0.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.jsonto add session-start status and Git commit-to-todo nudges. Widens hook timeouts and updates docs to require a CLI withagent-hook, plus trust-first guidance in Codex.New Features
hooks/hooks.jsonfor both agents:SessionStartinjects auth status; BashPreToolUse/PostToolUsedrive commit nudges;PostToolUseFailure(Claude) also nudges after a failed push; timeouts set to 5s/10s/12s.basecamp agent-hook <sub>across shells and stay silent on Bash. Codex ignores unknown hook event names and rejects unknown top-level keys.agent-hookCLI requirement; Codex setup now says “trust hooks with/hooks” before “start a new thread”;.codex-plugin/plugin.jsonmentions commit linking;AGENTS.mdlistshooks/.Migration
basecampCLI withagent-hook(release N+). If hook errors appear after install/refresh, runbasecamp upgrade, start a new session, and check withbasecamp agent-hook --help(“unknown command” means the CLI is too old)./hooks, then start a new thread. Claude needs no trust step.Written for commit 7e605dd. Summary will update on new commits.