441 Commits

Author	SHA1	Message	Date
Michael Ramos	82f5648ccb	chore: bump version to 0.22.0	2026-07-05 11:57:30 -07:00
Michael Ramos	42686f8f1e	docs(marketing): bring the code-review reference up to the since-base era ... The page still described the pre-#990 product: uncommitted default, no Git status/Tree/Commits panel views, no Guided Review, an agent roster without the marker engines, and a Server API table missing nine endpoints. Rewritten to match what ships.	2026-07-05 11:50:41 -07:00
Michael Ramos	ed4f2e82fe	feat(review): Guided Reviews intro dialog + Guide button discovery hint (#1000) ... * feat(review): one-time Guided Reviews intro dialog + Guide button hint New GuideIntroDialog slots into the announcement chain (look-and-feel -> guide intro -> review setup, never stacking): a hero screenshot of a guided review with a brief plain-language description and a mock header strip pointing at the real Guide button. Shows once per user (cookie), and only when a Guide button actually exists (hasSearchableFiles) -- skipping the render does not consume the one-shot cookie. The hero is CDN-hosted at https://plannotator.ai/assets/guided-review.webp (committed to apps/marketing/public/assets so every marketing deploy re-ships it past the --delete sync; uploaded to S3 now so it is already live) instead of growing the single-file bundle; on load failure the image container hides entirely for a clean text-only dialog. Independently of the dialog, the header Guide button shimmers (TextShimmer) with a pulsing dot until the guide is first opened -- acked on ANY open path (button, shortcut, job auto-open), not just the button click. * fix(review): guide intro first in the dialog chain, full-width panel Chain is now guide intro -> look-and-feel -> review setup (was second); panel widened to max-w-5xl to match the look-and-feel dialog. Intro seen flag switched to the versioned-cookie pattern so future revisions can re-show it. * fix(review): latch guide-intro eligibility at first post-load render hasSearchableFiles changes on mid-session diff switches; an empty-to-non-empty switch would pop the intro over work in progress, or yank an open look-and-feel dialog (its gate reads guideIntroVisible). Eligibility is now decided once when loading clears -- dialogs only mount after that, so the latch is always set before they render. A skipped session keeps the cookie unconsumed. Also corrects AGENTS.md's chain order (guide intro first).	2026-07-05 09:49:00 -07:00
Michael Ramos	aa46dbb29a	feat: guide per-file summaries + GitHub Copilot CLI agent engine (#997) ... * feat(guide): per-file summary on guide diff refs Each diffs[] entry now carries a required (schema-enforced) 1-2 sentence summary of the semantic change in that file, written from the diff hunks alone. Sanitizer passes it through when it's a non-blank string and omits it otherwise -- a missing summary never drops the ref or fails the guide. Rendered as a muted inline-markdown line above each diff in the guide; marker-engine contract and demo data updated to match. * fix(guide): repair prompt must not invent missing summaries The guide schema now requires summary, so a schema-enforced repair of a payload that lacks them (marker-engine output) would force the model to fabricate captions with no diff in sight. Instruct it to fill missing required fields with an empty string instead; the sanitizer already renders nothing for blanks. * feat(agents): GitHub Copilot CLI as a marker engine for review + guide jobs Adds copilot as the fourth marker engine (no schema flag, so it uses the nonce-tagged marker-block contract like Cursor/OpenCode/Pi): - marker-review.ts: COPILOT_ENGINE — 'copilot --output-format json' JSONL stream (assistant.message carries the assembled text; deltas skipped), models discovered from 'copilot help config', live-log formatting for tool.execution_start/complete. Non-interactive posture: --no-ask-user auto-denies unallowed tools, --deny-tool=write, read-only-oriented shell allowlist (git/gh/glab/jj/wc), builtin MCPs and auto-update disabled. - Exported MarkerEngineId and replaced every 'cursor'|'opencode'|'pi' cast with it (Bun server, Pi server mirror, guide-review) so the next engine is a two-edit change. - agent-jobs (both runtimes): copilot in SERVER_BUILT_PROVIDERS; capability entry + model discovery come free from the MARKER_ENGINES loop. - UI: copilot review/guide engine with per-surface model settings (useAgentSettings), AgentsTab launch + config rows, GuideEmptyState launcher, job-detail labels, CopilotIcon (currentColor, official mark). - Tests: argv/read-only flags, help-config model parsing, stream reduction, full marker pipeline; profile-map expectations widened. Verified live: composed review prompt through the real copilot binary, marker block parsed, seeded bug found. * fix(agents): structurally deny high-consequence verbs for Copilot jobs git:*/gh:*/glab:* stay allowed for inspection ergonomics, but Copilot's deny-precedence rules now block the verbs a prompt-injected background job could abuse: git push/reset/clean/checkout/restore (local reviews run in the user's real working tree), and PR/MR/issue comment/create/merge/ close/edit/review on gh and glab. Probe-verified: git log runs, git push --dry-run is denied by the shell(git push) rule. Addresses the one confirmed finding from the PR #997 review round; the other two (untracked-file reads, git -C) were refuted by live probes.	2026-07-05 09:12:20 -07:00
Michael Ramos	d6b98f0c0d	feat(review): Guided Review + Pi agent-job provider (#993) ... * docs(adr): Guided Review ADR/spec/research + agent-provider fit studies ADR 006 (Guided Review as a first-class feature), four implementation spikes, synthesis, spec (iterated through preflight and review fixes), recap, and the Flue/Pi provider-fit research syntheses. * feat(review): Guided Review takeover + Pi agent-job provider Guided Review (ADR 006) — a Linear-Guides-style chaptered review: - guide agent-job provider (packages/server/guide/guide-review.ts): schema-constrained sections (title/overview/file refs) over the live patch, coverage-validated server-side (every changed file exactly once, fabricated paths dropped, fail-closed on empty output), importance-first ordering prompt with speed discipline (diff-first, no repo exploration) - runs on claude/codex natively and cursor/opencode/pi via the marker contract; guide-scoped low effort defaults for quicker generation - takeover UI (packages/review-editor/components/guide/): Guide header badge + Mod+Shift+G, full-width screen that CSS-hides (never unmounts) the file tree/dock, empty state with inline model pickers, skeleton loading, generating state with collapsed activity log, two-column section cards (sticky prose column with internally-scrolling file list, content-height diffs, review-independent collapse, peek semantics) - annotation parity: guide diffs mount the real DiffViewer with file-scoped handlers into the same CodeAnnotation state and feedback export; per-section reviewed state persisted via /api/guide/:jobId - reviewed/tour prompt speed sections; renderMarkdownProse shared between tour and guide (fenced-block support, muted tone variant) Pi agent-job provider: - third MarkerEngine (pi --mode json --no-session --no-approve) with live model-catalog discovery, thinking-level control (--thinking), fail-closed marker parsing (Pi exits 0 on in-run errors by design) - available for review + guide jobs from both launch surfaces Shared UX: - searchable, provider-grouped model pickers (auto over 12 options) - full Claude version catalog with latest-resolving aliases labeled - AgentControls primitives extracted from AgentsTab; cross-instance settings sync in useAgentSettings - Pi extension server hand-mirrors + vendor.sh entries throughout * fix(guide): PR-993 review fixes + failure recovery ladder Review fixes: - coerce marker-engine title/intent at ingest (prompt-enforced output could store non-strings and crash the takeover) - keep placed files out of unplacedFiles (exactly-once guarantee) - shared REVIEW_ENGINE_LABEL (fixes 'generated by pi' header; typed exhaustiveness prevents recurrence) - key ActiveGuide by jobId so per-guide focus state resets - formatModel handles marker-engine tour/guide jobs (empty model chip) - drop dead guideCodexFast; memoize estimateDiffHeight - store + display Pi thinking level on job cards Failure recovery ladder (auto -> one click -> manual -> regenerate): - mechanical JSON repair pass (fences/slice/trailing-commas/bracket balance) before any guide parse fail-closes - failed payloads captured per job (200KB cap, per-engine extraction) - 'Fix output' repair job: pure text transform on a schema-capable engine (claude/codex preferred), forced low effort, lands as a normal guide job; repairOf threaded through launch validation - editable output panel: GET /api/guide/:jobId/output prefills a textarea, POST /api/guide/:jobId/submit runs the same server-side validation and opens the fixed guide; inline errors for iteration - Pi extension mirrors + AGENTS.md endpoint/body docs * fix(guide): string-aware mechanical JSON repair (self-review) - terminate a dangling string literal before appending bracket closers (truncation mid-string is the most common shape; closers appended inside the string never parsed) - strip trailing commas outside string literals only (the regex could rewrite overview content, violating the repair contract) * fix(guide): launch-snapshot validation, codex strict schema, repair persistence PR-993 round-2 review fixes + live-test findings: - validate guide completion (and manual repair) against the LAUNCH-time changed-file set, snapshotted per job — switching diff/base/PR while a guide generates no longer destroys valid output (client already degrades stale refs per-file) - codex strict structured output: unplacedFiles now required in the guide schema (OpenAI 400s schemas with optional properties under additionalProperties:false — live-confirmed, then live-verified fixed: 5 sections / 83 files placed on a ChatGPT-account codex) - manual repair survives reload: successful /submit flips the job to done via completeJobExternally + status guard on the endpoint - codex model defaults -> gpt-5.5 (5.3-codex is deprecated; ChatGPT accounts reject it) with one-shot migration of saved picks; guide codex default reasoning stays low - pi extension guide branch snapshots launch state (TOCTOU hygiene) - paragraph collector no longer swallows an unspaced code fence - tour hook backports: out-of-order fetch guard; save outside updater - guide empty-state copy rewritten value-first * fix(guide): PR-993 round-3 — repair-diff fidelity, failure surfacing, pi tool hardening - repair jobs validate against the FAILED job's recorded file set (the payload under repair references that changeset; validating against the diff on screen at repair time re-introduced destroy-on-switch) - takeover follows the NEWEST running guide (progress + Cancel), and a failure newer than the shown guide surfaces as a dismissible strip (error + Fix output + Details) instead of being silently masked - pi jobs run with --exclude-tools edit,write (bash stays: the agent needs git inspection; full read-only would break generation) - tour/guide result ingestion fails closed on unexpected throw (done- looking jobs no longer 404 their result) - sidebar guide mode gated on file availability like the header badge - drop orphaned DEFAULT_GUIDE_CODEX_FAST (fast mode intentionally not offered for guide); retry owns its fetch cancellation; final trailing-comma pass after bracket-closing in the repair ladder; collapsed-row checkbox/expand are sibling buttons (a11y) * fix(guide): PR-993 round-4 — callout text, guide job detail, toggle races - single-line callouts (> [!IMPORTANT] message) keep their message — the form our own prompt solicits rendered an empty labeled box; type now derives from the [!TAG] capture, not a whole-line scan that mistyped '> [!NOTE] this is important' - job detail panel gets a guide status card (Open guide via a new ReviewStateContext.openGuide) instead of mislabeling 'Guide Generated' as a red Incorrect review - marker-engine guide jobs get readable live logs (engine fallback in the formatter lookup; provider stays 'guide') - repair prefers the failed job's own schema-capable engine (proven working on this machine) before falling back by binary presence - toggleReviewed/toggleChecked: pure functional updaters + persistence effect with seed-skip — race-proof AND StrictMode-safe (resolves the tradeoff previous rounds accepted) - dedups: formatDuration/ElapsedTime shared from AgentsTab; whichCmd exported in the pi extension; SEARCHABLE_THRESHOLD imported by InlinePicker - new renderMarkdownProse tests (single-line/multi-line callouts, paragraph passthrough) * fix(guide): PR-993 round-5 — keep engine-default model option, label Pi jobs - GuideEmptyState's marker catalogs mirror AgentsTab's per-engine semantics: opencode/pi prepend the engine-managed Default ('' value) to the discovered list (dropping it left saved-default users a blank pill with no way back); cursor replaces (its list includes 'auto') - job detail provider pill labels pi review jobs 'Pi' instead of the 'Shell' fallback * fix(guide): PR-993 round-6 — context scoping, stale models, repair engine, staging gate - guide takeover + auto-open scoped to the current review context: a guide launched against PR A no longer shows over PR B, and a guide finishing for an away context defers its auto-open until the reviewer returns to that context (unmarked in the dedupe set on purpose) - guide launcher reconciles saved cursor/opencode/pi model ids against the live catalog at read time (picker + launch share one effective value) — no more posting dead ids after an account switch - repair prefers the failed job's OWN engine whenever its binary is present (provably runnable; marker binaries resolved via MARKER_ENGINES — cursor's CLI is 'agent'); claude/codex are fallback only, killing the broken-claude repair doom loop - per-file staging gate (canStagePath) in guide diffs AND the pre-existing same gap in ReviewDiffPanel — committed-only files in since-base reviews no longer offer a no-op Git Add * fix(review): scope tour auto-open to current context (self-review) Same cross-context gap just fixed for guides: a tour finishing for PR A popped its dialog over PR B. Both auto-open effects now share one jobMatchesCurrentContext helper with the same deferred-open semantics. * fix(guide): PR-993 round-7 — complete the context-scoping story Round 6 scoped guides to their review context; this closes the two surfaces that scoping left dangling, plus a focus gap: - switching to a context that already HAS a completed guide now shows that guide: the takeover falls back to the context's newest done guide job when activeGuideJobId belongs elsewhere (previously landed on the empty state with the guide sitting unreachable) - 'Open guide' affordances are context-gated everywhere they exist: job cards hide it for cross-context guides (opening can't switch PRs), and the job detail panel explains where the guide belongs instead of offering a dead button; one shared jobMatchesReviewContext predicate now backs App, GuideScreen, AgentsTab, and the detail panel - file-chip navigation retargets the guide's focus arbiter (scrolling under a stationary pointer fires no pointerenter, so the annotation toolbar stayed bound to the previous diff) * fix(tour): retry owns its fetch cancellation; document prUrl-only scoping - useTourData retry converts to the nonce-driven effect re-run pattern (the round-4 backport carried the fetch guard but not the retry fix useGuideData got in the same batch — half a backport) - jobMatchesReviewContext now documents WHY it matches by prUrl only and not diffScope/diffContext: guides/tours reference files and degrade per-file when the diff shifts; scope-strict matching would hide useful artifacts on layer/base/mode switches. Deliberate, do-not-tighten-without-UX-decision * fix(guide): PR-993 round-9 — worktree-aware context, guide-scoped marker settings - jobMatchesReviewContext now compares diffContext.worktreePath for local jobs: prUrl + worktree define WHERE the review is (matched strictly); mode/base/scope remain WHAT VIEW (deliberately loose) — a guide launched against worktree A no longer opens over worktree B. Jobs predating the snapshot coalesce to the main tree. - Cursor/OpenCode/Pi get guide-scoped model/thinking settings (guideCursor/guideOpencode/guidePi), mirroring the existing guideClaude/guideCodex isolation: tuning a guide's marker model no longer silently changes the next code review with that engine. Defaults are each engine's natural default, not seeded from review settings (same precedent as guideClaudeEffort='low'). AgentsTab's reconcile effects snap both surfaces against the shared catalogs. * fix(review): keep worktree parsing browser-safe (round-9 build fix) parseWorktreeDiffType lives in shared/review-core, which imports node:path at module top — pulling it into App.tsx broke the vite browser build (typecheck and tests don't catch it; only the bundle does). Context matching now reuses App's existing hand-parsed activeWorktreePath memo — one parse, and it's the same one that drives the sections/tree UI, so matching aligns with what's on screen. Also fixes a deps-array reference to the removed memo that bundled fine (treated as a global) but would have thrown a ReferenceError at runtime. * docs(review): mark worktree-subtype lockstep between server parser and App copy * fix(review): PR-993 round-10 — guide shortcut gate, settings sync race, heading markdown - bare a/v staging/viewed shortcuts suspend while the guide takeover is open (the dock is only CSS-hidden, so a diff panel stayed 'active' underneath and bare keys acted on an invisible file) - cross-instance settings sync replaces the was-last-update-remote boolean with value comparison (lastSyncedJsonRef): the flag conflated 'a commit happened' with 'the last change was remote', so a local edit batched with an incoming broadcast silently skipped its own cookie write and rebroadcast - ### headings in guide/tour prose run through renderInlineMarkdown like every other block (backticked symbols rendered as raw tokens); regression test added * fix(guide): PR-993 round-11 — reveal channel for sidebar jumps, repair exit code - new guideRevealFile channel closes the documented round-7 gap and its AI sibling: sidebar jumps (annotation clicks, AI line citations) made while the guide takeover is open no longer mutate the hidden dock's active file; instead the GuideSectionCard containing the target file expands its collapsed (reviewed) section, focuses the diff, and scrolls to it — so jumps into collapsed sections stop silently no-opping. Cleared on guide close/switch so keyed remounts don't replay the last reveal. - completeJobExternally resets exitCode to 0 (both servers): a successfully repaired guide kept showing the failed run's Exit 1 chip in the job detail panel * fix(guide): clear reveal channel in-batch at guide-switch sites (self-review) The clear-on-change effect fires after a switched guide's keyed cards have already mounted (child effects run before parent effects), leaving one commit where a stale reveal from guide A could expand+scroll a same-named file's section in guide B. All three setActiveGuideJobId sites now clear synchronously in the same batch; the effect stays as a backstop for close and future set sites. * fix(guide): PR-993 round-12 — titled sections, honest large-PR validation - sanitizeGuideSection gives every surviving section a non-empty title ('Untitled section' fallback): a diffs-only section rendered as a blank chapter inside a 'Guide Generated' job — no parse failure, so no recovery flow. Keeping (titled) beats dropping: placed files are not in unplacedFiles, so dropping would orphan them silently. - guide launches on large PRs (layerPatchIncomplete) recompute changedFiles from the local checkout (git diff --numstat origin/<base>...HEAD, rename/binary handling) when available — the PR-mode prompt tells the agent to read that full local diff, but the changed-files block and validation snapshot came from the truncated platform patch, under-listing files to the model and then dropping its valid refs (or failing the guide) at validation. Falls back to the partial list on any failure. Mirrored in the Pi extension server. Round-12's P1 (Pi marker extraction reads message_end, 'should' read text_delta) was refuted by live probe: Pi's assistant message_end carries the fully-assembled content blocks, which is exactly what piExtractText reads; the delta-reading code the finding cross- referenced is the Ask-AI STREAMING provider, a different job. * test(guide): first direct coverage for guide-review's pure logic (self-review) The module's repair ladder, validation, and stream parsing had zero unit tests — exercised only end-to-end through live agent runs. Pins the behaviors the review rounds fixed: blank-title fallback (round 12), first-placement-wins dedup, changedFiles filtering + fail-closed empty guides, prose-only vs lost-diffs section handling, unplacedFiles merge/dedup/fabrication-filtering, title/intent coercion, trailing- comma and unbalanced-bracket repair, truncated stream-line recovery. 13 tests; server suite 374 → 387. * fix(guide): PR-993 round-13 — full-stack recompute regression, flag snapshot, unplaced reveal - the round-12 large-PR recompute now runs ONLY in layer scope: in full-stack scope launchPatch is already a local full recompute, and the layer diff (origin/<base>...HEAD) was the WRONG file set — it omits earlier stack layers' files, dropping their refs at validation or failing the guide. Introduced by round 12; caught before any release. Both servers. - layerPatchIncomplete is snapshotted with the other launch locals (launchLayerPatchIncomplete): coherence with launchPatch was positional (same sync segment) — now structural, so a future await inserted upstream can't silently desync them. Both servers. - the 'Everything else' bucket handles guideRevealFile: sidebar jumps to unplaced files now focus + scroll their diff (the section-card effect only covers placed files) — closes the residual gap noted in the round-11 self-review	2026-07-04 10:23:59 -07:00
Michael Ramos	e8df06db7c	feat(review): Commits panel — linear history rail with per-commit diffs (#994) ... * docs(adr): spec for the commit-list review view (linear history rail) * feat(review): commit:<sha> diff mode along the shared diff-type seams A new git-only diff family for reviewing one historical commit against its first parent (git-show style), threaded through every seam a diff type crosses: - runGitDiff: git diff <sha>^ <sha>; a root commit diffs against the empty tree. Label is 'Commit <shortsha> — <subject>'. The sha is validated as bare hex (parseCommitDiffType) before it reaches any argv position, on top of --end-of-options. - getGitDiffFingerprint: anchored to the sha alone (present/gone), NOT headSha — new commits landing mid-review don't change this diff and must not raise the staleness banner. A vanished commit (rebase + gc) flips present→gone and fires it honestly. - getFileContentsForDiff: old side <sha>^ (null on root), new side <sha>. - parseWorktreeDiffType learns worktree:<path>:commit:<sha> (the sub-type contains a colon, so it needs its own split) — commit review composes with worktree sessions like every other mode. - vcs-core: the git provider owns commit:*; staging stays gated off (canStageFiles allowlist unchanged — nothing in a historical commit is stageable). - Ask AI context: a commit-mode inspect string ('git show <sha>', with an explicit 'historical commit, not the working tree' note). Also adds listCommitHistory for the upcoming /api/commits endpoint: one --first-parent page from HEAD (before=<sha> continues at its first parent, +1 fetch for an honest hasMore), with isHead / isRepoUser / isPastBase flags. isPastBase is reachability from the base — computed as the complement of 'rev-list --first-parent HEAD ^base', which is a prefix of the walk, so the client's single divider is exhaustive. An unresolvable base degrades to no divider (matches since-base's posture on such repos). * feat(review): GET /api/commits — linear history endpoint (Bun + Pi) One page of the branch's --first-parent history (?limit=&before=), served by listCommitHistory against the active diff's cwd (worktree- aware via resolveVcsCwd) and the active base (so the divider tracks the same baseline the review compares against, including the startup origin/<default> upgrade). Gated to plain local git sessions — PR / workspace / jj / p4 return 400, mirroring the client's commitsCapable gate. Both runtimes; the route-parity test covers the pair. commit:<sha> switching needs no endpoint changes: /api/diff/switch already dispatches through runVcsDiff (the git provider owns the new family), the epoch guard applies as-is, the sections sidecar correctly stays since-base-only, staging 400s via the canStageFiles allowlist, and baseRelevantDiffType keeps the behind-GitHub banner suppressed for commit diffs. * feat(ui): Commits panel view — the linear history rail Third left-panel view: 'Git status | Commits | Tree'. The Commits view is a pure commit list (short sha, subject, relative age compacted to '2h', author only when it isn't the repo user, HEAD dot, a divider where the branch meets the resolved base, 'Show more' paging). It never becomes a file list: clicking a commit switches the diff to commit:<sha> and the existing needsInitialDiffPanel flow lands the center dock on the all-files surface; re-clicking the active commit just re-focuses that panel. Persistence: reviewPanelView gains 'commits' with NO diff coupling — the view persists, the diff opens on the user's normal default until a commit is clicked, and a sha is never persisted (it may not survive a rebase). The sections⟺since-base coupling is untouched, but the classic-diff→Tree snap in Settings/ReviewSetupDialog now only fires when the current view is 'sections', so picking a default diff no longer stomps a Commits preference. The load-time self-heal ignores 'commits' by construction (it keys on panelView === 'sections'). Gates: commitsCapable = plain local git session (no PR / workspace / jj / p4, matching the server's /api/commits gate); the segment is hidden elsewhere. Staging is inert in commit diffs on both ends (STAGEABLE_DIFF_TYPES and the server allowlist never match commit:*). Worktree sessions compose — the client parses worktree:<path>:commit:<sha> and handleDiffSwitch prefixes as usual; the rail refetches on worktree/base changes but deliberately not on commit clicks, so paging state survives selection. Accepted edges (v1): no j/k selection in the rail (each move would run a full diff switch); toggling Commits→Tree keeps a commit diff active, where the tree's diff-type dropdown marks nothing — the gitRef header still names the commit. * fix(review): self-review cleanups on the commit-list feature - listCommitHistory: a repo with no commits yet (unborn HEAD) returns an empty page instead of null — the panel showed a 500-backed 'Retry' error where 'No commits' is the truth. Every other review surface already degrades gracefully on a commit-less repo; now this one does. Only a genuinely unanswerable repo (not a repo at all) stays null. - Hoist the bare-hex sha rule into one BARE_HEX_SHA_RE shared by parseCommitDiffType and the before-cursor validation (was written twice). - The 'all' diff case now uses the getEmptyTreeSha helper the commit mode extracted, instead of keeping its own inline copy. - Drop CommitsPanel's isLoadingDiff prop — declared and passed but never used. * feat(ui): commit cards, labeled groups, avatars — review round 1 Four items from the live review round: - Toggle: Commits moves to the far right (Git status | Tree | Commits) and the segment text goes 10px → 12px (padding trimmed to keep three segments inside the 256px header). - The bare '── origin/main ──' divider confused more than it explained. Replaced with two labeled groups: 'On this branch' (commits not yet reachable from the base) and 'In <base>' (shared history), each with a tooltip spelling out what the boundary means. - Rows become overview cards: subject (2-line clamp) on top; a meta row with author avatar, author name (only when it isn't the repo user), a HEAD badge, short sha, and compact age. Active card gets the primary border/tint. - Author avatars, reusing the PR machinery rather than reinventing it: parseRemoteUrl/parseRemoteHost classify the origin forge; the gh/glab invocation shape (incl. the --hostname self-hosted convention) and GitLab's relative-avatar absolutization rule are mirrored from pr-github/pr-gitlab. What could NOT be reused directly: PR avatars are keyed by platform login, which local commits don't have — so GitHub resolves via one repos/{o}/{r}/commits?per_page=100 call per session building an author-EMAIL → avatar map (unpushed commits by the same author resolve through it; verified live), and GitLab uses its per-email /avatar endpoint (capped, deduped). Everything is memoized per session and fails closed to the initials fallback — a repo with no remote, an opaque self-hosted host, or an unauthenticated CLI never delays or errors the commits endpoint. The Avatar component moves out of PRCommentsTab into its own module, now shared by the PR timeline and the commit cards. New shared module commit-avatars.ts is vendored to Pi (vendor.sh) and the enrichment runs in both runtimes. CommitListEntry gains authorEmail (resolver key) and avatarUrl (server-enriched). * ui(review): flush commit rows, prominent base boundary — review round 2 - Cards are gone: rows stack flush (no borders, no margins), hover tint only. Subject is strictly one line, ellipsized (no clamp/wrap). - Author name always shown (was: only when it differed from the repo user), with the avatar, on the compact meta line under the subject. - The base boundary is a prominent labeled rule ('— In origin/main —', foreground-weight lines) instead of the faint border under a group header; the 'On this branch' header above stays quiet. * docs(adr): evaluate local stack-parent detection — scenario matrix, no build decision Empirical stress-test of the merge-base-reachability algorithm across 9 scenarios plus this repo itself. Findings: the stacked-ness test is graph truth (never lied); parent attribution has one content-harmless failure (same-merge-base sibling labels) and one dangerous, provably graph- undetectable one (branches pointing inside B's own history win and silently hide the user's commits). Determination: only viable as a suggest-and-confirm affordance with per-branch persisted choice — never silent auto-defaulting. Decision to build deliberately left open. * fix(ui): Commits view is session-only — never the opening view Live-review catch: the panel toggle inherited the sections/tree 'last choice becomes the default' cookie write, so clicking Commits in one session made the NEXT review open on the history rail instead of the diff. Ruling: a review always opens on the persisted sections/tree default; Commits is entered via the toggle each session. - reviewPanelView narrows back to 'sections' | 'tree'; a stale 'commits' cookie is treated as unset (self-heals to the default). - The live view is now a session overlay in App state: selecting Commits flips it without touching config; selecting Git status/Tree clears it and persists as before. Single choke point in selectPanelView, so every toggle site inherits the rule. - Settings loses the Commits segment (it configures the OPENING view, which Commits can no longer be). Spec updated with the revision. * ui(review): the panel toggle is session-only — it never writes the default Second live-review ruling on persistence: the header toggle shouldn't persist ANY choice, not just Commits. Looking at another view mid-review must not silently change what the next review opens on. - selectPanelView becomes a pure session override (local state layered over the persisted value); no configStore writes from the toggle path. - handleSwitchToSections drops its defaultDiffType write — that write only existed to keep the persisted view/diff pair consistent, and the toggle no longer persists a view. Settings and the setup dialog remain the only reviewPanelView/defaultDiffType writers and keep enforcing the sections ⟺ since-base coupling; the load-time self-heal still repairs conflicted pairs those writers may leave. - Spec + settings-registry comments updated to name the new rule. * feat(ui): commit description card + collapsed all-files for commit diffs Clicking a commit now answers 'what and why' before 'which lines': - New commitInfo sidecar (Bun + Pi, same mode-conditional shape as sections): when a commit:<sha> diff is active, /api/diff and /api/diff/switch carry the commit's full metadata — subject, multiline body, author (+ avatar via the session resolver), sha, age — from a single 'git show -s' (getCommitDiffInfo in review-core, body as the trailing format field so newlines survive). - CommitDescriptionHeader heads the all-files surface: subject, author row, and the full body rendered as markdown via the same MarkdownBody the PR viewer uses. Long bodies scroll inside the card (max-h) so the diff keeps the viewport. - Commit diffs open FOLDED: AllFilesCodeView gains defaultCollapsed — items seed collapsed at build time, the seed is part of fileSetKey (CodeView seeds once per instance, so a seed change must remount), and the collapse-all toggle state initializes to match. Every other diff mode keeps opening expanded; leaving the commit diff clears the card and the folded default together (the sidecar is absent). Verified live through the compiled binary: switching to a real commit returns subject/body/avatar, switching back to since-base clears it. * ui(review): commit description scrolls with the diff — no pinned card Round feedback: the fixed header with its own inner scrollbar felt embedded; the description should read as the top of the document and scroll away with the files. - AllFilesCodeView gains leadingContent: rendered via a portal INTO CodeView's scroll container (absolutely positioned at content top, so it participates in the scrollable overflow), with its measured height fed into layout.paddingTop so the virtualized items start below it. CodeView stays the single scroll authority — no nested scrollers, no wrapper flex row. - CommitDescriptionHeader shows the full body inline (no inner OverlayScrollArea). Only genuinely huge bodies (>24 lines / >1800 chars) get a CSS max-height clamp with a fade mask and a Show more/Show less toggle — CSS clamping keeps the markdown intact, and the ResizeObserver feeds the expanded height back into paddingTop. Keyed by sha so the toggle resets per commit. * ui(review): entering the Commits view auto-opens the HEAD commit Toggling to Commits used to leave the previous mode's all-files diff on screen — a rail full of commits next to content that belonged to none of them. Now entering the view selects the HEAD commit (top of the rail) so the center immediately shows its diff + description card. Once per entry, ref-guarded: an already-active commit diff survives a toggle round-trip, a user click supersedes it, and a failed switch doesn't retry-loop. When entry races the first log fetch, the effect fires as soon as the log lands. * perf/ui(review): fast commit navigation — instant veil + skip context recompute Rail clicks felt laggy and jumpy: the old diff sat on screen for the whole switch round-trip and then snapped, and the switch itself was doing a full getVcsContext recompute (branch + worktree + recent-commit enumeration — three git walks) that a commit click can never invalidate. - Same-cwd commit:<sha> switches skip the context recompute in both runtimes (the client keeps its existing context when the field is absent — long-standing contract). Measured through the compiled binary: commit switches now ~40-60ms server-side. - The center dock shows an immediate 'Loading commit…' veil whenever a commit switch is in flight (or the Commits view was just entered and HEAD auto-select hasn't landed) — the stale previous diff never shows and the click reads as instant. Errors surface through the normal empty-state, never trapped under the veil. * fix(ui): PR-994 review round — toggle/settings separation + commit-nav fixes (App) The headline fix: the load-time settings repair guarded on the live panelView, which now carries the session toggle override — so clicking 'Git status' once mid-review (saved prefs tree + a classic diff) hit the healer and silently persisted defaultDiffType='since-base'. The toggle must never be a settings writer, directly or through a repair path. Keyed to persistedPanelView now; audited the remaining configStore.set call sites — no toggle path can reach a write. Also in this file, same review round: - Commit-navigation veil gains a real predicate: it drops on a commit-log fetch error (rail shows Retry; center was stuck under the spinner forever) AND on a genuinely empty history (zero commits — the second stuck-veil trigger the automated review missed), while still covering the log-loading and pre-auto-select frames. - handleSelectCommit composes the worktree prefix once and calls fetchDiffSwitch directly (the equality check and the switch previously used two different composition paths). - commitsCapable/showCommitsPanel hoisted above the global keyboard handler; Cmd+F is no longer intercepted in the Commits view (no search input exists there — the browser find takes over instead of a silent no-op). * fix(review): PR-994 review round — commit-log staleness, loading flags, avatar cap - useCommitLog clears its cached list when the history context changes (worktree/base switched while the view was away): the HEAD auto-select acted on the stale rows and opened the previous context's commit. Same contextKey still keeps the cache (no empty flash on re-entry). - The hook's cleanup now resets both loading flags: a generation-skipped finally never cleared them, leaving 'Show more' stuck disabled as 'Loading…' after leaving mid-page. - commit-avatars memoizes misses only for emails an attempt actually covered — the GitLab per-call cap (10) was recording every email past it as permanently unresolvable for the session. Regression test added. - GitLab host detection tightened from a bare substring to a structured match (gitlab.com / gitlab.* / *.gitlab.*) — 'mygitlabproxy.example.com' no longer classifies. - Collapse-all mirror re-derives from live item state after per-file toggles: with commit diffs seeding all-collapsed, expanding one file left the dock button on 'Expand all', and clicking it re-collapsed the file the user just opened. - The commit description card element is memoized on commitInfo identity so the measuring ResizeObserver stops churning on every context render. * fix(ui): entering the Commits view ends any open search session (self-review) The Cmd+F guard from the review round only blocked NEW searches in the Commits view; a search opened in Git status/Tree survived the toggle as hidden-but-live state — the query kept matching against the commit diff, marks kept rendering in the all-files body, Enter/F3 kept stepping matches, and the input to see or edit the query didn't exist anywhere on screen. handlePanelViewSelect (the single choke point every toggle site routes through) now clears and closes search on entry to Commits. Also re-traced the rest of the round under self-review — veil terminal states (log error/empty/loaded, switch failure, background refresh with a commit active), the hook's cleanup-before-rerun flag ordering, the avatar attempted-set on broken platforms, and both collapse-mirror paths — no further findings. * fix(ui): PR-994 round 2 — live rail freshness, non-destructive errors - The rail now keeps itself fresh: while the Commits view is visible, a quiet 10s poll head-compares page 1 and adopts it only when history actually moved. The commit DIFF's sha-anchored fingerprint stays as designed (a historical commit never goes stale, so the banner correctly stays quiet) — but the rail no longer freezes while an agent commits in the background. Adoption bumps the fetch generation so an in-flight 'Show more' from the old history can't append stale rows, and transient poll failures disturb nothing. - A page-1 fetch resets isLoadingMore: a refresh superseding an in-flight paging request skipped that request's generation-guarded finally, leaving 'Show more' stuck on 'Loading…' — rare before, a live race once the poll exists. - Errors are non-destructive when a list is on screen: a failed page or background refresh renders as an inline row with Retry under the list instead of replacing the whole rail; the full-panel error state is reserved for an empty rail. * perf/refactor(review): parallel GitLab avatar lookups; drop dead isRepoUser - The ≤10 per-call GitLab avatar lookups run in parallel instead of sequentially — they sit on /api/commits' critical path, and serial subprocess spawns added seconds to the rail's first paint on multi-author GitLab histories (PR-994 round 2 nit). - CommitListEntry.isRepoUser removed: v1 showed the author only when it wasn't the repo user; the live-review pivot to always-shown names left the field computed (one git config subprocess per page), serialized, and tested but never read, with a doc comment describing behavior the panel no longer has. Spec updated to match the revised row design. * fix(ui): commit-poll adoption clears all superseded fetch state (self-review) Adopting a new history from the background poll bumps the generation, which strands any in-flight fetch's generation-guarded finally — the same stale-flag class just fixed for isLoadingMore, but for isLoading (slow first load overtaken by the poll) — and a lingering inline error from the replaced history would otherwise sit under the fresh list. Adoption now clears isLoading, isLoadingMore, and error together. * fix(ui)/docs: PR-994 round 3 — worktree switches drop commit diffs; API docs - handleWorktreeSwitch treats commit:<sha> as non-portable: every other diff mode recomputes meaningfully against the target worktree, but a commit diff is context-bound content (worktrees share one object database — the reviewer's claimed git error doesn't exist, verified empirically), so 'preserving' it just re-rendered the OLD context's commit byte-for-byte. It now falls back to the session default (same option-availability rule resolveInitialDiffType applies). Swept every other activeDiffBase/diffType carrier: the whitespace toggle, staleness refresh, and fetch-base deliberately recompute the SAME diff (valid for commits); handleBaseSelect and the base-picker row already exclude commit mode; job diff-context labeling and the guarded diff-type dropdown are display-only. This was the single leak. - AGENTS.md (CLAUDE.md symlink): /api/commits row added to the Review Server table; /api/diff and /api/diff/switch now document the commitInfo sidecar and the commit:<sha> diffType family; the since-main section describes the three-segment session-only toggle and the never-persisted Commits view. * docs: normalize arrow glyphs in the /api/commits table row (self-review) * fix/refactor(review): PR-994 round 4 — rebase-safe paging, locale-proof ages, shared parsing - Pagination is rebase-safe: a 'Show more' cursor from a history that was rewritten mid-session (rebase/force-push) still resolves in the object store but is no longer on the branch — paging from it walked the orphaned pre-rewrite chain for the ≤10s window before the freshness poll adopts the new history. listCommitHistory now ancestor-checks the cursor (merge-base --is-ancestor) and returns an empty terminal page for orphaned AND vanished cursors alike; the poll replaces the list moments later. Regression test rewrites history between pages. - Ages are locale-proof: git localizes %cr via gettext ('vor 2 Stunden'), which the English-only compactAge regex silently couldn't shorten. CommitListEntry/CommitDiffInfo now carry committedAt (epoch ms, %ct); the rail and description card format it with the existing formatRelativeTime — compactAge and the third formatter variant are gone. - The %x1f over-split repair (fixed head/tail fields, rejoined free-text middle) was triplicated across listRecentCommits, listCommitHistory, and getCommitDiffInfo; one splitCommitFormatFields helper (head/tail counts, tail=0 covers the multiline-body shape) owns the edge case. - Avatar resets its broken-image state when src changes — latent today (every caller keys by identity), but it's a shared component and must be safe for callers that update src in place. Left alone from this round: hashString/hashFingerprintPart duplication is pre-existing, and the suggested cross-package import would drag node:path into the browser bundle — the documented reason client mirrors exist. * fix(review): PR-994 round 5 — merge-commit prompts, boundary-aware poll, auto-select guard - Agent prompts for commit:<sha> diffs instruct the exact on-screen command: git diff <sha>^ <sha> (first parent), explicitly steering away from git show — whose combined-diff presentation for a MERGE commit renders a different (often empty) changeset than the one under review. Root-commit fallback noted. One site covers Ask AI and every launched reviewer (and Pi via vendoring); prompt test pins the command. - The rail's freshness poll now adopts on boundary/base movement, not just a new head: an agent running git fetch advances origin/<base> while HEAD stays put, which re-partitions isPastBase — previously the head-only compare skipped adoption and the 'In origin/main' divider stayed stale until the view was reopened. Boundary is compared over the page-1 overlap window (a divider paged deeper than the probe can see re-syncs on the next full reload — accepted micro-edge). The reviewer's proposed trigger (the Fetch banner) is unreachable in commit mode; the external-fetch trigger is the real one. - The HEAD auto-select never fires while any diff switch is in flight — hardening only: the claimed first-click overwrite isn't reachable (isLoadingDiff was never a dependency of that effect, and auto-select settles before a human can click), but the guard makes the invariant explicit. * docs(adr): spec the pre-merge commits-view structure refactor Three behavior-preserving moves scoped for PR #994 before merge, motivated by the audit finding that every recent review-round bug lived at the App.tsx <-> useCommitLog seam: (R1) unify the commits-session state machine (poll, auto-select, veil) in one hook; (R2) lift the commit-rail block out of the 1.6k-line review-core into commit-history.ts; (R3) share the isSameCwdCommitSwitch predicate both runtimes inlined. Ownership tables name what deliberately stays put; execution runs smallest-first with full gates per commit. * refactor(review): share isSameCwdCommitSwitch across runtimes (spec R3) The ~10-line parse-and-compare predicate both /api/diff/switch handlers inlined moves to review-core beside its inputs (parseWorktreeDiffType / parseCommitDiffType — the canonical home for diff-type string logic); both call sites collapse to one line. Unit tests cover plain, worktree, cross-worktree, and non-commit-target cases. Behavior identical. * refactor(shared): lift the commit-rail block into commit-history.ts (spec R2) review-core.ts (1.6k lines) was absorbing the ~230-line Commits-panel data layer it doesn't need to own: CommitListEntry/CommitHistoryPage/ listCommitHistory and CommitDiffInfo/getCommitDiffInfo move verbatim to a new commit-history.ts, matching the package's per-concept split (jj-core, pr-stack, commit-avatars). The commit:<sha> DIFF plumbing (parseCommitDiffType, the runGitDiff/fingerprint/file-contents cases) stays in review-core with the other diff types — it participates in the dispatch; the rail does not. Wiring: review-core exports its three parsing primitives (COMMIT_FIELD_ SEP, splitCommitFormatFields, BARE_HEX_SHA_RE — listRecentCommits still uses them, so they can't move); package exports + vendor.sh gain the module (vendored siblings import relatively, same as pr-provider → pr-github); types.ts re-exports split by source; both runtimes import from the new module. History/metadata test suites move to commit-history.test.ts with the package-style per-file git harness; commit-DIFF tests stay in review-core.test.ts. Behavior identical — verified live through the compiled binary (/api/commits + commit switch + commitInfo sidecar). * refactor(ui): useCommitsView owns the whole commits-session machine (spec R1) The list cache, freshness poll, HEAD auto-select, and center-dock veil were split between useCommitLog and App.tsx — and every sync bug found across three review rounds (stuck flags, stale auto-select, adoption races) lived at exactly that seam. The auto-select effect and the veil derivation move into the hook verbatim (renamed useCommitsView, git mv), so the machine's invariants are locally checkable in one file. App now supplies only what it owns — visibility, the active commit, switch state, and onOpenCommit (its handleSelectCommit: the SAME path user clicks take, so auto- and user-selection cannot diverge by construction) — and consumes panel props plus veilActive. Net ~45 lines of App.tsx's most delicate logic deleted. One equivalence note: the hook's single "enabled" (showCommitsPanel && origin) now gates auto-select/veil where App used bare showCommitsPanel; the two only differ when origin is unset, which implies no gitContext (demo mode) and therefore showCommitsPanel === false — identical in all reachable states. Deliberately still in App (each guards a flow App owns): capability gating, handleSelectCommit, the Cmd+F / worktree-fallback / staleness-refresh / search-clear guards, and render wiring. Behavior identical; spec: adr/specs/refactor-commits-view-structure-20260703.md.	2026-07-04 10:13:11 -07:00
Michael Ramos	795f381ebe	feat(review): "All changes" git-status review view (#990) ... * feat(review): "Since main" git-status review view + correctness fixes Adds a composite `since-base` diff — merge-base(origin/main, HEAD) vs the working tree, plus untracked — as the default code-review view, rendered as a three-section "git status" panel (Committed / Changes / Untracked) with a Sections|Tree toggle, a first-run setup chooser, and a "baseline behind GitHub" fetch banner. Everything normalizes to one git patch, so the diff viewer, annotations, and feedback path are unchanged. New diff type wired through runGitDiff / fingerprint / file-content / context / staging / agent-context in packages/shared + both server runtimes (Bun + Pi mirror). Default flipped from `unstaged` to `since-base`; the old DiffTypeSetupDialog is replaced by ReviewSetupDialog (view + default-diff chooser, screenshots, Settings access). Includes a reviewed batch of correctness fixes: - P0: baseBehindRemote was permanently true (rev-parse missing --verify) - fingerprint blind to quoted/unicode untracked paths (unquote) - sections parser: record both sides of a rename; staged-delete wins over untracked (rm --cached collision) - graceful degrade when merge-base can't resolve (trunk/no-remote repos) - /api/fetch-base re-queries remote (narrow-refspec honesty) - /api/diff/switch concurrency guard (diffSwitchEpoch) + draft rekey - decouple remote-staleness probe from initialBase (Pi parity) - keyboard file nav in the sections view; banner gated to base-relative modes See adr/decisions/005-since-base-github-view-default-20260701-223706.md * fix(review): since-base review-round hardening (11 fixes, both runtimes) Addresses the multi-agent + PR-990 review findings: - Sections rename parser split ` -> ` on the raw quoted porcelain token, so a filename containing ` -> ` tore into garbage and a dirty file could render as "Committed". Now quote-aware (splitPorcelainRename + unit tests). - /api/diff/switch captured the epoch AFTER await req.json(), letting a slow-body older switch overwrite a newer confirmed one. Epoch now captured before any await; hideWhitespace committed only on win. Both runtimes. - Unresolvable base (trunk / no origin/HEAD) no longer auto-defaults to a degraded since-base that hides committed work — getGitContext only offers since-base when the base ref resolves, so the default falls through to uncommitted. Fingerprint + file-content degrade to HEAD to match the diff. - git rm --cached no longer yields two diff entries for one path (untracked files already in the tracked patch are dropped) — fixes wrong-file-open and j/k nav looping in the Sections panel; also fixes latent uncommitted/unstaged. - Settings' "Default Diff View" list now preserves the sections<->since-base coupling (can't leave an invalid sections+classic pair). - "Behind GitHub" banner only treats origin/* as fetchable; a bare local base ("main") is upgraded to its tracking ref at startup so Fetch can clear it. - hashUntracked resolves untracked paths against the repo toplevel, not cwd, so a review launched from a subdirectory isn't blind to untracked edits. - Agent review instruction now tells agents to enumerate/inspect untracked files. - Terminology: one "Committed changes" label; the live "Since <base>" label is dynamic (matches the header); "Since main" kept only as product copy. - Docs: CLAUDE.md endpoints/fields + since-base view section; ADR recap. Verified: typecheck (all projects), bun test 1793 pass, and empirical repro of the rm --cached dedup, trunk->uncommitted fallback, and rename parser. * fix(review): restore "(PR view)" on the Committed-changes label (unify toward it, not away) * fix(review): close since-base coverage gaps from PR-990 review round 2 - First-run setup no longer forces since-base on repos where it isn't available (base ref can't resolve). getGitContext omits since-base there; the first-run block now checks the same availability before resetting the default or showing the chooser, so committed work isn't silently hidden on trunk/no-origin repos. - reviewBase now includes 'since-base', so changing the base while in the git-status view passes the selected base to /api/file-content — expandable context is fetched from the right merge-base (was falling back to default). - Settings now mirror ReviewSetupDialog's coupling exactly: Sections ⇒ force since-base; Tree ⇒ leave the diff (Tree + since-base is valid and now saveable); classic diff ⇒ Tree; since-base diff ⇒ leave the view. Removes both coercions that previously discarded a supported preference. * fix(review): PR-990 review round 3 — fingerprint/dedup/base-canonicalization - Fingerprint now uses `git status --porcelain -uall`, so editing a file inside a brand-new untracked directory changes the fingerprint and the "Diff out of date" banner fires (was collapsed to `?? dir/` and hashed as unreadable). - extractTrackedPatchPaths pairs `---`/`+++` and excludes only the file's KEYED path (new side, or old side for a pure deletion) — no longer drops a recreated rename-source file (`git mv a b && touch a`). Verified rm --cached still dedups to one entry and the recreated file still shows. - resolveReviewBase canonicalizes a bare local default name ("main") to its tracking ref ("origin/main") on every call, so a client that loaded before the startup upgrade resolved can't revert the server to the stale local base on the next refresh/switch. Both runtimes (new Pi resolveReviewBase helper). - Removed the dead activeDiffLabel prop chain (FileTree -> DiffTypePicker). - ADR recap: documented the staleness-banner scope limitation (default base only). Verified: typecheck (all projects), bun test 23/23 review-core+fingerprint pass, empirical repro of the untracked-dir fingerprint, rm --cached dedup, and rename-recreate cases. * fix(review): PR-990 review round 4 — dedup content, staging gate, banner timing - Dedup rewrite: instead of dropping the untracked side of a same-path collision (which hid content), drop the tracked DELETION block and keep the untracked working-tree content. `git rm --cached f` + edit now shows the new content, not a phantom deletion — still one entry per path (no dock/nav collision). New stripHeaderPath also strips git's trailing-tab metadata, so space-named files dedup correctly. Verified: rm --cached+modify shows content (1 entry), space-named (1 entry), rename+recreate still shows both. - All-files (and the `a` shortcut on the focused file) now gate staging per-file: committed files in since-base mode are not stageable, matching SectionsPanel / FileTreeNode. Shared isPathStageable helper threaded via ReviewStateContext → AllFilesCodeView. Stops the confusing `git add` no-op that still flipped local staged/viewed state. - /api/diff/switch now awaits recomputeBaseBehindRemote before building the response, so the "Baseline behind GitHub" banner reflects the new base immediately (no ~5s lag switching in, no stale banner switching away). Both runtimes. - ReviewSetupDialog re-applies the recommended default only on first-run dismiss, not when reopened from the header menu (was snapping a mid-session diff back). - SectionsPanel "N added" header now counts sidecar-staged files too, so it matches the staged dots on rows. Verified: typecheck (all projects), bun test 1830 pass, empirical dedup repro, both bundles build. * refactor(review): reuse parsePatchPathToken in removeTrackedDeletions (self-review) Drop the duplicated stripHeaderPath helper — the shared diff-paths parsePatchPathToken already strips a/|b/ prefix + C-quoting + git trailing-tab, and verifies the prefix instead of blindly slicing two chars. Documents the binary-deletion edge (no --- line, so a binary rm --cached is not deduped). * fix(review): PR-990 review round 5 — mixed-base + staging/sort/flicker nits - Atomic base upgrade: the startup origin/* canonicalization swapped currentBase without rebuilding the patch, so /api/diff could advertise origin/main while the served hunks came from local main (mixed-base review on origin/HEAD-absent repos). Now rebuilds the diff for the new base and commits base+patch+ref+ fingerprint together (only if no user switch happened); the fingerprint change makes the client's freshness poll pick it up. Both runtimes. - isPathStageable: gate staging OFF when since-base is active but the sidecar hasn't loaded (was falling through to true, allowing a git-add no-op on a committed file). - SectionsPanel: session-staged files now float to the top of Changes — the sort key was `false ?? stagedFiles.has(...)` which short-circuits; now ORs them. - /api/diff/fresh: early returns now carry baseBehindRemote, so a snapshot change mid-probe no longer clears the "behind GitHub" banner for one poll. Both runtimes. - Removed the now-dead activeLabelFallback prop from DiffTypePicker (FileTree stopped forwarding it in round 3). Skipped (agreed): the fetch-base input-validation nit (no realistic attacker) and the diff-switch TOCTOU (concurrent switches aren't UI-reachable). Verified: typecheck (all projects), bun test 1830 pass, both bundles build. * refactor(review): drop redundant staged-OR in SectionsRow (self-review) item.staged (the grouping key) already ORs in stagedFiles as of the round-5 sort fix, so the row-level `|| stagedFiles.has(...)` is dead weight. * fix(review): surface the startup base upgrade to already-loaded clients Review round 6 fixes: - The startup main -> origin/main upgrade re-baselined the freshness fingerprint, so a client that fetched /api/diff before the rebuild kept the old patch and every /api/diff/fresh probe reported fresh (the probe compares server state to itself, never to what the client renders). Now the fingerprint is only re-baselined when no client has loaded the pre-upgrade snapshot; otherwise the stale baseline trips the normal "Diff out of date - Refresh" banner. Bun + Pi. - preserveFile refreshes (staleness Refresh, post-Fetch) now adopt the server's returned base — exactly the paths where the server may have canonicalized main -> origin/main; keeping the old name sent /api/file-content and Ask AI context against the wrong base. - ReviewSetupDialog: clamp the fixed 800px height to the viewport so the dialog fits on small laptop screens. - Docs: /api/diff/fresh response also carries baseBehindRemote/agentCwd. - Documented the committed-deletion + untracked-recreation dedupe edge as accepted (code comment + ADR) — fixing it needs two same-path diff entries, which the path-keyed UI cannot represent. * fix(review): stop the Git-status default from silently reverting to tree Users with reviewPanelView=sections could keep opening in the tree view on a stale diff type despite their cookie saying Git status. Three causes: - The header Sections toggle persisted the view but not defaultDiffType, creating a conflicted pair (sections + non-since-base default) that every UI writer is supposed to prevent. It now couples the diff default like the setup dialog and Settings do. - configStore's debounced POST /api/config could be lost when a session closed within 300ms of a change, leaving cookie and config.json split. Pending writes now flush on pagehide with a keepalive fetch. - configStore.init() applies config.json over the cookie without the UI coupling, so a stale server value re-corrupted the pair on every load. The app now self-heals at load: if the view says sections but the diff default isn't since-base, it repairs the default (cookie + config.json) and switches the live session to since-base. * fix(ui): keep the stage (+) button border visible on the active file row The button's --border border has no contrast against the active row's 30% primary tint, so it vanished on the selected row until hovered. Tint the border with the row's primary color on active rows (both sections and tree views); the button's own hover border still applies. * ui(review): spell out the panel view toggle — 'Git status | Tree' text instead of icons * fix(review): PR-990 review round 7 — export label, diff snapshot race, unicode paths - exportFeedback describeDiff(): add the missing since-base case — every feedback export in the new default mode read "**Diff:** since-base". - /api/diff GET: snapshot patch/base/ref/error BEFORE the sections-sidecar await and pass the pinned base into buildSectionsSidecar. The startup base upgrade landing mid-await could pair a rebuilt patch with sections grouped against the old base, with initialDiffServed still false so no refresh banner ever came. Bun + Pi. - SectionsPanel: a session-staged untracked file now moves to the Changes section immediately (anticipating the server's next sidecar) instead of sitting in Untracked with a staged dot until refresh. - unquoteGitPath: real C-style unquoting with octal (UTF-8 byte) escape decoding. JSON.parse rejects octal escapes, so non-ASCII names kept their literal \303\251 form — an untracked "café.txt" was silently ABSENT from the review (git diff --no-index could not access the quoted name) and the deletion dedupe Set lookup could never match. getUntrackedFileDiffs now unquotes ls-files output. Unit tests: octal decoding + end-to-end unicode untracked file in a real repo. - review-core.test: worktree subtype round-trip now covers since-base and all (was 6 of 8). Parked (deliberate): SectionsPanel/FileTree keyboard-nav dedup refactor. * fix(shared): unquoteGitPath keeps literal unicode intact (self-review) The byte-collector treated literal non-ASCII code units as single bytes, which would mojibake headers synthesized by our own quoteGitPath (JSON.stringify leaves unicode unescaped inside quotes — workspace-mode prefixed headers round-trip through the same parser). Literal chars now append as string code units (surrogate-safe); only octal escapes go through the UTF-8 byte decoder. * fix(review): PR-990 review round 8 — pre-staged toggles, explicit base, AI context race - useGitAdd: session Set replaced with a tri-state override map folded over the sections sidecar; stagedFiles is now the EFFECTIVE staged set (sidecar + session stages - session unstages). Fixes pre-staged files: the first `a` press actually unstages (was a git-add no-op), the sidebar dot clears after a real unstage (was stuck via sidecar OR), and the All-files header agrees with the sidebar on load. Overrides reset whenever a fresh sidecar arrives (switch, preserveFile refresh, PR response) so stale session intent can't fight new porcelain truth. SectionsPanel drops its own sidecar OR; stagedCount = effective size. - Explicit base picks are honored verbatim: the picker sends explicitBase, and the server permanently disables local-name -> origin/* canonicalization once set (the local/remote groups are distinct choices). The behind-GitHub banner also exempts an explicitly-picked local name — Fetch advances origin/*, so the banner would be un-clearable nagging. Bun + Pi. - buildCurrentAiReviewContext(patch, base): GET /api/diff builds Ask AI context from the same served snapshot as the patch — the startup base upgrade could hand Ask AI a different changeset than the screen. Bun + Pi. - recomputeBaseBehindRemote: capture remoteDefaultInfo once — a concurrent refresh nulling it mid-await threw. Bun + Pi. - Revert stray "Status Update / Updated!" edit to adr/0001 (test debris swept into the round-2 commit). - splitPorcelainRename comment corrected (porcelain v1 does NOT quote plain spaces; a name containing " -> " is ambiguous without -z) + ADR notes for the index-only-changes semantics and the rename edge. * fix(review): FileTreeNode uses the effective staged set — round 9 Round 8 made stagedFiles the effective set (sidecar + session overrides) and removed SectionsPanel's sidecar OR, but missed the same OR in FileTreeNode's since-base row (sectionStaged). In the Tree fallback a pre-staged file unstaged this session kept its dot and the next toggle re-staged it. Grep-swept: this was the last surviving sidecar-staged OR. * refactor(review): make the staged-display invariant unrepresentable The round-8/9 bug class (sidecar staged flag ORed over the effective set) existed because surfaces had a second staging source to reach for. Remove it: stagedFiles is now a REQUIRED prop on SectionsPanel/FileTree/ FileTreeNode and the optional-prop fallback branches reading sectionEntry.staged for display are deleted — a future surface cannot reintroduce the OR because the pattern no longer exists to copy. The sidecar type's staged field and AGENTS.md now document the invariant at the point of temptation. Grep for display reads of .staged now hits only useGitAdd (owner) and the sidecar builder (producer). * refactor(review): self-review cleanups on the staged-invariant hardening - orderFilesBySections: document why its snapshot .staged read is safe (only called at sidecar-fresh moments where snapshot = effective) and that it must not be reused mid-session — the one remaining display-side snapshot read the invariant sweep surfaced. - FileTreeNode: drop the now-pure sectionStaged alias; use isStaged. * fix(review): PR-990 review round 10 — header staging gate, fingerprint cap, single-writer coupling - Single-file diff header now uses the per-path staging gate (canStagePath), closing the last ungated staging trigger: committed-only files in since-base offered a no-op Git Add that flipped local staged/viewed state. Full trigger inventory swept: App `a` shortcut, all-files `a` + header, SectionsPanel rows, FileTreeNode rows, single-file header — all six now per-path gated or group-gated; no context-menu staging exists. - Fingerprint circuit-breaker: the freshness poll's `git status --porcelain -uall` degrades permanently (per cwd, per process) to collapsed -unormal once its output exceeds 2MB — a forgotten node_modules/ no longer burns CPU every 5s for the whole session. Costs untracked-dir edit sensitivity only on such repos; one possibly-spurious staleness banner at the switch. - The sections ⟺ since-base coupling now has a single writer: setReviewPanelView/setReviewDefaultDiffType in @plannotator/ui/config. All five call sites (setup dialog x2, Settings x2, header toggle, first-run reset, self-heal) converted; grep for direct writes of either setting now hits only reviewView.ts. - /api/diff/switch responses pass snapshot args to the AI context builder (both branches, Bun + Pi) — correct today, now robust against future awaits between the epoch check and the response. - Docs: explicitBase in the /api/diff/switch body. Parked per discussion: SectionsPanel/FileTree nav+search+footer dedup (extract when the commit-list view adds a third panel), querySelector row measurement, fetch-base stderr passthrough (standing decision), Pi hasAgentLocalAccess (pre-existing follow-up list). * fix(review): PR-990 review round 11 — subdirectory launches, escape decode, settings note - Repo-root-relative patch paths now resolve against the git toplevel everywhere they meet a filesystem path or pathspec, via a shared resolveRepoToplevel helper: file-content working-tree reads (hunk expansion returned null from a subdirectory launch) and gitAddFile/ gitResetFile (stage/unstage failed with pathspec errors). Both bugs pre-existed for uncommitted/unstaged; since-base made them the default experience. The two existing inline toplevel resolutions (untracked diffs, fingerprint) now use the same helper. Shared code — Pi inherits via vendoring. Real-repo subdirectory tests for both. - unquoteGitPath decodes \uXXXX (JSON.stringify emits it for control chars without a short escape; our synthesized workspace headers round-trip through this decoder). Malformed \u stays literal. Tests. - Pi explicitBase guard matches Bun byte-for-byte on empty-string base. - Settings Git tab notes when the CURRENT repo can't serve the Git-status view (base ref unresolvable) instead of letting the preference look silently broken — it's a global preference, so the options stay. Same-class items left parked (pre-existing, untouched by this PR): open-in root resolution and code-nav file reads from subdirectory launches — on the follow-up list with the Pi divergences. * fix(review): PR-990 review round 12 — per-client freshness, keyboard-operable row controls - Freshness is now judged PER CLIENT: every patch-carrying response (/api/diff, /api/diff/switch, pr-switch, pr-diff-scope) includes snapshotId (the server's draftKey), and the client echoes it on /api/diff/fresh probes. A mismatch reports stale for THAT client regardless of the VCS fingerprint. This fixes the round-12 finding — reloads/second tabs after the startup base upgrade got a permanently bogus staleness banner from the shared pre-upgrade baseline — and deletes the round-6 conditional re-baseline hack entirely (the fingerprint recaptures unconditionally again; the old client's banner now comes from its snapshot mismatch, not a deliberately stale baseline). Also gives unfingerprintable modes (P4, PR layer) snapshot- level staleness for free. Bun + Pi + client hook. - StageControl and ViewedControl (which had the identical gap) are keyboard-operable: tabIndex + Enter/Space activation + focus outline. They're spans inside the row <button> (nested real buttons are invalid HTML), so they need their own focus stop; the a/v shortcuts remain the power path. - Docs: snapshotId on /api/diff, ?snapshot= on /api/diff/fresh. * fix(review): PR-990 review round 13 — re-key snapshots on scope switch, PR-tab refresh - The PR scope switch now re-keys draftKey (= snapshotId + draft storage key) at BOTH commit points, unconditionally — matching every other snapshot commit site. The full-stack branch previously kept the layer patch's key: stale layer tabs never got the banner after a cross-tab scope switch (the exact case snapshotId exists for), and full-stack drafts collided with layer drafts (pre-existing). The layer branch's !layerPatchIncomplete conditional is gone — it only stayed consistent because full-stack never re-keyed. Invariant now: every currentPatch commit is followed by a re-key. Bun + Pi. - Refresh works for any stale PR tab: re-selects the CURRENT scope instead of no-opping for layer (only full-stack could go stale in the fingerprint-only world; snapshot mismatch changed that). Accepted residual (documented in code): after a cross-tab PR switch, refresh updates the patch but not prMetadata — the scope endpoint doesn't carry it, and the full-rehydrate refactor isn't worth the two-tab edge. * fix(review): stale incomplete-layer Refresh uses the non-blocking upgrade path (self-review) Round 13 made Refresh re-select the current scope for stale PR tabs; for an INCOMPLETE layer patch that POST triggers the server's local recompute, which can park for minutes behind checkout warmup — and handlePRDiffScopeSelect renders the full-screen switch overlay the whole time. Route that case through handleLoadFullDiff (same POST, progress notice instead of modal), which exists for exactly this slow path. * fix(review): round 14 — composite snapshot id, fully-pinned /api/diff, "All changes" label - snapshotId is now content hash + diff type (+ PR scope), built by a single currentSnapshotId() helper used at every response site and the freshness compare. A cross-tab MODE switch with a byte-identical patch (layer vs full-stack on a single-PR stack) now flags old tabs; the base is deliberately excluded so a same-commit main -> origin/main canonicalization stays banner-silent (round-12 noise-avoidance kept). draftKey stays a pure content hash — drafts survive content-identical round-trips. Bun + Pi. - GET /api/diff pins ALL served fields (diffType, hideWhitespace, prDiffScope join patch/base/ref/error/snapshotId) and the sidecar + AI context builders take the pinned type instead of reading globals — a concurrent tab switch during the sidecar await can no longer produce a since-base patch labeled with another mode. Bun + Pi. - First-user feedback: the since-base label is now plain English — "All changes since origin/main" (dynamic, follows the picked base); dialog/Settings short form "All changes"; feedback exports match. uncommitted reverts to "Uncommitted" where it had borrowed "All changes", so the two stay distinguishable side by side. * docs(review): finish the Since-main -> All-changes terminology sweep in comments (self-review) * fix(review): PR-990 review round 15 — base-revert race, freshness reset, unstage regroup - resolveReviewBase gains a second, probe-independent rule: a non-explicit echo of the bare local name of the CURRENT origin/* base stays on the tracking ref. The existing rule keys off remoteDefaultInfo, which comes from a second network probe that can lag the startup upgrade by seconds; in that window a diff-type/whitespace switch echoing "main" committed the session back onto the stale local branch and set baseEverSwitched, permanently blocking the upgrade. Bun + Pi. - useDiffFreshness resets stale/dismissed state on snapshotId too — since round 14 a new snapshot can reuse identical patch text with a different id, and the old banner state wrongly carried over until the next poll. - SectionsPanel: unstaging a PRE-staged add moves it back to Untracked (mirror of the round-7 stage regroup). Detection deliberately uses the sidecar's snapshot flag to recognize "was pre-staged"; staged modifications stay in Changes, staged renames remain a refresh-heals edge. * fix(review): PR-990 review round 16 — rename staged count, guarded fetch-base replay - "N added" no longer double-counts staged renames: the sidecar truthfully marks BOTH porcelain sides staged, but an above-threshold rename renders as ONE file — the hidden old path inflated the count and left a phantom effective-staged entry that unstaging the visible row couldn't clear. The client now filters sidecar-staged paths to rendered files, which is correct in both patch shapes (below-threshold renames render delete+add as two rows and both sides pass the filter). Client-only; the sidecar stays faithful to porcelain. - The fetch-base completion only replays the diff refresh if the user's diff type/base selection is unchanged since the click — a slow fetch no longer yanks the review back to the view captured at click time. Declined with reasons (in review thread): first-run persist-before-confirm (approved forced default, second flagging), explicit-base flag ordering (early-set preserves user intent; commit-after-win would leave picker and server disagreeing), base-picker revert target (near-unreachable compound race, deferred). * fix(review): PR-990 review round 17 — find origin/main on feature-only clones getDefaultBranch's chain (origin/HEAD -> local main -> blind "master") skipped the fetched remote-tracking ref entirely. On checkouts with no origin/HEAD symref and no local main/master — CI checkouts, `clone --branch feature`, extra worktrees — it guessed "master", the base didn't resolve, getGitContext suppressed since-base, and the flagship "All changes" view silently disabled itself for the whole session even though origin/main was fetched and diffable. The startup upgrade can't rescue it either (its guard reads the bogus "master" as a deliberate non-default base). Chain is now: origin/HEAD (verified) -> origin/main -> local main -> origin/master -> "master" — remote-tracking refs preferred, matching the function's stated prefer-upstream intent. Shared core (Pi inherits); real-repo test reproducing the exact clone shape.	2026-07-04 09:53:49 -07:00
Michael Ramos	9bded34324	chore: bump version to 0.21.4	2026-07-01 11:32:47 -07:00
Michael Ramos	e3de938914	feat(review): PR Overview panel + description/comment annotations + media (#981) ... Combine the PR Summary/Comments/Checks tabs into one PR Overview panel, then make the description and comments annotatable and render their media. - PR Overview panel (one sidebar entry) + comment UI (avatars, filters, hide bots, live context, responsive stacking). - Annotate the PR description (select → comment) and PR comments (Annotate button), with Ask AI; notes show in the Annotations sidebar and ship to the agent. - Split/Unified diff toggle relocated into the dock tab strip. - Render images + video in descriptions and comments (raw HTML + markdown), capped to the card so nothing bleeds. - Review-flow fixes: copy-all feedback, prose-only feedback preamble, no image control on prose notes, GitHub review-body seeding; stronger review trailer. - Add Claude Sonnet 5 as the default Ask AI model. No server, endpoint, or Pi-runtime changes.	2026-06-30 23:05:43 -07:00
Michael Ramos	1eee3eb313	chore(vscode): bump extension to 0.16.7 (clipboard/keyboard bridge #970, data-dir #795)	2026-06-28 12:51:07 -07:00
Michael Ramos	ec8342c34a	chore: bump version to 0.21.3	2026-06-28 11:37:37 -07:00
Michael Ramos	cb6667e991	fix(ai): drive Codex Ask AI via codex app-server (#971) ... * fix(ai): drive Codex Ask AI via app-server + answer-first review prompts (#971) Codex Ask AI previously ran via @openai/codex-sdk (codex exec), which forces approval_policy=never and breaks in enterprise-managed Codex environments that ban it (#971). Replace the transport with a long-lived 'codex app-server' process over JSON-RPC. - New provider packages/ai/providers/codex-app-server.ts (registered as 'codex-sdk' to preserve cookie/agents.ts/UI-gate); omits approvalPolicy so Codex resolves the user's + managed policy, pins read-only sandbox, and surfaces interactive approvals through the existing PermissionCard. - Delete codex-sdk.ts and drop the @openai/codex-sdk dependency (and its 6 prebuilt platform binaries); gate registration on 'which codex'. - SessionManager: additive, optional dispose?() hook to kill the spawned process on evict/remove — a no-op for Claude/OpenCode/Pi (they don't implement it). Also rework the Ask AI prompts (all providers, separate from the transport): - Every mode now instructs the agent to answer the user's message directly and not launch an unprompted review of the context. - Code review stops pasting the whole diff for git-reproducible diff types and instead tells the agent how to inspect it (git diff <base>..HEAD, three-dot for merge-base); non-git/PR/workspace types still paste. - Claude gains the Bash tool so it can run git (still gated by approvals). - The UI passes diffType/base (session) and what the user is viewing (per question) into the context. Verified: full typecheck, full test suite (101 ai tests), and a live end-to-end smoke against codex app-server. * fix(review): pin AI approval card above the input/model bar Render pending approval cards just above the input + provider/model bar in both the document chat (DocumentAIChatPanel) and code-review AI tab, instead of at the top of the scroll, so the user sees them where they act. * fix(ai): harden Codex abort/cancel + add Ask AI Stop button Addresses code-review findings on the codex app-server provider: - turn/interrupt was sent as a notification (no id) so Codex ignored it and abort never took effect. It's now a proper JSON-RPC request. - Filter turn events/approvals by turnId and reject an aborted turn's stragglers, so a stopped turn can no longer leak output into — or prematurely finish — the next turn (ask-stop-ask race). - Guard listeners by query generation and end the drain loop on the abort signal, so a superseded/stopped turn can't touch the live one and abort returns promptly instead of waiting for turn/completed. - Handle abort during startup: once the turn id is known, interrupt it instead of running it in the background. - Add a sendAndWait timeout so a stalled (alive-but-unresponsive) process errors instead of hanging forever. - Drain stderr (stdio 'ignore') to avoid a pipe-buffer deadlock. Also add a Stop button to both Ask AI surfaces (plan/annotate DocumentAIChatPanel and code-review AITab via ReviewSidebar). It replaces Send while streaming and calls the hook's abort -> /api/ai/abort -> session.abort(); the hook already exposed abort but nothing surfaced it. * feat(ai): drive Codex models + reasoning levels from model/list - Codex provider fetches model/list at startup (throwaway app-server, like Pi/OpenCode) and populates the real models plus each model's actual supportedReasoningEfforts + defaultReasoningEffort. Replaces the hardcoded model list and the static AI_REASONING_EFFORTS (which mislabeled xhigh as 'Max' and omitted minimal). - AIProviderBar + AIConfigBar now show the selected model's real efforts and hide the control when a model reports none. xhigh is shown verbatim. - Fix: the Stop button now also appears in the populated code-review chat state (a prior edit missed the second GeneralInput due to indentation). * fix(ai): scope Claude Ask AI Bash to read-only git; clear stale approval cards on Stop - Claude Ask AI no longer auto-allows bare Bash (which ran arbitrary shell with no Allow/Deny prompt). Replace it with scoped read-only git rules (Bash(git diff:*), show, log, status, rev-parse, merge-base, ls-files). git reads auto-run so the agent can inspect large diffs itself; any other command (write git, arbitrary, or injected compound) falls through to the permission card. Keeps the git-inspect approach (large diffs don't fit in the prompt) while closing the auto-exec hole. - useAIChat.abort() now drops still-undecided permission cards: abort cancels them server-side, so leaving them visible was a dead Allow/Deny. * feat(ai): code-review Ask AI shares the agent-review prompt machine, delivered as user messages Code-review Ask AI built its own diff description (gitInspectInstruction) in the system prompt from just diffType+base, which was wrong for full-stack, hide-whitespace, untracked files, and PR worktrees. Replace it with the same machine the review jobs use, delivered on the user's messages. - Server: buildCurrentAiReviewContext() reuses buildAgentReviewUserMessageForTarget (contextOnly) for the current view and ships it as aiReviewContext in every diff payload (/api/diff + switch/PR/scope). Mirrored in the Pi server. - Client: review-editor latches aiReviewContext onto each question via the pure buildReviewContextPreamble (packages/ui/utils/aiPrompt.ts) and buildDefaultPrompt — full block on the first message / when the view changes (incl. after a provider switch via the !sessionId fresh-session check), a short reminder otherwise (never re-pastes a large diff). - context.ts: delete the duplicate gitInspectInstruction; code-review system prompt is now role-only. Providers untouched (provider-agnostic user message). - Tests: machine scenario gaps (plain PR, full-stack default, PR-worktree origin/<base> + stale-main warning, untracked mention, jj-evolog, workspace lines); composition (first/reminder, command/pasted, preamble ordering). * fix(ai): agentic remote-PR context, UTF-8 stream decode, real Stop on supersede/disconnect Addresses review findings on the Ask AI prompt work: - Remote PR without a confirmed local checkout no longer gets URL-only. The agent is told it's in a PR worktree that's being prepared, to verify the PR files exist before relying on them, and to diff with git diff origin/<base>... HEAD (URL fallback). Inform + trust the agent rather than pasting. Shared machine, so review jobs get the same framing. - Decode Codex stdout with a streaming TextDecoder instead of per-chunk toString(), so multi-byte UTF-8 split across chunks no longer corrupts into U+FFFD (matches the Pi provider). - Stop now actually stops the server turn: ask() awaits /api/ai/abort when a new question supersedes a streaming one (awaiting avoids racing the new query into session_busy), and the /api/ai/query SSE stream gains a cancel handler so tab-close/navigation aborts the turn too. Both reuse the existing per-provider session.abort(); factored a shared postServerAbort helper. * fix(ai): per-model reasoning effort, shared PR-checkout readiness, Stop-then-ask race, type hole - Reasoning effort is now tracked per model (a map keyed by model) instead of one global value, so switching to a model that doesn't support the prior level (e.g. xhigh) no longer posts a stale/unsupported effort that Codex rejects. Each model keeps its own level; nothing leaks across. - Extract resolvePoolCwd into packages/shared/worktree-pool.ts (ready/pending/ absent) and use it from both servers' resolvePRLocalCwd so the readiness rule can't drift. Fix the Pi Ask AI helper to ready-check like Bun, so a warming PR checkout no longer claims 'checked out at PR head' and misdirects the diff. - Stop-then-ask no longer races into session_busy: the abort promise (from Stop or a superseding question) is stashed and the next ask() awaits it before sending. Stop still kills the turn instantly; a follow-up just waits the one abort round-trip. - Declare aiReviewContext on the initial /api/diff response type. * fix(ai): surface real Codex error messages, skip transient retries, auto-deny permission escalations Validated against codex-rs: - Codex's ErrorNotification nests the text at params.error.message (TurnError.message); we read params.message → always 'Unknown error'. Read the nested field (top-level fallback) so auth/usage-limit/stream failures show their real, actionable text. - Skip transient error notifications (willRetry=true): Codex retries on its own and the turn continues, so surfacing them flashed a spurious failure before the real answer. - Handle item/permissions/requestApproval: respond {permissions:{}, scope:'turn'} — byte-for-byte Codex's own cancel response (codex_delegate.rs) — instead of 'Unsupported request'. Ask AI is read-only, so denying the escalation lets the turn continue sandboxed rather than failing. Interactive grant deferred. * refactor(ai): share AI provider/model config in one hook; aggregate paginated model/list - Extract useAIProviderConfig (packages/ui/hooks): one home for provider/model/ reasoning-effort selection — initial state, auto-resolve on capabilities load, per-model effort (no leak across models), and persistence. Both the plan and code-review apps now call it and only compose the session reset (the hook can't own reset without a cycle through useAIChat). Plan editor adapted to the effect-based resolve (adds aiDefaultProvider state). This fixes the plan editor's stale-effort-on-model-switch bug by construction and stops the two apps' copies from drifting again. - fetchModels now follows model/list's nextCursor and aggregates every page into one list, so larger model catalogs aren't silently truncated (with a page guard against a misbehaving cursor). Skipped per discussion: legacy v1 approval methods (we're a v2 client). * feat(agents): per-agent review default; bound Codex model discovery so it can't stall the AI panel - The selected review profile is now tracked per review engine (claude/codex/ cursor/opencode) instead of one flat global value, so each agent keeps its own review default. Public hook API (reviewProfileId/setReviewProfileId) is unchanged — getter derives the current engine's value, setter writes it — so AgentsTab needs no changes. One-shot migration seeds every engine with any existing flat pick. Adds parseReviewProfileByEngine + tests. - fetchModels (Codex model discovery) now uses a short 6s timeout for its initialize + model/list RPCs instead of the 30s default. /api/ai/capabilities awaits model discovery, so an installed-but-unauthenticated codex could otherwise block the AI panel for ~30s; it now falls back to the static model list fast. Authed codex is unaffected (discovery completes in tens of ms).	2026-06-28 09:58:09 -07:00
Michael Ramos	82acc4b0aa	fix(cli): print per-subcommand help instead of launching the UI (#974) ... * fix(cli): print per-subcommand help instead of launching the UI `plannotator review --help` (and other subcommands) fell through to their command branch because only top-level `--help` was handled. For `review`, `--help` was parsed as a non-URL positional, dropping into local review mode and opening a browser tab. When Claude Code probes the CLI with `--help`, that stray tab's close injects a bogus "no feedback → proceed" signal into the session. Handle `--help`/`-h` for every user-facing subcommand (review, annotate, annotate-last/last, setup-goal, archive, sessions) before any subcommand branch can run: print command-specific usage on stdout and exit 0. Also accept `-h` at the top level and advertise per-command help there. Fixes #964 * fix(cli): handle `improve-context --help` too The top-level help advertises `plannotator <command> --help`, but `improve-context` (the only internal hook command listed there) had no help entry, so `improve-context --help` fell through to the hook branch and emitted additionalContext JSON instead of usage. Add a help entry so every advertised command responds to --help.	2026-06-26 13:10:56 -07:00
Aleksey Shelementev	1e631ac62e	fix(vscode): bridge clipboard and forward keystrokes in webview (#970) ... The Plannotator app runs in a nested cross-origin iframe inside the VS Code webview, where two input paths break: - Clipboard: the iframe document never holds focus, so the async Clipboard API is rejected ("Document is not focused") and native copy/cut/paste events never fire. Copy, cut, and paste all silently fail. - Keybindings: keydown events don't cross the iframe boundary, so VS Code never sees global shortcuts (Cmd+P, etc.) while a Plannotator tab is focused. Fix both off the one input signal the iframe still receives — keydown: - Route clipboard reads/writes through the extension host, which owns the system clipboard via vscode.env.clipboard. The injected script overrides navigator.clipboard and handles Cmd+C/X/V, relaying through the wrapper to an onDidReceiveMessage handler and back. - Forward all other keystrokes to the wrapper, which re-dispatches a synthetic KeyboardEvent on the webview document so VS Code can resolve its keybindings. Undo/redo/select-all stay native. Fixes #864 Fixes #969 Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-26 12:54:19 -07:00
Michael Ramos	3efd9962cd	chore: bump version to 0.21.2	2026-06-25 06:10:46 -07:00
Michael Ramos	842896a370	docs(review): fix Codex command flag (--full-auto, not --sandbox workspace-write) ... The ai-code-review.md Codex example showed --sandbox workspace-write, but the runtime (codex-review.ts) uses --full-auto. Align the doc with the actual command.	2026-06-25 06:04:22 -07:00
Michael Ramos	e138c824aa	feat(review): Cursor + OpenCode review engines (unified marker-review) (#959) ... Add Cursor CLI and OpenCode as background code-review engines alongside Claude/Codex/Tour, unified behind a shared marker-review module (prose marker-block parsing, since neither CLI exposes a schema flag). Includes multi-provider dynamic model discovery, review-profile support, fail-closed parse/ingest, Bun+Pi parity, and the review UI (Provider picker, brand icons). Hardening from review: per-job nonce on marker tags (prevents echoed/quoted tags from corrupting extraction), truncation fail-closed, async model discovery (never blocks the event loop), nested-slash model ids, shared finding types.	2026-06-24 14:35:52 -07:00
Michael Ramos	affeaa07fc	feat(review): custom reviews as Agent Skills + whole-file/general findings (#955) ... Custom reviews: pick an Agent Skill and its SKILL.md body becomes the review prompt (read live from global skill folders, never copied; default review byte-identical). Agent findings can be a line, a whole file, or a general review-level comment, and are never silently dropped. Plus: agent-icon engine picker, icon-only file-header opener, semantic diff restored as its own dock panel, and a dead-code sweep.	2026-06-23 20:49:48 -07:00
Michael Ramos	e385ca90f5	feat(marketing): move capabilities section above demos on landing page (#953) ... Surface "Everything the plugin does" immediately after the hero/social proof so visitors see the full feature set before scrolling to demos. Reorder commands to lead with annotate and review, and clarify the review description.	2026-06-22 09:31:48 -07:00
Michael Ramos	caf8bb54bf	chore: bump version to 0.21.1	2026-06-22 05:26:21 -07:00
Michael Ramos	90bc8f8db5	chore: bump version to 0.21.0	2026-06-19 11:10:38 -07:00
Michael Ramos	740d6fb2eb	Add WebTUI agent panel to annotate mode (#941) ... * feat(annotate): add WebTUI agent terminal * feat(annotate): wire WebTUI agent into annotate UI * docs: recap annotate agent terminal work * fix(annotate): harden agent terminal runtime * docs: add annotate agent terminal runtime ADRs * fix(annotate): polish agent terminal integration * fix(ui): preserve comment draft on Ask AI failure * fix(annotate): address terminal review findings * fix(annotate): harden agent terminal runtime fallback	2026-06-19 09:04:15 -07:00
Michael Ramos	2a6fe5c457	Open files in external apps + code-review UX pass (#942) ... Adds a server-side "open the current file in an external app" control to the code-review and annotate surfaces (split-button with host-detected apps; last-used becomes the default; cross-platform launch mirrored across the Bun and Pi runtimes), plus a code-review UX pass: file-header change letters and line counts, the diff-settings cog grouped with Split/Unified, an all-files collapse/expand-all toggle, and the semantic diff moved to a resizable sidebar accordion. Also fixes Cmd+click code navigation (worker token transformer). Hardened over three adversarial review rounds: shared, tested path-containment for open-in (Bun + Pi), annotate scoping aligned to /api/doc reference roots, strict PR-checkout rooting (never the launch repo), deduped app-catalog and semantic-diff fetches, macOS bundle-only availability, Windows launch fixes (reveal + terminal off cmd's parser), and PR-checkout tracking across switch and pool warmup.	2026-06-19 08:56:06 -07:00
Michael Ramos	195328f7a6	Persist saved annotate file edits in drafts (#936) ... * Persist saved annotate file edits in drafts * Handle stale saved file edit context * Test source edit conflict actions * Return source metadata for single-file docs * Fix saved file edit conflict races * Handle deleted source files in annotate edits * Tighten annotate missing-file recovery * Reset edit state for missing file reopen * Tighten source edit restore path handling * Harden source edit recovery paths * Harden source edit disk reconciliation * Fix live file tree startup delay * Document file tree watcher startup ordering * Preserve source save through missing files and symlinks * Harden missing source file recovery * Tighten annotate source edit boundaries	2026-06-18 21:31:25 -07:00
Michael Ramos	1fcfad80c8	Polish live file tree refresh	2026-06-17 18:30:25 -07:00
Michael Ramos	40210fdfd2	Add live annotate file tree workspace status (#931) ... * feat(annotate): add live file tree workspace status * fix(annotate): guard dirty feedback and rename stats * fix(annotate): tighten live file tree status * fix(annotate): surface deleted file browser roots * fix(annotate): normalize file tree status paths * fix(annotate): avoid optional git locks for workspace status * fix(annotate): tighten workspace status git metadata * fix(annotate): refresh file tree after reconnect * fix(pi): expose file browser stream route	2026-06-17 14:25:28 -07:00
Michael Ramos	9c77c2f812	fix(server): always show remote URL + close share-html symlink gap (#929) ... * fix(server): always show remote URL + close share-html symlink gap Two pre-release fixes found by the QA sweep. 1. Remote URL stranding: handleServerReady only opened a browser (silent on a headless box) and wrote a ready-file; the user-visible URL came solely from writeRemoteShareLink, gated on sharingEnabled. So a remote/SSH user with sharing disabled (PLANNOTATOR_SHARE=disabled or config.json {share:"disabled"}, widened by #921) saw no URL and the agent hung on waitForDecision() forever. Now handleServerReady prints the reachable localhost URL whenever the session is remote, independent of sharing; the share link stays an extra. Pi/OpenCode already printed the URL unconditionally and are unaffected. 2. share-html symlink gap: the #927 symlink-containment fix hardened the /api/html-assets asset sinks but missed packages/server/annotate.ts's /api/share-html containment, which stayed lexical. A symlinked *.html inside the doc directory pointing outside it leaked the target's contents into the share payload. Now realpath-resolves both root and target like the other sinks. Bun-only — Pi's copy was already hardened. Adds regression tests for both (handleServerReady remote stderr; share-html returns 403 on symlink escape). * refactor(server): single shared isWithinDirectory for all asset/share sinks The symlink-containment check existed as four byte-identical copies (Bun html-assets route, share inliner, annotate /api/share-html, Pi server). That duplication is exactly why the escape was missed in one sink before — #927 hardened three and missed the fourth, and the #929 fix added a fifth-in-waiting. Export the single canonical isWithinDirectory from the shared (Pi-vendored) html-assets-node module and have every sink import it; delete the three duplicate bodies and their now-dead realpathSync/relative/isAbsolute imports. A new sink can no longer silently diverge. Regression tests + 690 server/shared tests pass; build:pi clean. * fix(pi): surface the session URL in the in-turn 'opened' notice for remote Remote Pi users never saw a review/annotate/last URL: it was emitted from openBrowserForServer AFTER the command's turn ended (fire-and-forget), and Pi only renders a notify during an active turn — so it silently dropped. The 'X opened. You can keep chatting' notice fires in-turn and DOES show, so fold the URL into it for remote sessions (single-line, matching the notify convention). Local sessions are unchanged (browser auto-opens).	2026-06-17 07:52:24 -07:00
Josemi Liébana	b7ef0756d9	fix(annotate): add /api/save-notes POST endpoint to annotate server (#884) ... * fix(annotate): add /api/save-notes POST endpoint to both servers Copies the save-notes route from the plan review server into the annotate server (Bun source and Pi extension copy), enabling Save to Obsidian in annotation mode. Fixes #844 * test(annotate): add saveToObsidian unit tests and HTTP endpoint tests Verifies saveToObsidian writes files correctly and handles missing vaults. HTTP endpoint tests cover success, empty integrations, and integration-level error (not 500). Imports consolidation from ./integrations into a single statement. * fix(annotate): normalize server port fallback * refactor(server): extract shared handleSaveNotes handler, fix catch-block bug Move the /api/save-notes logic into shared handler modules (shared-handlers.ts for Bun, handlers.ts for Pi) following the existing pattern for handleImage, handleUpload, handleDraftSave. Replaces four inline copies with two canonical implementations. Fixes: - Bun annotate catch block now correctly returns 500 (was logging only) - Misindented brace in Pi serverAnnotate.ts resolved by extraction - Revert unrelated port fallback change (keep server.port! for consistency) - Static imports in integrations.test.ts - Add /api/save-notes to CLAUDE.md Annotate Server API table * fix(opencode): inject annotate server starter instead of global mock.module commands.test.ts mocked the annotate server with `mock.module("@plannotator/server/annotate", ...)`. Bun module mocks are process-global and cannot be unset (oven-sh/bun#7823, #12823), so the stub leaked into every suite that runs after it — in particular any test that boots the real annotate server received a stub with no `.url`. Make `startAnnotateServer` injectable through the existing CommandDeps (defaulting to the real import, so production is unchanged) and have the test pass its stub that way. This keeps the fake local to the opencode suite and unblocks real annotate-server integration tests. * test(server): cover save-notes — handler unit tests + annotate e2e wiring - shared-handlers.test.ts: unit-test handleSaveNotes directly (Obsidian write, empty integrations, integration-error reported not thrown, 500 on bad body). - annotate.test.ts: boot the real annotate server and POST /api/save-notes, asserting it is served as JSON (not the SPA HTML catch-all) — the regression guard for #844. Now possible because the opencode suite no longer installs a global annotate module mock. --------- Co-authored-by: Michael Ramos <mdramos8@gmail.com>	2026-06-16 21:38:39 -07:00
Michael Ramos	f564448650	fix(annotate): block symlink escape in HTML asset serving + cleanups (#927) ... * fix(annotate): resolve symlinks before HTML asset containment check The /api/html-assets route and the share-payload inliner checked path containment lexically, so an in-directory symlink pointing outside the HTML's folder (e.g. evil.css -> ~/.ssh/id_rsa) passed the check and was served or base64-inlined into the share payload. In remote mode the inliner auto-fires at startup, so this could upload symlinked local files to the paste service with no user action. Resolve symlinks with realpathSync on both the asset and the root before the relative-path check, in all three runtime copies (Bun route handler, shared node inliner, Pi route handler). Non-existent assets fall back to the lexical path and 404 on read. Adds regression tests for both sinks. * test(pi): build rich git state in sandbox-pi.sh for review diff modes Expand the Pi sandbox harness to create multiple commits, a feature branch, and a rename+delete+modify commit so /plannotator-review can exercise every diff mode (uncommitted, staged, branch, merge-base). * docs: fix broken verification link in READMEs The READMEs pointed at a non-existent anchor (installation/#verifying-your-install); the verification guide is a standalone reference page. Point to /docs/reference/verifying-your-install/ (and split the hook README's link so version pinning -> installation, verification -> the reference page).	2026-06-16 19:55:47 -07:00
Yoni Hornstein	201ca11ec4	feat(config): support share toggle via config.json (#921) ... Allow disabling URL sharing through ~/.plannotator/config.json ({ "share": "disabled" }) in addition to the PLANNOTATOR_SHARE env var. Adds a resolveSharingEnabled() helper (env var > config > default enabled) and routes all sharing checks through it across the hook server, OpenCode plugin, and Pi extension. Docs updated.	2026-06-16 18:47:24 -07:00
Michael Ramos	9ed3ba8937	feat(editor): markdown edit mode — direct document editing with diff-to-agent feedback ... Adds direct markdown editing, source-backed annotate saves, folder edit buffers, and review-hardening fixes.	2026-06-16 16:35:51 -07:00
Michael Ramos	be2d06a7c2	Make HTML annotations render HTML by default ... * feat(annotate): render html files by default * fix(annotate): support raw html assets and sharing * fix(annotate): address html first review followups * fix(editor): avoid raw html sidebar init crash * fix(annotate): support portable html shares * fix(annotate): harden html share support * fix(share): clear attachments when loading shared payloads * fix(share): warn on remote share link failures * perf(annotate): lazy-build html share payloads * test(annotate): guard lazy html share generation * test(annotate): drop flaky html share server test	2026-06-16 16:16:05 -07:00
Michael Ramos	5e31b79a01	chore: bump version to 0.20.3	2026-06-16 00:33:34 -07:00
Michael Ramos	aaa0cab402	chore: bump version to 0.20.2	2026-06-14 00:52:47 -07:00
Vyacheslav	d682c2cd9e	feat(pi): add programmatic plan mode event (#898)	2026-06-13 17:36:39 -07:00
Michael Ramos	6c64b96dd7	fix(review): signal-safe cleanup + triage suffix for PR feedback (#914) ... Two QA follow-ups found while validating the release: - Route SIGINT/SIGTERM through process.exit() so the existing "exit" handlers actually run on Ctrl-C / termination. A signal death previously skipped them, leaking background PR-checkout warmup children and stale `git worktree` registrations. A second signal still force-quits if cleanup hangs. - Append the review-denied triage suffix for PR-mode feedback, not just local diffs. The old `!isPRMode` gate suppressed it for every PR review. Gate on whether the reviewer actually sent annotations instead: genuine feedback always carries annotations, while platform PR actions (approve/comment posted to the host) return an empty annotation set + status message and correctly get no suffix. Applied consistently across hook, OpenCode, and Pi.	2026-06-13 16:29:07 -07:00
Michael Ramos	6ec1a66c9b	feat(review): large-PR pipeline, instant-open checkout, scroll perf, and worker-pool highlighting (#893) ... * feat(review): large GitHub PR fallback + non-blocking PR checkout Two PR-mode improvements: 1. Large GitHub PRs no longer fail to load. When `gh pr diff` is refused (HTTP 406 for oversized diffs), fetchGhPR pages through the pulls files API and stitches the per-file patches into a unified diff — mirroring the existing GitLab raw_diffs fallback. Path quoting matches git's exact rules (bare spaces unquoted) so downstream parsers round-trip; truncation at the API's 3000-file cap is surfaced, never silent. 2. The --local worktree/clone no longer blocks startup. The review server opens as soon as the platform diff arrives; the checkout warms in the background as a seeded not-ready pool entry. Consumers that need real files (agent jobs, full-stack diff, code-nav, semantic diff, AI sessions) await pool.ensure(), with creations serialized so concurrent fetches can't clobber the shared FETCH_HEAD. Cross-repo clone steps converted from spawnSync to async spawns; warmup children are killed on exit (plus `git worktree prune`) so aborted sessions can't leak stale registrations; failed checkouts degrade honestly (no agent runs in the wrong directory claiming local access) with a 30s retry cooldown. * fix(review): survive long PR checkout warmups + classify reconstructed renames Stress-testing against oven-sh/bun#30412 (2,188 files) surfaced three bugs: - Bun.serve's default 10s idleTimeout killed /api/semantic-diff while it parked on the background checkout warmup (a clone that can take minutes). Disable the idle timeout on all servers — AI SSE streams can also stall >10s between bytes while a permission prompt waits. - The file-badge hook memoized that failed fetch in a module-level cache keyed by patch, pinning every badge to empty until a hard refresh. Never cache failures; retry with backoff (5s/15s/30s). - reconstructGhPatch/reconstructPatch omitted the `similarity index` line, which Pierre's parser keys rename classification off — pure renames rendered as blank plain changes with no old path. Emit 100% for patch-less renames/copies (exactly accurate) and a synthetic 99% for patched ones (consumers only branch on 100% vs not). * feat(review): local full-diff upgrade for PRs whose API diff is truncated On oversized PRs the platform APIs withhold per-file patch content entirely (bun#30412: 1,066 of 2,188 files came back with status added/modified, zeroed counts, and no patch). Those files rendered as empty stubs with no diff. - fetchGhPR/fetchGlMR flag the result `patchIncomplete` when patch-less non-rename entries exist or the 3000-file cap truncates the listing. - New runPRLayerLocalDiff (pr-stack.ts) recomputes the exact layer diff in the local checkout: platform merge-base + head SHA two-dot diff (three-dot vs baseSha fallback), fetch-by-SHA for objects missing from shallow clones, -l0 so rename detection doesn't silently degrade on huge PRs. - The review UI shows a "Partial diff · Load full diff" notice in layer scope; clicking re-requests the layer scope and the server swaps in the recomputed full diff (waiting out the background clone if needed). - PR scope/switch state writes are epoch-guarded: a request parked on the checkout warmup can no longer overwrite a newer scope select or pr-switch. - draftKey follows the upgraded patch so annotation drafts survive pr-switch round-trips; recompute failures surface in the response error field. - Pi server mirrors all of it, including an agentCwd fallback so the upgrade works for PRs switched-to under a cross-repo clone pool. * fix(review): use GitLab's too_large/collapsed flags for withheld-diff detection External review caught a false negative: a too-large ADDED file comes back new_file:true with an empty diff — indistinguishable from a legitimately empty new file under the old heuristic, so the partial-diff upgrade was never offered for exactly the files that matter most on big MRs. The REST /diffs endpoint marks withheld content explicitly per entry (verified against gitlab.com): too_large/collapsed are now authoritative in both directions — withheld adds/deletes are flagged, binaries and empty files are never misflagged. Older GitLab without the fields keeps the empty-diff-on-modification heuristic. * feat(prompts): unify review-denied suffix — triage first, no coding off raw feedback The per-runtime defaults map (#627) gave OpenCode and Pi a different review-denied suffix than every other runtime; updating one meant the others silently kept "you must address all of them" — an instruction to start coding immediately. Claude Code, Amp, Droid, Codex, Copilot, Gemini, and Kiro were all still on it. One default for every runtime now: triage the feedback, verify it against the code, discuss before changing anything. Per-runtime customization remains available via config (prompts.review.runtimes.<rt>.denied), which resolves above the built-in default as before. * fix(prompts): generalize review-denied suffix — 'from review', not 'external AI reviewers' Review feedback isn't always from AI reviewers or agent jobs; often it's the human reviewer's own annotations. Neutral wording covers both. * fix(review): non-blocking 'Load full diff' + flag-handling hardenings Self-review findings: - The partial-diff upgrade reused the scope-switch handler, so clicking "Load full diff" raised the full-screen PRSwitchOverlay — blocking the entire UI, potentially for minutes behind a cold clone, with no text and no cancel. The upgrade now has its own loading state: the notice shows a spinner ("Loading full diff…") and the reviewer keeps working with the partial diff while the request parks. Server-side epoch guards already handle scope/PR changes made during the wait. - GitLab too_large/collapsed: treat explicit null like absent (flags inconclusive → legacy heuristic decides) instead of silently exonerating. - Rename-limit lift uses -l100000 instead of -l0 ("0 = unlimited" only holds on git >= 2.29; on older git it could disable detection outright). * fix(review): stop scroll-driven sem stampede when semantic diff is failing The badge retry change (a2d19a4e) cleared the client-side sem cache on failure so transient errors could recover. But file-header badges mount and unmount on every scroll in the virtualized all-files view, and each mount re-requests /api/semantic-diff — and the server only cached SUCCESSFUL runs. With sem erroring, scrolling spawned a continuous stream of sem processes, pegging the CPU and making scrolling severely choppy. Bound retry rate by time, not by mount events: - client: keep the failed result memoized and expire it after a 60s cooldown instead of clearing immediately - server (Bun + Pi): memoize failed sem runs for 30s in SemanticDiffResponseCache — request rate can no longer drive execution rate * fix(review): eliminate all-files scroll jank (pre-existing on main, from #885) The CodeView migration introduced severe scroll chop; scrolling UP could freeze the viewport entirely ("scrolling but nothing changes"). Three compounding causes, diagnosed against Pierre 1.2.8 source: 1. Lazy full-content augmentation landed updateItem() mid-scroll-gesture: the full-content parse counts collapsed-context regions the raw-patch parse doesn't, so the item GROWS — re-render + re-tokenize hitches both directions, and when the grown item sat above CodeView's scroll anchor, its corrective scrollTo() killed wheel momentum (the up-scroll freeze). Fetches still start as items enter the window; the item mutation now waits for 150ms of scroll quiet (staleness re-checked at apply time). 2. reportVisibleFile read container.scrollTop/clientHeight/scrollHeight on EVERY scroll event — a forced synchronous layout right after each frame's DOM writes. Replaced with CodeView's cached accessors and coalesced the handler to once per animation frame. 3. Missing containment CSS: Pierre's own production wrapper uses contain:strict + will-change:scroll-position so forced layouts stay scoped to the scroller instead of the whole document. Adopted. Also: __devOnlyValidateItemHeights now requires explicit opt-in (VITE_PIERRE_VALIDATE_HEIGHTS=1) — it runs getBoundingClientRect() per rendered item per frame and made dev-server scrolling choppy by itself. * feat(review): change-type status in headers + tree, diffshub CSS parity Adopts two diffshub practices identified in the architecture comparison: - DiffFile now carries a derived status (added/deleted/renamed/modified) from the chunk's git metadata lines. FileHeader shows a status icon and renders renames as "old/path → new/path" (dimmed old, arrow — diffshub's treatment, including its rename blue); the file tree shows A/D/R markers. 'modified' is deliberately undecorated so the others pop. Works in both the all-files surface and the single-file panel, including header-only pure renames from the large-PR reconstruction. - CodeView container gains diffshub's remaining perf CSS: overflow-anchor: none (native scroll anchoring fights CodeView's own anchor resolution whenever item heights change — exactly our augmentation applies), overflow-x-clip, and overflow-clip containment on item elements. * feat(review): worker-pool syntax highlighting (diffshub parity) A performance trace of scrolling a small local diff attributed 2.2s of 2.6s main-thread CPU to findNextMatchSync — shiki's TextMate regex scanner tokenizing on the main thread. diffshub avoids this entirely by running tokenization in Pierre's worker pool; we never opted in. Wires WorkerPoolContextProvider around the review app (pool size min(cores-1, 3), 100-entry AST LRU, common languages preloaded), gates the all-files surface on pool readiness with a 5s escape hatch (a dead pool degrades to plaintext-then-highlight, never a blank view), and syncs the UI theme pair into the long-lived pool. Single-file build constraint solved with Vite's ?worker&inline (base64 blob worker) + worker.format 'es' with inlineDynamicImports — the worker's lazy import("shiki/wasm") branch collapses into the bundle and is never taken (shiki-js engine: the win is moving work off the main thread, with no .wasm asset to smuggle into one HTML file). Bundle +850KB. * fix(review): un-poison worker-pool theme dedup on failed setRenderOptions A failed round-trip recorded the theme as synced and never retried, pinning the pool to the wrong palette for the session. * fix(review): report partial diffs without a checkout; fail fast on missing checkout Dogfood review of this PR (via plannotator itself) caught two valid issues: - prPatchIncomplete was gated on the worktree pool, so a --no-local session showed a truncated diff with no indication at all. Partiality is information; upgradability is a capability. The flag is now always reported, with a separate prPatchUpgradeAvailable — the UI shows the amber notice either way, with the "Load full diff" button only when a checkout can exist (otherwise a "re-run with --local" hint). - After a FAILED checkout warmup, Ask AI sessions and agent jobs fell back to process.cwd() (or a wrong revision on Pi) — running in the wrong tree instead of failing. Both launch points now refuse with a clear "Local PR checkout unavailable — retry shortly" error (503); the job handlers surface buildCommand refusals instead of mislabeling them "Invalid JSON". Bun and Pi mirrored. A third finding (sem availability stuck after warmup) was triaged invalid: the availability probe detects the sem binary, which is cwd-independent. * fix(review): runtime-neutral copy for the no-checkout partial-diff hint --local is a CLI remedy; OpenCode sessions have no such flag. Visible text states the fact, the tooltip carries the CLI guidance.	2026-06-12 13:50:09 -07:00
Michael Ramos	afac44b940	feat(review): migrate all-files code review to Pierre CodeView (#885) ... Replaces the all-files renderer with Pierre's virtualized CodeView (one scroller, identical look, scales to huge diffs); deletes the legacy AllFilesDiffView/LazyFileDiff and their flag; single-file panels stay on FileDiff. @pierre/diffs pinned exact 1.2.8 (1.2.9's tree is broken on npm — see #880). Also ships: diff staleness detection ('Diff out of date · Refresh' toolbar notice backed by shared per-VCS fingerprints + GET /api/diff/fresh on both Bun and Pi servers), a stale-content guard so files edited mid-review degrade to raw-patch view instead of breaking virtualization, the Ask AI wrong-file fix (pre-existing), and toolbar/header UI polish (global settings cog, aligned sem badges, no all-files split dragger). Hardened through five review waves; tested with 1384 passing tests incl. real-git fingerprint and patch-consistency suites. Known minor gaps documented in the PR.	2026-06-10 21:07:58 -07:00
Devin	4b76db9396	fix(annotate): improve error message for unsupported file types (#870) ... - When a file exists but has an unsupported type (.cs, .ts, .py, etc.), show 'File type not supported' instead of misleading 'File not found' - Display supported file types (.md, .mdx, .html, .htm) - Suggest using 'plannotator review' for code files - Maintains backward compatibility for actual missing files Fixes #757 Co-authored-by: ishowman <ishowman@users.noreply.github.com>	2026-06-10 16:16:44 -07:00
Michael Ramos	97b5e62a4d	chore: bump version to 0.20.1	2026-06-10 09:26:34 -07:00
Michael Ramos	4e841e2124	fix(deps): pin @pierre/diffs to exact 1.1.20 ... Upstream @pierre/diffs@1.2.x depends on @pierre/theming@0.0.1, which is not published on npm, so any fresh install resolving our ^1.1.12 range floated to 1.2.9 and failed with a 404. Our lockfile masked this for repo builds, but npm consumers of @plannotator/pi-extension resolve fresh and hit it (#880). Pin every declaration to exact 1.1.20, whose dependency tree is fully resolvable (@pierre/theme@0.0.28).	2026-06-10 09:25:40 -07:00
Michael Ramos	ad2bb25147	chore: bump version to 0.20.0	2026-06-09 12:07:48 -07:00
Michael Ramos	6573f2f519	chore(release): fix README command count + point dialog at v0.20.0 notes ... - apps/hook/README.md: 'four' -> 'three' slash commands (archive row was removed in #873 but the count wasn't reverted). - LookAndFeelAnnouncementDialog: 'Full release notes' now deep-links to the v0.20.0 release tag instead of the generic releases list.	2026-06-09 10:58:35 -07:00
Michael Ramos	0be4295a2e	Plan-look chooser + 0.20.0 release dialog (with GitLab / sem / install fixes) (#879) ... * feat(plan): default to the grid look + look-and-feel image chooser - Flip gridEnabled default back to true (classic grid / floating-card look), reverting #863's flat-by-default so existing users without an explicit choice return to the grid view they're used to. - Rewrite the look-and-feel announcement into a two-image chooser: Grid (classic, now the default) vs Clean (the new flat look), each a clickable screenshot that hover-zooms to preview — mirroring the code-review DiffTypeSetupDialog pattern. - Bump the announcement version (1 -> 2) so existing users are re-offered the choice once. - Add look-grid.png / look-flat.png preview assets (resized app screenshots). * fix(review): restore GitLab /diffs fallback for MR review #871's pr-gitlab refactor moved to the raw_diffs endpoint and deleted the paginated /diffs path, hard-breaking MR review on self-hosted GitLab too old to expose raw_diffs (opaque "Failed to fetch MR diff") and silently rendering zero files when raw_diffs returns empty for oversized MRs. Keep raw_diffs as the primary path (it preserves binary markers + collapsed content), but fall back to the restored paginated /diffs + reconstructPatch when raw_diffs fails or is empty, and throw a clear "diff is empty / too large" error if both come back empty. Restores parsePaginatedArray / reconstructPatch and adds tests for the fallback paths. * fix(install): bound the optional sem sidecar download The semantic-diff 'sem' download (#871) ran curl / Invoke-WebRequest with no timeout. The sidecar installs after plannotator itself, so a slow or hung fetch could wedge an otherwise-complete install. Add --connect-timeout 10 --max-time 120/60 (sh, cmd) and -TimeoutSec 120/60 (ps1) so it times out and skips gracefully, and document the PLANNOTATOR_SKIP_SEM_INSTALL opt-out in the installer help. * chore(install): remove the Glimpse install option from the installers The installers offered to `npm install -g glimpseui` (a wizard prompt + --glimpse/--no-glimpse flags + saved pref). Drop all of it across install.sh, install.ps1, and install.cmd: flags, usage text, the wizard question, the glimpse_present detection, the saved-pref read/write, the persistence clauses, and the install block. The guided wizard is now two questions (extras, model-invocable). Runtime Glimpse support is unchanged: the app still opens in the native window if glimpseui is on PATH (packages/server/browser.ts), still gated by PLANNOTATOR_GLIMPSE. Updated install.test.ts (asserts the installers are glimpse-free) and the installation docs. * feat(plan): 0.20.0 release announcement dialog Turn the first-run look chooser into a two-page 0.20.0 announcement. - Page 1: header with a "Full release notes" link, a four-up feature grid (fresh look, semantic review, multi-repo review, leaner install), and the grid/clean plan-look chooser. - Page 2: "Workspaces are coming" teaser with the waitlist image + link. - Footer actions grouped bottom-right on both pages (shimmering "Workspaces are coming" teaser via TextShimmer + the primary action) so focus stays in one corner across the page turn. - Reshoot the grid/clean chooser screenshots on a clean prose plan and add the workspaces teaser image. * feat(plan): add full-page HTML feature card, lead with leaner install Add a fifth what's-new card for --render-html (annotate HTML reports and explainers rendered full-screen) and move Leaner install to the front of the grid.	2026-06-09 09:45:20 -07:00
Michael Ramos	c3ba55e889	feat(review): add semantic diff overview (#871) ... * feat(review): add semantic diff overview * fix(review): address semantic diff review findings * fix(review): harden semantic diff fallback * fix(review): avoid repo-local sem execution * fix(review): normalize semantic diff cwd * test(review): cover semantic diff local cwd * fix(gitlab): fetch raw MR diffs * fix(review): harden sem path resolution * docs(review): add semantic diff handoff * feat(review): semantic diff cards, header sem badges, jump-to-line - Restyle the semantic panel as real bordered cards (shadcn surface) instead of ASCII box-drawing; centered column, grid-aligned entity rows. - Hide orphan (module-level) changes from the rows, matching sem's default; the count still surfaces in the summary line. - Add a 'sem · N' hover popover to each diff file header showing that file's semantic changes, reusing the same row component as the panel. - Clicking an entity (panel or popover) scrolls the diff to the lines via pierre's [data-selected-line]; centers only when off-screen so manual drag-selection isn't disturbed. - Extract shared rows/helpers into semanticDiffShared; share one cached /api/semantic-diff fetch across header badges. * feat(review): land on All files by default Semantic diff stays available via the file-tree nav entry and header badges, but it's no longer the initial landing view. * chore(review): drop dead change-symbol entries, log badge fetch failures - Remove unreachable 'moved'/'renamed' entries from the changeSymbols table (getChangeSymbol early-returns for those before the lookup). - Log a console.error once per patch when the file-header badge's semantic diff fetch fails or returns a non-ok status, so a systemic failure leaves a trace instead of every badge silently showing nothing. * feat(review): flatten semantic diff panel into grouped list - Dissolve the per-file bordered cards into flat sections: the file path is now a quiet underlined header (single hairline) with entity rows flush beneath, hierarchy from whitespace + hover tint instead of boxes. - Split the path into muted directory + emphasized filename for faster scanning. - Nudge the add/remove glyphs (⊕/⊖) ~15% larger, line-height pinned so rows don't grow; modified/rename/reorder glyphs unchanged.	2026-06-08 22:23:54 -07:00
Michael Ramos	b19505efd3	chore: remove the redundant /plannotator-status and /plannotator-archive commands (#873) ... Two agent command-surface cleanups. Both remove only the command entry points; all underlying infrastructure stays. 1. /plannotator-status (Pi): removed — it echoed phase/plan-file/progress on demand, but that state is already shown ambiently (status bar + live checklist widget). The phase/checklist state machine is untouched. 2. /plannotator-archive (all agents): removed the command/skill entry points across every surface — Claude/Codex/Kiro skills, Pi, OpenCode (handler + dispatch + cli-bridge + embedded + stub), Droid, the Kiro agent prompt, all three installers, docs, marketing, and the CI deprecated-command guard. The installers also gained a stale-skill cleanup so upgraders drop a previously installed plannotator-archive skill. Kept (infrastructure) — archive browsing stays available in-review via the sidebar: the `plannotator archive` CLI subcommand (apps/hook/server), the mode:"archive" server path + /api/archive endpoints, ArchiveBrowser/useArchive, the sidebar Archive tab, sessions.ts "archive" mode, and ~/.plannotator/plans storage. Verified: bun test scripts/install.test.ts → 72 pass; pi-extension typecheck + build:opencode pass; repo-wide residual scan clean; KEEP-set integrity confirmed; one orphaned import (opencode commands.ts) caught in self-review and removed.	2026-06-08 11:08:11 -07:00
Michael Ramos	aac5aacbe8	fix(install): restore /plannotator-* bash execution on Claude Code + harden unattended installs (#872) ... Follow-up to #850 (unreleased), fixing two pre-release regressions vs v0.19.27. - Claude Code: add apps/skills/claude/* skills using `!`plannotator <sub> $ARGUMENTS`` + allowed-tools + disable-model-invocation, so /plannotator-review|annotate|last| archive auto-run with no permission prompt and pass arguments through — matching the old slash commands. Codex keeps the prose apps/skills/core/*. Installers route ~/.claude/skills <- apps/skills/claude and ~/.agents/skills <- apps/skills/core. - Installers: gate the wizard so an unattended-but-open /dev/tty can't hang (PLANNOTATOR_PROMPT_TIMEOUT, default 30s; timeout/EOF resolves to safe "no"). Don't gate on $CI (explicit --reconfigure/--extras must win). Don't persist timed-out wizard answers as install-prefs. Keep ask_yes_no's read in a tested context so set -e never aborts on a timeout. 72 installer tests pass.	2026-06-08 10:28:18 -07:00
Oscar Silva	f08764063d	feat(review): support multi-repo workspace reviews (#543) ... * feat(review): support multi-repo workspace reviews (#527) * fix(workspace): address critical issues from deep review - Fix race condition in label generation by pre-computing labels sequentially - Fix rewritePatchLine to support quoted paths and rename/copy headers - Add separator between aggregated patches to avoid invalid diffs - Normalize input paths in resolveWorkspaceFilePath - Add timeout to PR discovery (15s) to prevent server hangs - Fix PATCH /api/workspace/repo to rollback state on failure via applyRepoMutation - Validate body.source runtime (must be 'local' or 'pr') - Snapshot active repo in agent jobs at launch to prevent race in onJobComplete - Prevent double-prefixing of agent findings when paths are already prefixed - Fix frontend findWorkspaceRepoForPath to use longest-prefix matching - Fix shared types: diffType uses DiffType, platformUser is string | null * fix: remove duplicate gitRuntime export in vcs.ts * fix: resolve remaining merge conflict in local review mode, remove stale detectManagedVcs import * Add local multi-repo workspace review support * Fix workspace review edge cases * Remove session query from browser launch * Add switchable workspace review modes * fix(review): cover opencode workspace bridge * fix(review): clean workspace review plumbing * fix(review): clarify workspace agent finding paths * fix(review): preserve diff paths with spaces --------- Co-authored-by: Michael Ramos <mdramos8@gmail.com>	2026-06-05 14:29:38 -07:00
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
Michael Ramos	3de555f5e5	Fix OpenCode plugin runtime compatibility (#849) ... * fix(opencode): add host-compatible runtime bridge * fix(opencode): preserve parity in cli bridge * test(opencode): add isolated sandbox launcher * test(opencode): keep reusable sandbox launchers * test(opencode): export local plugin default * test(opencode): install OpenChamber deps when needed * test(opencode): avoid OpenChamber default port collision * fix(opencode): harden cli bridge fallback * test(opencode): clean isolated sandbox helpers	2026-06-04 18:14:05 -07:00