onboarding: add the contract gate, and make the agent ask which repos - #59
Conversation
There was a problem hiding this comment.
🟡 Not ready to approve
A few documentation instructions as written are ambiguous/brittle (notably hook matcher escaping, AGY-vs-Claude “stop and ask” wording, and non-idempotent PATH symlink), which can cause onboarding misconfiguration.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Adds a new “contract gate” onboarding step that wires Claude Code hooks and documents the required “ask-the-user” repo selection flow (no default gated repo list), plus corresponding maintenance rationale and smoke-test verification guidance.
Changes:
- Add ONBOARDING step 13 describing the Claude Code contract gate, including hook wiring, the required repo multi-select ask, enable/disable guidance, and PATH symlink option.
- Extend the verification smoke test to check contract-gate behavior in enabled vs non-enabled repos.
- Update MAINTAINING artifact map/decisions/deferred sections to document the contract gate and the “ask-don’t-default” rationale, plus the AGY port being deferred.
File summaries
| File | Description |
|---|---|
| onboarding/ONBOARDING.md | Adds step 13 for the contract gate (Claude Code only) and a smoke-test check covering enabled vs non-enabled repos. |
| onboarding/MAINTAINING.md | Documents the contract gate artifact, the decision to ask (no default repo list), and defers the AGY port. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 3
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| | Event | Matcher | Command | | ||
| |---|---|---| | ||
| | `PreToolUse` | `Edit\|Write\|NotebookEdit` | `~/.claude/skills/contract/hooks/contract-gate.sh` | | ||
| | `SessionStart` | *(none)* | `~/.claude/skills/contract/hooks/contract-inject.sh` | |
| > **Agent running this onboarding: stop here and ask.** Use `AskUserQuestion`, multi-select, | ||
| > offering the repos the member has actually cloned as options. Enabling a repo changes how | ||
| > every future session behaves in it, so the list is the member's call, not yours — and a list | ||
| > you guessed is one they will disable in a week. Recommend starting with two or three where a | ||
| > misread request costs the most to unwind. |
| **c. Put the CLI on your PATH** — optional, for typing `contract new <slug>` by hand. The hooks | ||
| call it by absolute path, so nothing breaks without this: | ||
| ```bash | ||
| ln -s ~/.claude/skills/contract/bin/contract ~/.local/bin/contract |
New ONBOARDING step 13 covers the contract skill: wire the two hooks, enable repos, put the CLI on PATH, expand on skip-log evidence. The substantive part is (b): the agent walking a member through onboarding must stop and ask which repos to gate, via AskUserQuestion over the repos they actually cloned. Enabling changes how every future session behaves in that repo, so the list is the member's call -- and a guessed list is one they disable in a week. No default list ships. Added as step 13 rather than inserted near the other skills steps so the existing cross-references to steps 5/6/7/11/12 and section D keep resolving. It also has to follow step 8, since there is nothing to enable before a repo is cloned. Marked Claude Code only. The gate returns CC's permissionDecision deny JSON from a PreToolUse hook in ~/.claude/settings.json, and this onboarding is first-pass Antigravity, which manages hooks under ~/.gemini/config/. Follows the precedent already set by the statusline in step 5: document it, mark it unported, track the port in MAINTAINING. MAINTAINING gains three rows: the artifact map entry, the decision record for asking-not-defaulting (and why this bends the "settings.json left literal" rule the same way herdr's hook does), and the AGY port under deferred. Verified: every ~/.claude/skills path the step cites exists, the skip-log filename matches what bin/contract writes, and the repo's own leak scan is clean on the added lines.
bec085c to
031267e
Compare
Adds the
contractstart gate to the onboarding kit.The ask
That's step 13(b), and it's the load-bearing part. The onboarding agent must stop and ask —
AskUserQuestion, multi-select, over the repos the member actually cloned. No default repo list ships. Enabling a repo changes how every future session behaves in it, so the list is the member's call; a list we guessed is one they disable in a week.Also in step 13
/hooks, both fail open — with no repo enabled they exit 0 and change nothing)contract enableper chosen repo, noting it is keyed on the shared git dir so one call covers every worktreecontract disablewhere skips dominatePlus a smoke-test check in section C, and three
MAINTAINING.mdrows: artifact map, the ask-don't-default decision, and the AGY port under deferred.Two judgment calls worth reviewing
1. Marked Claude Code only. This onboarding is first-pass Antigravity — skills live in
~/.gemini/antigravity-cli/skills/, hooks under~/.gemini/config/. The contract gate returns Claude Code'spermissionDecision: denyJSON from aPreToolUsehook in~/.claude/settings.json, so on an AGY setup it installs nothing.Rather than ship a dead step, I followed the precedent already in the doc — step 5's statusline note ("patches Claude Code's
settings.jsonand does not apply to Antigravity. Skip it… porting tracked in MAINTAINING"). Documented, banner-marked unported, port row added.bin/contractand the note format are agent-agnostic; only the hook shim needs porting.2. Numbered 13, not inserted near the skills steps. Inserting at 6 would renumber 6→12 and break the in-text references to steps 5/6/7/11/12 and section D. It also genuinely has to follow step 8 — there's nothing to enable before a repo is cloned.
It does bend one existing rule, deliberately, and I recorded why: MAINTAINING says
~/.claude/settings.jsonis "left literal, except.statusLine" because its hooks are PI-personal infra. The contract hooks are not — they point at~/.claude/skills/...and carry nothing personal — so this follows the herdr precedent instead (member runs an installer against their own file).Verification
~/.claude/skills/...path cited in the step existscontract-skips.logmatches whatbin/contractactually writes (SKILLS:29)enable, deny after,Bashnever gated