Repository navigation
fix(install): restore /plannotator-* bash execution on Claude Code + harden unattended installs - #872
Merged
Merged
Conversation
…harden unattended installs Follow-up to #850 (unreleased). Pre-release review surfaced two regressions vs v0.19.27; both fixes live in the installer scripts and skill sources. Claude Code commands were demoted to prose - #850 pointed Claude Code at the shared prose core skills, so the model now launches `plannotator` via a Bash tool call: a permission prompt on first run and lost `$ARGUMENTS` passthrough, vs the old slash commands. - Claude Code is skills-only now, but skills support the same dynamic-context injection the commands used. Add authoritative apps/skills/claude/* skills that use `!`plannotator <sub> $ARGUMENTS`` + allowed-tools + disable-model-invocation, so /plannotator-review|annotate|last|archive auto-run with no prompt and pass arguments through — matching v0.19.27. - Codex keeps the prose apps/skills/core/* (it follows the agentskills standard, which has no `!`…`` injection). Sourced separately, no shared-body magic. - Installers (sh/ps1/cmd) now route ~/.claude/skills <- apps/skills/claude and ~/.agents/skills <- apps/skills/core. Pi/OpenCode/Amp/Droid/Gemini/Copilot/ Kiro are unaffected — each runs through its own plugin or own files. Unattended install could hang (or silently install software) - The wizard gate probed /dev/tty openability, not interactivity, so a terminal with no human (docker run -t, devcontainer/Ansible pty) blocked forever on the first prompt. - The gate now also skips when $CI is set, and every prompt is bounded by a timeout (PLANNOTATOR_PROMPT_TIMEOUT, default 30s). On timeout/EOF (no human) prompts resolve to the safe "no" — never the prompt's default — so Glimpse (default yes) can't auto `npm install -g glimpseui` on an unattended terminal. Tests: install.test.ts updated for the per-agent skill routing; 72 pass.
Addresses review on #872. Keying can_prompt off $CI meant an exported CI var (some wrapper shells / dev environments) silently forced the non-interactive path, so --reconfigure no longer reopened the wizard and --extras only printed the manual command instead of launching `npx skills add`. Explicit flags are unambiguous user intent and must win over an env heuristic. The CI gate was only ever an optimization: real CI has no /dev/tty (already can_prompt=0), and the per-prompt timeout already prevents the unattended-tty hang safely. So drop the $CI check entirely and rely on the timeout.
Addresses review on #872. In the unattended-/dev/tty case the wizard runs (run_wizard=1) but every prompt times out to a safe `no`; the persist block then wrote extras=no/model_invocable=none/glimpse=no into install-prefs. Because a later interactive run skips the wizard when install-prefs exists, a provisioning/devcontainer bootstrap could make the wizard silently sticky-skip on the first real human install (unless they passed --reconfigure). ask_yes_no now returns non-zero on timeout/EOF; each wizard prompt ORs that into wizard_timed_out, and the prefs write is gated on wizard_timed_out=0. A timeout is now a one-off safe fallback, not a persisted preference. Real answers and explicit flags still persist as before. ps1/cmd don't have the timeout fallback, so they're unaffected.
Self-review follow-up. The script runs under `set -e`. ask_yes_no captured the read status with `read; rc=$?`, which only avoids aborting on a timeout because every current caller wraps it in `|| wizard_timed_out=1` (set -e is suppressed inside a function called in a tested context). That works today but leans on a subtle rule for the inner read. Switch to `read || rc=$?` so the read itself is always in a tested context, independent of how ask_yes_no is called. The function still returns non-zero on timeout/EOF to signal "no human"; callers continue to consume that with `|| wizard_timed_out=1` (a bare `$(ask_yes_no)` would still abort under set -e by design — the timeout must surface). Adds a regression assertion.
backnotprop
added a commit
that referenced
this pull request
Aug 3, 2026
…overage (#1187) Ports five small items from the closed parallel PR #1185 into the tolerant annotate argument resolution that landed in #1183 (#1182): - Bug fix: the token probe tested the raw token against the URL regex, but the pipeline strips the @ reference marker and wrapping quotes first, so a multi-token 'annotate @https://example.com/page and summarize it' probed to nothing and emitted the handoff instead of opening the URL. The probe now unwraps with stripAtPrefix before the regex and returns the unwrapped form (the pipeline re-strips harmlessly). Tests cover @-prefixed and quote-wrapped URLs as multi-token candidates. - Test ports: absolute-path candidate, the wider plain-text set (.txt, .yaml) guarding ANNOTATABLE_DOC_REGEX breadth, the scoped-package literal-@ fallback against a real @scope/ directory, and the whole-un-split-string preference over its own tokens ('Meeting Notes.md' wins over a resolving 'Notes.md' token) covering annotateInputNamesExistingTarget. - Defensive scan: the strict-mode source-scan test now asserts the annotate startup block gates tolerance on !strictAnnotate via isStrictAnnotateInvocation, since an inverted gate cannot be spawn-tested without starting a server. - DRY: the strict predicate was defined twice (strict-annotate-result exit-code helper and the index.ts tolerance bypass). Extracted isStrictAnnotateInvocation with a StrictAnnotateFlags type; both sites use it so the exit-code path and the tolerance bypass can never drift. Behavior byte-identical; existing subprocess tests unchanged. - Docs: the tolerant-resolution section now cites #872 (commit aac5aac) for why the bang prefix is deliberate and states that argument-shape issues belong in the CLI's resolution, not the skill templates. Refs #1185, #1182 Co-authored-by: Josh Nichols <josh.nichols+agent@gusto.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #850 (still unreleased). The pre-release review of #850 surfaced two regressions vs the released v0.19.27. Both fixes are isolated to the installer scripts and the skill sources.
1. Claude Code
/plannotator-*lost auto-run, argument passthrough, and gained a permission prompt#850 pointed Claude Code at the shared prose core skills, so the model now launches
plannotatorthrough a Bash tool call — a permission prompt on first run, and the typed argument (/plannotator-annotate <file>) is no longer passed through.Claude Code is skills-only now, but skills support the same dynamic-context injection the old commands used. This adds authoritative
apps/skills/claude/*skills that use:so
/plannotator-review|annotate|last|archiveauto-run with no prompt and pass$ARGUMENTSthrough — matching v0.19.27.apps/skills/core/*(it follows the agentskills standard, which has no!`…`injection). Sourced separately — no shared-body string magic.~/.claude/skills←apps/skills/claudeand~/.agents/skills←apps/skills/core.apps/skills/corebodies.2. Unattended install could hang (or silently install software)
The wizard gate probed
/dev/ttyopenability, not interactivity, so a terminal with no human (docker run -t, devcontainer/Ansible pty) blocked forever on the first prompt.$CIis set.PLANNOTATOR_PROMPT_TIMEOUT, default 30s).no— never the prompt's default — so Glimpse (defaultyes) can't autonpm install -g glimpseuion an unattended terminal. A real human pressing Enter still gets the prompt default.Test plan
bun test scripts/install.test.ts→ 72 pass (updated for the per-agent skill routing).bash -nclean oninstall.sh./dev/ttyheld open with no writer) and confirmed the bounded read returns and resolves tono.Notes / not in scope
install.cmddoesn't print a "predates layout" message whenapps/skills/claudeis missing on an old pinned tag (sh/ps1 do) — cosmetic.plannotator annotate-last; core/Codex skill usesplannotator last— both are CLI-accepted aliases.!`…`injection blocks during skill preprocessing until the browser session closes (same as the old commands); recommend a live smoke test before release.