Commit Graph

5 Commits

Author SHA1 Message Date
Michael Ramos 26ca4e0275 Single-source skills (core/extra), replace Claude Code commands with skills, de-hardcode installers (#850)
* feat: single-source skills into core/extra, replace Claude commands with skills, de-hardcode installers

- apps/skills/core/{review,annotate,last,archive}: single authoritative
  source for the always-installed skills (archive is new); all carry
  disable-model-invocation + agents/openai.yaml sidecars
- apps/skills/extra/{compound,setup-goal,visual-explainer}: no longer
  default-installed (except Kiro); installers print an
  `npx skills add backnotprop/plannotator/apps/skills/extra` suggestion
- Claude Code: apps/hook/commands/ deleted, command heredocs removed;
  core skills in ~/.claude/skills are the slash commands now
- Installers: OpenCode/Gemini command files copied from an extended
  sparse checkout instead of heredocs; install.cmd gains the previously
  missing OpenCode command install; aggressive cleanup of legacy
  ~/.claude/commands and ~/.codex/skills artifacts
- Codex: core skills install to ~/.agents/skills (official path);
  ~/.codex/skills install removed
- Pi: extension no longer bundles skills; #670 settings filter removed

* fix: review findings — old-tag soft guards, cmd replace-not-merge, plugin-update hint, frontmatter test

- install.sh: a --version tag predating apps/skills/core no longer aborts
  the whole copy subshell (which also skipped OpenCode/Gemini commands);
  core skills now soft-skip with an accurate message, matching ps1/cmd
- install.sh: subshell failure message no longer claims "git required"
  when git was present (clone/network errors get their own wording)
- install.cmd: pre-remove skill dirs before xcopy so upgrades replace
  rather than merge (stale files from renamed/deleted skill files no
  longer linger; parity with sh/ps1)
- all installers + docs: tell upgraders to run /plugin marketplace update
  so the plugin's old namespaced plannotator:* commands disappear (#817)
- install.test.ts: assert every core SKILL.md sets
  disable-model-invocation: true — the load-bearing line that keeps core
  skills out of Pi's system prompt (#842 regression guard)

* test: pin old-tag soft-guard behavior, dedupe core-skill list in tests

* fix: interrogation review findings — cross-installer diagnostic parity

- install.ps1/install.cmd: emit the "predates the core/extra skill
  layout" diagnostic on old pinned tags instead of silently skipping
  core skills (parity with install.sh)
- install.ps1: clone/network failure no longer claims "git required"
  (git was already verified present); the outer catch now reports the
  actual exception
- install.sh: "Installed OpenCode/Gemini commands" echoes are guarded
  on the copy actually having a source, so old pinned tags don't print
  false success (ps1/cmd already gated this way)
- AGENTS.md: opencode-plugin commands/ comment now reflects all four
  command stubs
- install.test.ts: shared test asserts the soft-skip diagnostic exists
  in all three installers and pins ps1's honest failure wording

* fix: respect CODEX_HOME for Codex home directory (#852)

Codex stores config and state under $CODEX_HOME when set, falling back
to ~/.codex (developers.openai.com/codex/config-advanced). Plannotator
hardcoded ~/.codex in two places:

- runtime: codex-session.ts scanned ~/.codex/sessions for rollout
  files, so `plannotator last` failed with "No rendered assistant
  message found" when CODEX_HOME pointed elsewhere. Now resolved the
  same way copilot-session.ts handles COPILOT_HOME and session-log.ts
  handles CLAUDE_CONFIG_DIR.
- installers: detection, config.toml/hooks.json paths, manual-setup
  instructions, and the stale-skills cleanup now derive from
  CODEX_HOME in all three scripts.

Tests: codex-session.test.ts covers rollout discovery under a
CODEX_HOME temp dir; install.test.ts asserts all three installers
respect the variable and that the fallback is the only hardcoded
~/.codex path left in install.sh.

* fix: hard-fail skill install, guard command cleanup, one-time extras migration

External review triage on PR #850 surfaced two real installer issues:

P1 — commands deleted before replacement: the Claude command cleanup
ran before the git-gated skill install, so a missing git, a failed
clone, or an old pinned tag deleted the user's slash commands and
installed nothing (a regression — the old installer needed no git).
Now:
- missing git is a hard failure before anything is touched ("install
  git, then run this installer again")
- a failed fetch is a hard failure ("something went wrong — run the
  installer again") instead of a silent skip
- the legacy command cleanup runs AFTER the install and only removes a
  command file when its same-name replacement skill exists on disk
- old pinned tags keep the soft-skip (no deletion happens, commands
  survive, CI e2e against old tags stays green)

P2 — recurring extras deletion: the extras cleanup ran on every
invocation, deleting copies users reinstalled via the suggested
`npx skills add` (the copies are byte-identical, so only provenance
can tell them apart). The cleanup is now a one-time migration recorded
in a migrations ledger under the Plannotator data dir
(<PLANNOTATOR_DATA_DIR|~/.plannotator>/migrations/), the same
record-what-you-did pattern package managers use.

All three installers (sh/ps1/cmd) updated in parity; tests pin the
guard condition, the ledger gating, and the hard-fail messages.

* test: tripwire — install.cmd must never contain /dev/null redirects

* fix: every skill sets disable-model-invocation — no exceptions

Maintainer rule: all Plannotator skills are user-invoked, never
model-auto-invoked. setup-goal (missing since #665) and the three Kiro
skills now carry the flag. The frontmatter test scans every SKILL.md in
apps/skills/core, apps/skills/extra, and apps/kiro-cli/skills
dynamically — with a floor of 10 — so a future skill cannot ship
without it.

* docs: git is a hard installer requirement; clarify post-gate sections complete on re-run

* docs: align ps1/cmd comments with hard-fail semantics

* feat: guided install — extras opt-in via skills CLI, model-invocation picker

Interactive terminals get a two-question wizard on first run:
1. Install the extra skills? Yes delegates to `npx skills add
   backnotprop/plannotator/apps/skills/extra` (its UI picks the agents),
   wired to /dev/tty so piped curl|bash installs still work. Skipped
   when extras already exist on disk.
2. Make any skills callable by the model? Yes opens a space-toggle
   checkbox (sh/ps1) or numbered toggles (cmd), listing all skills if
   extras were chosen, core-only otherwise. Chosen skills get
   disable-model-invocation stripped from their INSTALLED copies and the
   Codex sidecar's allow_implicit_invocation flipped — re-applied every
   run since installs replace skill folders. Repo sources stay locked.

Answers persist to <data dir>/install-prefs (shared format across all
three installers) and re-runs reuse them silently; --reconfigure
re-opens the wizard. Automation is untouched: no terminal means no
prompts and today's defaults; --extras/--no-extras/--model-invocable/
--non-interactive give scripts explicit control.

* fix: self-review of guided install — cmd pipe expansion bug, flag/wizard interplay

- install.cmd: the checkbox preselection used `echo !var! | findstr` —
  each side of a cmd pipe runs in a child WITHOUT delayed expansion, so
  the saved choices passed through as literal !var! text and
  preselection never matched. Replaced with a substring-replace
  containment test (no pipe).
- all three: a wizard question whose answer was already provided by a
  CLI flag (--extras/--no-extras/--model-invocable) is no longer asked
  and then silently overridden — the flag pre-answers it.
- install.cmd: unknown-option usage line now lists the wizard flags.

* feat: guided install question 3 — install Glimpse (native window)

glimpseui (third-party npm package, PR #840) gives Plannotator a native
WebView window instead of a browser tab; the runtime already
auto-detects it on PATH, so a global install is all that's needed.

- Wizard asks "Install Glimpse?" (default yes) after the skills
  questions; skipped when glimpseui is already on PATH
- Yes runs `npm install -g glimpseui` (bun fallback on sh/ps1; printed
  instruction when neither exists) — wizard or explicit flag only,
  silent re-runs never install software
- --glimpse / --no-glimpse flags for automation; choice persisted to
  install-prefs like the others
- docs + tests updated (glimpse detection, install command, flags, and
  persist-condition assertions across all three installers)

* fix: self-review of Glimpse question — cmd bun fallback, stale usage text

* fix: merge-window hardening — guard Codex cleanup, remove old-installer junk dirs

plannotator.ai serves install.sh live from main (public/ symlink,
deployed on push), while the script fetches repo files at the LATEST
RELEASE TAG. Between merging the core/extra restructure and cutting the
release that ships it, the live script runs against the old-layout tag.
Two hazards in that window:

1. The Codex stale-skill cleanup removed working ~/.codex/skills with
   no successor installed (core skills soft-skip on old tags). Now the
   cleanup runs AFTER the install and removes a core skill only once
   its replacement exists in ~/.agents/skills — same guard the Claude
   command cleanup uses. The compound/setup-goal stale copies stay
   unconditional (never Codex's to begin with).

2. The reverse combo (cached OLD script + NEW release tag) wholesale-
   copies apps/skills/* and leaves junk core/ and extra/ directory
   copies in ~/.claude/skills. Never valid skill names — all three
   installers now remove them on every run.

* fix: glimpseui is a devDependency — consumers never use it from node_modules

PR #840 added glimpseui to dependencies in @plannotator/server and
@plannotator/pi-extension, but nothing imports it: both runtimes detect
the CLI on PATH (Bun.which / a manual PATH walk) and spawn it. The dep
only ever mattered in repo development, where `bun run` prepends
node_modules/.bin to PATH. For consumers it was inert download weight —
OpenCode plugin installs and `pi install` pulled a third-party package
that could never be detected (Pi's loader does not expose
node_modules/.bin; verified). Moved to devDependencies in both: dev
flows keep working, published packages stop shipping it. The sanctioned
end-user path is the guided installer's global `npm install -g
glimpseui`.

* fix: clean stale plugin command files from the installed plugin checkout (#817)

The installer already manages hooks.json inside
~/.claude/plugins/marketplaces/plannotator/apps/hook/, so the earlier
"don't reach into plugin storage" rationale for leaving the old
namespaced plannotator:* command files there was inconsistent. All
three installers now remove them — same replacement-skill guard as the
bare ~/.claude/commands cleanup — making the #817 duplicate menu
entries die on a single installer run + restart instead of waiting for
/plugin marketplace update. Hints/docs reworded accordingly.

* Revert "fix: clean stale plugin command files from the installed plugin checkout (#817)"

This reverts commit 3df37da92c.

* fix: Windows CI — assert the new no-commands contract; gate cmd wizard on a real console

The cmd e2e CI step still asserted the OLD contract (installer writes
~/.claude/commands/plannotator-*.md with the ! prefix). Commands are
dead; the step now guards the NEW contract: a fresh install must write
NO plannotator command files. The Gemini TOML assertions stay — they
now verify the verbatim checkout copy delivers intact files (the tag
the e2e pins, v0.17.1, contains apps/gemini/commands).

The failure also exposed that install.cmd ran the wizard on redirected
stdin (set /p falls through to defaults at EOF) — and Q3's default
being yes meant CI silently ran `npm install -g glimpseui`. cmd now
probes for a real console via `timeout /t 0` (errors when stdin is
redirected), matching sh's /dev/tty and ps1's IsInputRedirected gates:
no console, no wizard, no wizard-only installs.
2026-06-05 09:23:45 -07:00
Graham Lipsman be2c81fa3b annotate-last: pick which message to annotate (fixes #800) (#809)
* feat: message picker for annotate-last (#800)

When running /plannotator-last after /rewind, the newest transcript
entry is no longer the message the user intended to annotate, and there
was no affordance to pick a different one.

Adds a picker UI that surfaces the recent assistant messages so the
user can choose which one to annotate:

- A "Message N of M" button in the Viewer's sticky-top action bar
  (alongside Copy / Global comment / Attachments), so it stays
  accessible while scrolling.
- A "Messages" tab in the left sidebar with the full list
  (newest-first, preview + timestamp, default ★), mirroring the
  existing Files / Versions / Archive tab pattern.

Wired for Claude Code, Codex, and Droid (all share apps/hook/server).
OpenCode, Pi, and Copilot still get the original single-message
behavior — they don't emit recentMessages, so the picker affordances
hide cleanly.

Default selection (index 0) matches today's "last message" behavior,
so users who don't interact with the picker see no change.

* feat: extend annotate-last picker to Copilot and OpenCode

The picker UI from #800 was wired for Claude / Codex / Droid only. Pull
Copilot and OpenCode onto the same shape so users on those harnesses
also get the recent-messages picker when annotating the last assistant
message.

- Copilot: replace getLastCopilotMessage with getRecentCopilotMessages,
  walking events.jsonl newest-first up to 25 assistant.message events.
- OpenCode: rewrite the session walk to collect up to 25 messages
  (newest first) instead of bailing on the first hit; normalize the SDK
  time.created (ms epoch) to ISO to match the shared picker contract.
- Both pass recentMessages to startAnnotateServer only when length > 1,
  matching the existing Claude/Codex/Droid behavior.

Also trims a leftover narrating comment in MessagesBrowser and refreshes
the stale Copilot session-parser header.

Pi parity follows in the next commit (needs round-trip of the picker
selection through /api/feedback so its post-submit anchoring quotes the
right message).

* feat(pi): wire annotate-last picker with feedback round-trip

Extends the picker UI (#800) to Pi and fixes a Pi-specific anchoring bug
the picker would otherwise introduce.

Picker plumbing
- assistant-message: getRecentAssistantMessages walks the active branch
  newest-first, returning { messageId, text, timestamp? } in the same
  shape the other harnesses produce.
- Plumbed through plannotator-browser / plannotator-events so the Bun
  server's recentMessages option is populated when the branch has more
  than one assistant message.

Anchoring fix
- Pi quotes the targeted assistant message back to the agent because its
  UX is async — the conversation may have moved on by feedback time.
  With the picker, that target is no longer guaranteed to be the
  snapshot taken when the UI opened. The editor now sends the user's
  selectedMessageId with /api/feedback; Pi looks it up in the current
  branch via findAssistantMessageByEntryId and quotes that message
  instead. Falls back to the original snapshot if the entry is gone.
- The round-trip field is optional and only meaningful in annotate-last
  mode; other harnesses (and other modes) ignore it.

Timestamp safety
- Pi's SDK currently types SessionEntryBase.timestamp as string, but the
  picker contract everywhere else is ISO. Treat the value as unknown and
  normalize string/number(ms)/Date to ISO; drop anything else, rather
  than blind-casting and risking silent drift if the SDK changes.

* chore: strip issue-number references from comments

Comments shouldn't rely on external references — issue numbers age out
of context, link rot is a thing, and a reader shouldn't need to open
GitHub to understand why a line exists. Strip the `(#800)` and `(#570)`
parentheticals from comments and doc strings across the picker and
review-gate code; the surrounding "why" content is preserved.

* fix: prevent removeChild crash when switching annotate-last messages

Switching the picked message remounted nothing, so React reconciled new
content against DOM that web-highlighter had mutated with <mark> nodes,
throwing removeChild. Drive the Viewer key (and StickyHeaderLane's
remount token) off a shared viewerContentKey so a message switch fully
remounts the Viewer and re-anchors the sticky-header observer.

Also cap MessagesBrowser row previews via previewText() and drop the
redundant 'block' class that was overriding line-clamp-2.

* feat: persist annotate-last feedback across messages

---------

Co-authored-by: Michael Ramos <mdramos8@gmail.com>
2026-06-03 13:01:36 -07:00
Muhammad Hashmi 9873cdb3f6 Fix Codex annotate-last message selection (#740) 2026-05-17 20:45:10 -07:00
Andrei Ivanov a22a744749 Add Codex Stop-hook plan review (#577)
* feat: add codex stop hook plan review

* Install Codex plan review hooks

* Remove Codex manual test screenshots

* Update Codex plan mode docs

* Tighten Codex release readiness

* Preserve custom Codex hook wrappers

* ci: smoke test release artifacts

* ci: reduce release smoke flake risk

* fix: keep Codex last-message extraction to output text

* ci: poll release smoke servers on loopback

* ci: skip macOS release smoke jobs

---------

Co-authored-by: Michael Ramos <mdramos8@gmail.com>
2026-05-02 13:21:42 -07:00
Michael Ramos 6b775ea1ed feat: /plannotator-last — annotate the last agent message (#325)
* feat: add /plannotator-last command to annotate last assistant message

Adds a new slash command that extracts the last rendered assistant message
from Claude Code's session log and opens it in the annotation UI.

Session log parser (apps/hook/server/session-log.ts):
- Parses Claude Code JSONL logs at ~/.claude/projects/{slug}/*.jsonl
- Finds the last assistant message.id with text content blocks
- Skips noise entries (progress, system, file-history-snapshot, queue-operation)
- Filters system-generated user messages by prefix to avoid false turn boundaries
- Walks backward through empty turns when back-to-back user messages exist
- No anchoring — reads from end of log since <command-message> isn't written
  until after the binary completes

New files:
- apps/hook/commands/plannotator-last.md — slash command definition
- apps/hook/server/session-log.ts — Claude-Code-specific log parser
- apps/hook/server/session-log.test.ts — 30 tests covering streaming chunks,
  tool call turns, sub-agent noise, stop hooks, thinking blocks, and edge cases

Modified:
- apps/hook/server/index.ts — annotate-last subcommand

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* chore: remove 3 redundant real-world scenario tests

These duplicated coverage already provided by focused unit tests:
- "full conversation" → covered by "grabs last message.id in multi-tool turn"
- "stop hook interrupted" → covered by "skips progress and system noise"
- "long tool-only sequence" → covered by "skips tool-only assistant entries"

Kept the thinking block test (unique coverage). 27 tests remain.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat: add /plannotator-last command to Pi extension

Uses Pi's session manager API to find the last assistant message —
walks backward through ctx.sessionManager.getEntries(), finds the
last entry with role "assistant" and text content, opens it in the
annotation UI. Reuses existing isAssistantMessage(), getTextContent(),
startAnnotateServer(), and runBrowserReview() from the extension.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat: add /plannotator-last to OpenCode plugin + extract command handlers

Adds annotate-last command that fetches session messages via
client.session.messages(), finds the last assistant message with text
parts, and opens it in the annotation UI.

Refactors command handling: extracts review, annotate, and annotate-last
handlers from the inline event hook into commands.ts module. Reduces
index.ts by ~120 lines and makes adding future commands cleaner.

New files:
- apps/opencode-plugin/commands.ts — extracted command handlers
- apps/opencode-plugin/commands/plannotator-last.md — command metadata

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat: context-aware UI labels for annotate-last mode

Adds "annotate-last" mode to the annotate server, passed through to the
UI via /api/plan response. The editor uses this to show "Copy message"
instead of "Copy plan", and "annotations on the message" in the
completion overlay.

- packages/server/annotate.ts: new `mode` option on AnnotateServerOptions
- packages/editor/App.tsx: annotateSource state derived from mode
- packages/ui/components/Viewer.tsx: copyLabel prop for button text
- All three harnesses pass mode: "annotate-last" in their callers

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat: add Codex support to annotate-last command

Detects Codex via CODEX_THREAD_ID env var (injected by Codex into every
spawned process). Uses the thread ID to find the rollout file in
~/.codex/sessions/, parses the Codex rollout JSONL format to extract
the last assistant message.

Also adds `plannotator last` alias for shorter usage in Codex bang
commands (!plannotator last).

New files:
- apps/hook/server/codex-session.ts — Codex rollout parser
- apps/hook/server/codex-session.test.ts — 9 tests

Modified:
- apps/hook/server/index.ts — Codex detection + `last` alias

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: context-aware feedback title + top spacing for paragraph-first content

- exportAnnotations now accepts a title param: "Message Feedback" for
  annotate-last, "File Feedback" for file annotation, "Plan Feedback"
  for plan review (default)
- Adds top spacer when content starts with a paragraph (not a heading)
  and has no frontmatter, fixing tight spacing in annotate-last mode

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* chore: add sandbox scripts for Pi and Codex testing

- sandbox-pi.sh: builds extension, creates temp project, installs via
  `pi install`, launches Pi with sample files
- sandbox-codex.sh: compiles binary, creates temp project, launches
  Codex. Test with `!plannotator last`

Both follow the same pattern as sandbox-opencode.sh.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: add hook build step to opencode sandbox script

The opencode build copies HTML from hook/dist/ — without building hook
first, the sandbox could use stale HTML. Pi and Codex sandboxes already
had this step.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: remove command body from plannotator-last to prevent agent response

The .md body was being sent to the agent as a prompt, causing it to
respond with "Opening annotation UI..." before the event handler could
fetch messages. That response became the "last message" instead of the
actual one. Empty body = agent stays silent, event handler intercepts.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: use command.execute.before hook for OpenCode annotate-last

Moves plannotator-last from the passive event hook to the
command.execute.before hook. This intercepts the command before the
agent sees it, clears output.parts so the agent stays silent, fetches
session messages, opens the annotation UI, then sends feedback via
client.session.prompt() — same pattern as review/annotate.

Previously the agent would respond to the command body before the
event handler could fetch messages, polluting the session history.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: add Codex to origin type and agent name mapping

Origin "codex" was falling through to the default "Coding Agent" label.
Added "codex" to the origin union type across annotate server, editor,
and removed the `as any` cast in the hook.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: remote share link, plan-specific prose, and codex type unions

- Add writeRemoteShareLink to annotate-last onReady callback so remote
  sessions get a reachable URL
- Add subject parameter to exportAnnotations so feedback says "message"
  or "file" instead of "plan" when appropriate
- Add 'codex' to origin type unions in useAgents, Settings, UpdateBanner,
  and App.tsx fetch handler

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: correct JSDoc for projectSlugFromCwd (leading dash is kept, not stripped)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor: use RenderedMessage type instead of inline structural type

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-17 23:49:34 -07:00