mirror of
https://github.com/backnotprop/plannotator.git
synced 2026-09-14 14:17:26 +08:00
codex-mobile-touch-selection
82 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Michael Ramos
|
cfd3907788 |
test(review): cover the compact-touch preserved range on both diff surfaces
DiffViewer.compactTouchSelection.test.tsx drives the FileDiff options the component hands Pierre: a drag preserves the range instead of opening the composer and paints it through selectedLines, a second drag repaints through onLineSelectionChange, the gutter action opens the composer, a cleared range still reaches the toolbar host, and desktop keeps routing completed drags straight to the composer with no change handler at all. The last three assertions fail against the pre-fix component. The AllFilesCodeView compact test only checked what was published upward, which a range nothing paints would also satisfy; it now asserts the range reaches the CodeView props. That needs the App loop, so the mount feeds published selections back down as pendingSelection: without it the reconcile effect clears the highlight the preserve branch just painted. |
||
|
Michael Ramos
|
64062af9a1 |
feat: Portable Guided Reviews — export, share links, agent-authored guides, guides.show (#1324)
A Guided Review can now leave Plannotator: as a single self-contained HTML file that renders exactly like the in-app guide, as an encrypted-by-default share link on guides.show, or authored by any agent through the new guide CLI. Highlights: packages/guide-viewer extracted from review-editor at the injection seam (read-only host, no third renderer); guides.show Worker with R2-backed share storage, per-IP rate limiting on creation, delete tokens hashed at rest, and 128-bit ids; portable exports pin the viewer by SRI hash with budget and manifest gates in PR CI and at deploy; two-runtime parity across Bun and Pi verified; v0.27.x saved guides load unchanged. Retention is indefinite by explicit decision, to revisit with the lean sharing refactor. Decision record: adr/decisions/007-portable-guided-reviews-20260815.md |
||
|
Michael Ramos
|
d6d727b34f | ci(release): add SBOM and Grype release gate (#1298) | ||
|
Michael Ramos
|
58598bbf2b | ci(security): add isolated ZAP DAST monitoring (#1299) | ||
|
Michael Ramos
|
64e3fa7762 |
feat(review): add compact touch review shell (#1301)
* feat(review): add compact touch review shell * fix(review): let submission dialog own initial focus * fix(review): refine compact mobile review chrome * fix(review): restore reliable mobile diff scrolling * fix(review): preserve mobile file identity * docs(mobile): specify phase 2b plan shell * fix(review): close mobile shell regressions * fix(review): restore narrow overview stacking * test(review): preserve real syntax theme resolver |
||
|
Michael Ramos
|
d3633c9c52 |
Mobile foundation and quieter first run (#1295)
* feat: establish mobile foundation and simplify onboarding * fix(ui): finish mobile foundation cleanup |
||
|
Michael Ramos
|
356b628b6f |
ci(security): add Semgrep CE and Trivy monitoring (#1294)
* ci(security): add Semgrep CE and Trivy monitoring * fix(ci): diagnose Trivy coverage assertions * fix(ci): accept Trivy repository scan metadata * fix(ci): harden scanner failure diagnostics |
||
|
Michael Ramos
|
3282be673b | fix(review): hide viewed and stage controls in file headers when toggled off (#1288) | ||
|
Michael Ramos
|
d4ce3dcb57 |
ci: harden releases and add security scanning (#1274)
* ci: harden release and add security scanning * Harden release and deploy recovery paths * Fix npm artifact pack destinations |
||
|
Michael Ramos
|
8e7b5ce300 |
feat(review): refine Call Flow navigation and annotations (#1277)
* feat(review): refine Call Flow navigation and annotations * fix(review): align viewed controls with panel navigation * feat(review): add Call Flow path search controls * fix(ui): wrap long tooltip identifiers * feat(review): annotate raw Call Flow output * feat(review): refine call flow lens context * fix(review): keep call flow lens search accessible * fix(review): scope call flow find shortcuts |
||
|
Michael Ramos
|
fc348687bf |
fix(review): contain /api/call-flow analysis throws as JSON error responses (#1272)
* fix(review): contain /api/call-flow analysis throws as JSON error responses
A hard VCS failure during patch materialization escaped the handler in
both runtimes. On Pi the unhandled rejection reached the process-level
handler and killed the user's session; on Bun it surfaced as a non-JSON
500 the client's quiet-failure UX could not parse. Both handlers now
return the standard { status: "error", reason: "analysis-failed" }
envelope.
* fix(review): cut the Call Flow consent copy down to the three facts that matter
Six sentences of disclosure read as noise. The dialog and Settings now
say: what it does, what it installs (languages + size), Node 22+, and
that other languages install as needed. Nothing consent-relevant was
removed.
* test(review): pin consent-copy facts, not prose
The presentation test now asserts the server-derived facts (languages,
size, Node floor); the dialog and Settings tests assert only that the
disclosure prop renders, via a sentinel string. Copy edits no longer
break three test files.
* docs: add Testing Rules to AGENTS.md (no prose-pinning, no round-trip prop tests)
* docs: refine copy-pinning rule — deliberate locks allowed, incidental snapshots banned
* fix(review): use the maintainer's Call flow description in the intro dialog and Settings
* fix(review): Call flow description is the maintainer's exact copy; remove the dynamic disclosure plumbing
The intro dialog and Settings now show only: 'Diffs for function call
stacks across git commits. 22 languages supported (AST-based, built
using Tree-sitter).' The callFlowEnableDescription prop, its App wiring,
and getCallFlowEnableDescription are removed; install size and Node
requirements remain visible in the Call Flow panel itself.
* fix(review): reject empty-path worktree diff types; clean up QA findings
- parseWorktreeDiffType returns null for a worktree diff type with no path.
An empty path resolved to an empty cwd, and Bun.spawn({ cwd: "" }) runs
git in the server's own directory instead of the target repo, so a
malformed 'worktree:' switch returned an unrelated checkout's diff.
Fail closed to the caller's real cwd. (Pre-existing; surfaced by QA.)
- Remove an orphaned JSDoc comment left by the callFlowEnableDescription
prop removal in Settings.tsx.
- Add useCallFlowAnalysis.test.tsx to the CI DOM_TESTS list; its two
tests were silently skipping on every run.
|
||
|
Michael Ramos
|
caf7ce1ccd | feat(review): install Call Flow automatically in the background on opt-in (#1271) | ||
|
Michael Ramos
|
9ee2e83287 |
feat(review): make the CallDiff runtime a strictly opt-in, in-UI install (#1270)
* feat(review): make the CallDiff runtime a strictly opt-in, in-UI install
The merged CallDiff integration eagerly installed a ~784MB runtime for
every user at install time, for a feature that is off by default. The
runtime is now strictly opt-in and the opt-in lives in the review UI:
toggle Call flow, click Install in the panel, watch staged progress, and
use the analysis in the same session.
Installers: the default sequence no longer installs the runtime. Opt in
with --with-call-flow (PowerShell: -WithCallFlow),
PLANNOTATOR_INSTALL_CALLDIFF=1, or { "installCallFlow": true } in
config.json (flag > env > config). PLANNOTATOR_SKIP_CALLDIFF_INSTALL is
deleted; --minimal keeps excluding the runtime; the installer prints an
honest note pointing at the in-app install. The headless CLI path
(plannotator install-runtime call-flow) is unchanged.
Server (both runtimes, contract-identical): POST /api/call-flow/install
starts installCallFlowRuntime() in the background via a single-flighted
coordinator (concurrent POSTs join the in-flight install), runs a
Node 22+ preflight before any download (distinct node-unavailable
error), and rejects cross-origin POSTs with 403. GET
/api/call-flow/install-status reports idle/running/done/error with
stage: downloading, verifying, installing-deps, building. Install
completion invalidates the 30s runtime probe cache so the next
capability advert resolves available without a server restart.
Client: the Call flow Dock's runtime-missing state is now the opt-in
funnel with an honest disclosure (about 800 MB on disk, Node 22+,
one-time), staged reduced-motion-safe progress, and error + retry with
a no-node hint. On done the advert is refreshed through
POST /api/review-analysis and the existing available-change refetch
starts the analysis for the current snapshot with no reload. The intro
dialog and Settings toggle note the separate first-use runtime.
Docs: AGENTS.md env table + Review Server API table, marketing
environment-variables / installation / ui-settings / code-review /
api-endpoints pages, and the CallDiff ADR runtime-boundary and server
contract sections.
* test(review): stop leaking PLANNOTATOR_DATA_DIR from the install endpoint tests
The call-flow install endpoint tests overrode PLANNOTATOR_DATA_DIR at
module-eval time and never restored it. bun runs CI's full suite in one
process and evaluates every test file's module before running tests,
while Pi's generated/storage.ts caches its data dir at import time; the
override therefore made storage's cached dir and later files' live
getPlannotatorDataDir() calls disagree, failing the Pi annotate-history
unwritable-dir test and both durable-submit-record tests.
An afterAll restore alone is not enough: it reproduces the same three
failures with the mismatch inverted (storage caches the leaked dir at
module eval, tests then run against the restored one). The env var is
now never touched at module-eval time at all; it changes only inside
tests and is restored to its original value in afterEach, exactly like
the PORT/PATH pattern. The config writes the advert tests persist
through the process's frozen config module are snapshotted at load and
restored in afterAll so a standalone run never flips a real
config.json setting, and the process-global scope of the mock.module
seams is documented.
Regression proof (previously failing in either mismatch direction, now
green in both orderings):
bun test packages/server/call-flow-install-endpoint.test.ts \
apps/pi-extension/server/annotate-history.test.ts \
apps/pi-extension/server/annotate-submission.test.ts
* feat(review): install CallDiff grammars selectively
* fix(review): harden CallDiff worker environment
* fix(review): close CallDiff verification gaps
|
||
|
Michael Ramos
|
e53a933106 |
fix(ci): bump Bun build pin to 1.3.14 for sandbox env loading (#1249) (#1250)
Bun <= 1.3.11 loads an empty process.env when a cwd ancestor directory is unreadable, the normal state inside OS sandboxes (Seatbelt/Landlock): every released binary silently ignored all PLANNOTATOR_* env vars there (oven-sh/bun#27802, fixed in 1.3.13). The 1.3.11 pin existed for the Bun 1.3.12 cross-compile signing regression (#541, binaries SIGKILLed on macOS); verified gone on 1.3.14: cross-compiled darwin-arm64 output carries the same linker-signed CodeDirectory as the known-good shipped binaries and executes cleanly on macOS 26.3, and the unreadable-ancestor env repro passes on a 1.3.14 build. Guardrails so neither regression class can ship silently again: the release smoke matrix gains a macOS arm64 leg (executing the binary is the signing assertion), and a new smoke step runs the annotate server from a cwd with an unreadable ancestor, where responding on the fixed PLANNOTATOR_PORT proves env vars were read. |
||
|
Michael Ramos
|
4d9147200d |
ci: wire 8 orphaned DOM-gated test files into the DOM test step
The DOM step runs an explicit file list, and eight describe.if(hasDom) suites were never added to it, so they silently reported 0 pass / all skip on every CI run: the three new #1243 suites (htmlPinpointProtocol, htmlChrome, inputMethod) plus five older vim/highlight suites (vimHud, Viewer.vimMode.integration, useVimSelection, codeHighlight, vimNavigation). Found by the pre-release sweep. All 69 tests pass together locally under the exact CI invocation. |
||
|
Michael Ramos
|
005c32c7c8 |
ci: remove the install-script auto-sync job (manual sync only)
The deploy-install-scripts job added in #1214 assumed the CI deploy role could write to the plannotator-install-scripts bucket. It cannot, by design, and the job's first-ever real execution (the #1239 installer change) failed on AccessDenied. Install-script syncs are manual with local credentials; the workflow now carries a comment pointing at the release runbook's exact commands instead of a job that cannot succeed. |
||
|
Raúl
|
183cae2c44 |
fix(vim): keep j/k cursor clear of HUD bands when scrolling (#1154)
Document Vim navigation moved the cursor with
`Element.scrollIntoView({ block: 'nearest' })`, which parks the target
flush against the nearest viewport edge — exactly where the sticky action
bar (top) and the key HUD / status pill (bottom) float. Motion to the top
or bottom of a document then hid the caret behind an overlay, while a
mouse wheel (which the browser lets overshoot) kept the same line nearer
centre.
Add vimScroll.ts: a pure computeVimScrollDelta returning the signed
scrollTop delta needed to clear a HUD band at each edge (0 when the
target is already inside the safe band; a target taller than the band
aligns to its top edge so reading order wins), resolveVimScrollMargin
sizing the fallback band as clamp(20% of viewport height, 24px..160px),
and a scrollVimTargetIntoView wrapper.
Both bands are measured from live geometry instead of guessed constants:
the top band widens past the ratio margin to clear the sticky action
bar, and the bottom band derives from the portaled key HUD / mode pill
rects ([data-vim-key-hud] / [data-vim-mode-badge]). The opt-in key HUD
(fixed bottom: 150, height: 88 — a ~238px band, past the 160px clamp) is
actually cleared, while the default pill reserves ~49px instead of
over-reserving 160px. An expanded key HUD is a deliberate modal state
that can outgrow the viewport, so it keeps the ratio margin.
The scrolling element is the native-scroll host fed through
ScrollViewportContext, so the wrapper takes that element from the caller
(useScrollViewport() in Viewer, the same node the reticle measures
against) and falls back to the historical scrollIntoView when it is
absent, so behaviour never regresses.
Route every cursor/target move in useVimSelection through it, and add
vimScroll.test.ts to the DOM allowlist in test.yml so its integration
tests run in CI.
|
||
|
Michael Ramos
|
7ad4d39ed9 |
feat(comments): reference agent skills with / or $ in plan review and annotate comments (#1229)
* feat(comments): reference agent skills with / or $ in plan and annotate comments
Typing / or $ at the start of a word in the document-UI comment composer
opens a picker of the user's global agent skills (~/.claude/skills,
~/.codex/skills, ~/.agents skills roots), served by a new GET /api/skills
on the plan and annotate servers in both runtimes (Bun + Pi mirror).
Multiple references per comment are supported; references live in the
comment text itself and are appended to exported feedback as a
'Skills referenced' block so the acting agent knows which skills to apply.
Human-invocation-only skills (disable-model-invocation: true frontmatter)
stay listed and selectable but render dimmed with a badge, warn in the
menu and composer, and are marked in the export so the agent is never
asked to invoke something it cannot.
Discovery reuses the review-skill loader (same roots, precedence, and
skip-and-log discipline), reads only an 8KB head per SKILL.md, caps the
catalog at 500 skills, takes no client input, and is never persisted;
any failure degrades to plain typing.
* fix(comments): harden skill references per review (trigger, IME, seam, fail-closed frontmatter)
Blockers:
- B1: a trigger now requires at least one query character. A bare / or $
no longer opens the catalog, so Enter stays a newline and Tab still
leaves the field ("This costs $" + Enter, "cd /" + Tab, bullets).
- B2: the menu ignores keys mid-IME-composition (nativeEvent.isComposing),
matching the 16 existing guards; Enter committing a Pinyin/Telex/Korean
candidate can no longer insert a skill.
- B3: the catalog request is a host seam (skillCatalogTransport via
configurePlannotatorUI), defaulting to the existing GET /api/skills.
- B4: resetSkillCatalogCache() invalidates outstanding requests
(generation counter), and a late-resolving stale request can no longer
overwrite a newer cached value or the export registry. The catalog
tests reset in beforeEach, so they hold in any file order.
Also:
- F1: skillReferences={false} is fully inert — the human-only notice memo
and the cache seed are gated on the prop.
- F4: frontmatter flag parsing no longer fails open: trailing YAML
comments are stripped, on/1 (and TRUE/yes etc.) read as true, the head
read is 64KB, and truncated unterminated frontmatter fails CLOSED on
disable-model-invocation.
- F5: extraction ignores markdown link destinations ([x](/name)), shell
redirects (cat /x > out), and /-triggered FHS root names (/run, /tmp);
menu insertion switches / to $ for those names so inserted references
always survive extraction.
- F6: the 500-skill cap slices after sorting, so which skills survive no
longer depends on readdir order.
- F3: /api/skills wiring guards for the Bun and Pi plan + annotate
servers (skills-endpoint.test.ts).
- Keyboard state machine tests against the real CommentPopover in
happy-dom (bare trigger, insertion, composition, Escape, highlight
bounding, opt-out inertness), added to the CI DOM step.
- The insertion path dismisses the trigger start so the menu close is
ordering-safe against React's select-plugin re-reading a stale caret.
* feat(comments): redesign the skill reference menu (bare triggers, no preselection, highlighted tokens)
Per maintainer direction, reversing the earlier bare-trigger opt-out
deliberately: typing a bare / or $ at the start of a word now opens the
full skill catalog immediately, and the safety story moves from the
trigger to the menu itself.
No preselection (the load-bearing rule): the menu opens with NO row
active, and while nothing is active every key behaves exactly as if the
menu were closed. "This costs $" + Enter is a newline; "cd /" + Tab
leaves the field (the proven regression that must never return). A row
activates only via ArrowDown/ArrowUp (Down from none lands on the first
row, Up on the last); only then do Enter/Tab insert. Pointer hover never
activates a row, because the menu floats exactly where the mouse rests
over the composer; a click inserts directly and never arms Enter.
Continuing to type re-filters and disarms any active row. Escape clears
the active row and dismisses when the user engaged (query typed or row
active); an unengaged bare-trigger menu passes Escape through so closing
the composer still costs one press.
Menu redesign to the reference look: icon, bold name, dimmed inline
description with ellipsis, right-aligned source column (Agents / Claude
/ Codex from the discovery roots), rounded generously padded rows, and a
subtle active-row background; human-only rows stay dimmed with their
badge and the warning now shows while such a row is ACTIVE.
Inserted references render highlighted in the composer via a mirrored
aria-hidden overlay behind a transparent-text textarea (identical font,
padding and wrapping metrics; scroll synced; tokens change color and
background only, drawn from the --primary theme token so every palette
works in light and dark). The caret keeps --foreground, selection uses a
translucent primary wash, and IME composition temporarily restores
native textarea text so composition underlines render normally.
skillReferences={false} still renders the plain pre-feature textarea.
Also, per review:
- extraction: dropped the over-broad shell-redirect exclusion (false
negatives on prose like "use /animate <- this one"; the motivating
case stays covered by the reserved-path rule)
- frontmatter: an unterminated frontmatter block now fails CLOSED on
disable-model-invocation even in complete (untruncated) files
- the reserved-path / to $ insertion switch stays: extraction still
reads /run as a path, and the new token highlight makes the switch
self-explanatory (an unhighlighted insert would look broken)
The composition guard, transport seam, catalog generation counter,
enabled gating, and export rules are unchanged and re-covered by the
rewritten DOM test matrix.
* fix(comments): give the skill reference menu adaptive, viewport-clamped placement
The menu rendered bottom-full with a fixed max-h-64: always upward, up to
256px, with no viewport awareness. With the comment popover near the top of
the viewport (annotating near the top of a document), typing a trigger ran
the menu off the top of the screen with its upper rows unreachable.
Placement now mirrors the popover's own computePosition idiom: measure the
space above and below the composer wrapper against window.innerHeight,
prefer above (the shipped direction; keeps the action row and human-only
notice visible), flip below when the list fits below but not above, and when
neither side fits pick the roomier side. The list's max height is clamped to
the available space (still capped at the former 256px), so the menu never
extends past a viewport edge. Recomputes on every commit (drag moves,
popover flips, filtering changing the item count, warning-footer toggles)
plus capture-phase scroll and resize listeners, matching the popover's
tracking. Visual design of the menu and rows is unchanged.
* feat(comments): inject human-only skill instructions into exported feedback
A human-only skill (disable-model-invocation: true) referenced in a review
comment used to export as a dead name the agent could do nothing with. A
human referencing a human-only skill IS the human invocation, so the export
now injects the skill's SKILL.md body verbatim (frontmatter stripped) inside
clearly delimited BEGIN/END SKILL INSTRUCTIONS markers, with the absolute
skill directory and the resolve-relative-paths pointer so references/,
scripts/, and assets/ stay actionable. Model-invocable skills keep exporting
as names the agent can invoke itself.
Transport is lazy: a new GET /api/skills/content?name= endpoint (Bun and Pi)
serves one discovered skill's body, capped at 20k chars with an explicit
truncation notice pointing at the file; the client fetches contents only for
the human-only skills actually referenced, keyed off comment state, and the
catalog now carries each skill's absolute dir so every failure path (deleted
skill, unreadable file, race with submit) degrades to naming the skill plus
its directory. Names are matched against discovery only and never used as
paths, so traversal cannot escape the skill roots. A per-export dedupe
injects each skill once even when several comments reference it, and
GLOBAL_COMMENT annotations run through the same block.
The referenced-skills header now says the reviewer is asking for the
invocation, and the human-only menu footer and composer notice explain that
the skill's instructions will be included with the feedback instead of
warning that the reference will not work.
* polish(comments): quiet, progressive human-only skill treatment
The human-only surfaces shipped with too much emphasis: a dimmed row plus
a bordered uppercase badge, an amber warning footer, and a persistent
amber notice in the composer after insertion. Human-only is a property of
a skill, not an error state, so the treatment is now quiet and
progressively disclosed:
- Menu rows render at full strength with a small muted 'human-only' pill
(bg-muted / muted-foreground tokens; no border, no dimming).
- The plain-language explanation (a model cannot invoke it, so its
instructions will be included with your feedback) appears as a muted
footer only while a human-only row is active (keyboard) or hovered
(pointer). Hover disclosure is purely visual state local to the menu;
it never touches activeIndex, so the no-preselection invariant and the
hover-never-arms-Enter rule are unchanged and re-asserted by a new test.
- When not disclosed, the same sentence stays in the DOM sr-only and
human-only rows point at it with aria-describedby, so the state reaches
assistive tech as text rather than as a purely visual badge (this does
not attempt the #1233 combobox semantics, and does not worsen them).
- After insertion, the highlighted token itself carries the quiet inline
marker (a dotted primary underline; text-decoration cannot move glyphs,
so overlay alignment is untouched) and the standing amber notice is
replaced by a native <details> disclosure: a single muted 'Includes
skill instructions' summary line that expands to the full accurate
sentence, operable by pointer, keyboard, and AT alike.
No amber remains; every color is a theme token (muted, muted-foreground,
border, primary, ring), so the treatment follows every palette in light
and dark. Copy is unchanged where it was accurate. Behavior is unchanged:
human-only skills stay selectable and injection still happens.
* fix(comments): harden human-only skill injection per adversarial review
Three findings on the injection path, each with tests that fail pre-fix:
1. Marker forgery: an injected SKILL.md body containing our own
`--- BEGIN/END SKILL INSTRUCTIONS ---` markers (or an
`[Instructions truncated:` notice) could close the block early — making
everything after it read as the reviewer's own words — forge a block for
a skill nobody referenced, or forge a truncation notice pointing at an
attacker-chosen path. Body lines matching the structural marker forms
(leading-whitespace and case variants included) are now visibly
neutralized before injection: kept verbatim but prefixed, never silently
deleted (neutralizeSkillMarkerLines).
2. Forged human invocation: POST /api/external-annotations is
unauthenticated on localhost, so any local process could submit a
comment referencing a human-only skill and cause its instructions to be
injected "at the reviewer's request". Annotations carrying a `source`
now still LIST their skill references but never cause verbatim
injection — human-only references fall back to naming the skill plus
its directory, with an honest reason. The content-prime effect skips
external texts for the same reason. A human referencing a human-only
skill IS the human invocation; a tool is not.
3. Unbounded read: readReferenceSkillContent read the whole SKILL.md
before slicing to the 20k cap, so an unauthenticated no-cors fetch loop
could balloon RSS by file size per request (measured +64.4MB for a 64MB
file). It now uses the same bounded readFileHead as the catalog,
reading only frontmatter allowance + 4 bytes per capped char + slack;
truncation detection is unchanged for any file whose frontmatter fits
the catalog bound, and frontmatter that overflows the read falls back
to null rather than serving raw YAML. Measured: 12 reads of a 64MB
SKILL.md now cost +5.1MB total.
Also: the fast-fail guard no longer rejects legitimately discovered names —
`name.includes("..")` 404'd a real `v1..2` skill dir forever (and `\` is
legal in POSIX names) while defending nothing, since the name is only ever
matched against discovery output and never joined into a path. It now
rejects exactly the names that can never be a readdir entry: empty, `.`,
`..`.
|
||
|
Michael Ramos
|
548497e6f1 |
fix(annotate): hide the collapsed sidebar tab flags with "Hide tools" (#1226)
The header's "Hide tools" toggle dropped the annotation toolstrip and the HtmlViewer action cluster, but the collapsed sidebar tab flags kept protruding from the left edge, so a rendered HTML page never actually got the whole viewport. Fold both overlay guards into one derived `htmlChromeHidden` and apply it to the SidebarTabs render. The flags unmount rather than fade, so nothing focusable stays in the tab order and no click target sits over the page. The toggle only exists on HTML surfaces, and both restore paths stay visible: "Show tools" lives in the header (never hidden), and Mod+B still opens the sidebar directly. |
||
|
Michael Ramos
|
75e8b78cc7 |
fix(review): stop stubbing files whose worktree content the size probe cannot find (#1220)
The oversized preflight in buildBoundedTrackedDiff mapped every object the cat-file batch could not size to infinity. `missing` is routine: for tree-vs-worktree diffs git hashes the WORKING-TREE content of any path pulled into rename/copy detection and prints that hash in --raw output without ever writing the blob, and partial clones report it for unfetched blobs. Those files were excluded by pathspec and replaced with a contents-free binary stub, so a renamed-and-edited file rendered as a silently empty card and /api/file-content refused it as binary. Missing now means unknown, not oversized, and an unreadable new side is bounded by the working-tree file's stat size instead. Every rendered diff stays bounded git-side by core.bigFileThreshold (#1205), genuinely oversized files still stub via real probe sizes plus the stat door, and a blob git truly cannot read now fails loudly through assertGitSuccess instead of blanking a file. Client side, a chunk with a binary marker and no hunks now renders an explicit placeholder in both the all-files view and the single-file viewer, so an empty card can never again pass for "no changes here". AI-assisted. |
||
|
Michael Ramos
|
7ba4e3b3e4 |
fix(review): mint content-derived diff cache keys so single-file tabs render fully (#1219)
* fix(review): mint content-derived diff cache keys so single-file tabs render fully Single-file diff tabs have not rendered their full-content diff since v0.26.0: the expansion gap bars show no chevrons and clicking them does nothing, at every file size. @pierre/diffs 1.3.2 (the 1.2.8 -> 1.3.2 bump, upstream "Fix diff rerender in edit mode (#878)") added name-based cacheKey defaulting in FileDiff.render: an unset `fileDiff.cacheKey` becomes the file's name. `areDiffTargetsEqual` — the only identity check its render and highlight caches make — compares nothing but that key. DiffViewer renders each file twice on one surviving FileDiff instance (key={filePath}): first the PARTIAL diff from getSingularPatch, then the AUGMENTED full-content diff from processFile once /api/file-content lands. Neither set a cacheKey, so both defaulted to the filename and Pierre served the stale partial render forever. Only the augmented diff is expandable, hence the dead gap bars. Both diffs now mint content-derived keys (`<path>#<hash>` and `<path>#full#<hash>`), matching how AllFilesCodeView already keys its items — which is why the all-files view was never affected. The hash (not patch.length) matters because Pierre's worker highlight cache is a singleton that outlives remounts. The partial diff needs its own key too: with key={filePath} the instance also survives diff-type and base switches, where a same-named new patch would otherwise hit the same name-keyed stale cache. hashString moves from AllFilesCodeView to utils/hashString.ts so both surfaces mint keys the same way. Covered by a new DOM test that mounts DiffViewer against the real @pierre/diffs renderer, holds the /api/file-content response until the non-expandable partial baseline is asserted, then requires the expansion affordances to reach the pixels. It fails against the unfixed tree. * fix(review): explain why an oversized file's card has no diff Files over the 5 MB review limit are replaced by a contents-free stub (buildOversizedTrackedStub, plus the untracked equivalent), which renders as a header-only card with no counts and no explanation. Users read that as a broken diff. The stub now carries an explicit marker line in its extended header (OVERSIZED_REVIEW_STUB_MARKER). A marker rather than a client heuristic because the only other signal, `Binary files ... differ`, is exactly what a genuine binary file emits, so a heuristic would put a false size-cap explanation on every image in the diff. The marker lives in shared/diff-paths so the browser bundle can detect it without pulling in the node-facing review core; both server runtimes pick it up from review-core, which vendor.sh already copies to Pi. Git ignores unknown extended-header lines and @pierre/diffs parses the stub identically with or without it, so nothing else moves. Which files get stubbed is unchanged. Both review surfaces now render one line under the file header saying the file is over the limit and only a stub is shown. * test(review): make the diff-swap proof machine independent, not stopwatch based CI failed two tests that pass locally. Both were timing races, neither was an app bug. 1. DiffViewer.fullContentSwap: the swap assertion carried a 15s internal wall-clock budget, which a cold, contended CI runner blows and a warm laptop clears. Two changes, both aimed at the clock rather than the symptom: - The waits are now budgeted in SCHEDULER TURNS, not milliseconds. A slower box spends longer inside each turn but needs no more of them, so the budget never has to be retuned for CI hardware. - Pierre's shared Shiki highlighter is preloaded before mounting. It is a module singleton, and building it was the entire multi-second cost the old budget was accidentally measuring; warming it moves that work into an unbounded await OUTSIDE the observed window. Disposed in afterAll, because packages/ui/utils/codeHighlight.test.ts asserts the pre-attachment behaviour of that same singleton. Verified against an artificially stalled clock: forcing 20s of dead time into every wait (41s total, far past the old 15s budget) still passes, and with the cacheKey fix removed it still fails on the assertion (not as an opaque timeout) in ~12s. A 20-turn budget with the preload removed and every core saturated also passed 10/10, so 400 turns is a wide margin rather than a guess. 2. App.archiveReadOnly compared the fenced block's innerHTML before and after a click. Since #1218, applyHighlight writes plain text first and swaps in Shiki markup when the grammar attaches, so that MARKUP changes on its own schedule and the assertion was racing the swap. The test is checking that the click opened no mutation entry point, which textContent plus the absence of an annotation <mark> says exactly, and which no highlight swap can perturb. Latent on main; the branch's run happened to lose the race. Also fixed while confirming the above: codeHighlight.test.ts asserted a GLOBAL precondition ("no grammar attached yet") that any earlier file attaching a typescript fence invalidates, so `DOM_TESTS=1 bun test packages/ui packages/editor` failed by file order alone. It now resets the attachment cache through the existing __resetCodeHighlightCacheForTests seam and asserts the contract instead of the run order. Not currently reachable from CI (that file is not in the DOM list), but one list edit away. * test(review): drop the highlighter preload, harden the swap proof, report why a paint is missing The preload added in the previous commit made CI strictly worse, so it is gone. Before it, CI's partial diff painted and only the swap was missing; with it, CI never painted at all. It was an optimization for a theory the evidence has since killed, and it mutated a process-wide singleton to buy it, so it is not worth keeping while the real failure is unexplained. The afterAll dispose that existed only to undo the preload goes with it. What the CI log actually shows: - The "WorkerPoolManager: operation canceled because the pool terminated" error is inside discardRestoreRender.test.tsx's own group, ~0.3s BEFORE this file's group opens. It is that file's provider unmounting and terminating the pool singleton it created: end-of-file teardown, the same benign noise documented on #1209. It also prints on every local run, where the whole list passes. It is not a mid-test terminator, and nothing in this file uses the worker pool (no WorkerPoolContextProvider is mounted, so useWorkerPool() is undefined and rendering takes the main-thread path). - This file's group prints NOTHING for its whole 10.3s: no console.warn from the stale-content guard, no error. Pierre simply painted nothing. Not reproducible locally: the exact DOM list from test.yml, one bun process, forward and reverse order, 13 runs with every core saturated, all green. So the remaining difference is the environment, which cannot be reasoned out from here. Three changes make the next CI run answer it instead of costing another guess: - renderDiagnostics() dumps what Pierre actually painted (container / separator / chevron / line-number counts plus a markup fragment) when a wait gives up. Prints only on failure, so it is worth keeping. - The precondition is asserted rather than assumed: the REAL getSingularPatch and processFile must produce partial-then-full on these fixtures. Bun's mock.module is process global and an earlier file in this very list mocks '@pierre/diffs', so a leaked mock now fails in milliseconds with a clear message instead of as a render that never arrives. - The first paint is now REPORTED, not asserted. The verdict belongs to the swap; gating on the partial paint let a slow or absent first paint mask the result the test exists for. Removing the cacheKey fix still fails it (verified), because that tree paints no chevrons at any point. Also fixed a real trap in the fixture: the hunk header said @@ -61 while its context lines start at line 59 of both contents. Pierre realigns a misaligned header rather than rejecting it, so it was silently tolerated. * test(review): stop a leaked module mock from silently unrendering the diff tests Root cause, and it was never a timing problem. AllFilesCodeView.lifecycle.test.tsx calls `mock.module('@pierre/diffs', ...)` with a hunk-less `getSingularPatch` and `processFile: () => null`. Bun's module mocks are process global and are not unwound at file boundaries, and that file sits immediately before DiffViewer.fullContentSwap.test.tsx in the DOM step's list. On the Linux runner the stub reached this file; on macOS it did not, which is why 13 local runs of the exact list, both orders, cores saturated, stayed green. It explains both CI symptoms exactly, including the one that looked like a contradiction: `processFile: () => null` means the augmented diff never exists, so no chevrons ever (the failure before the preload); the stub `getSingularPatch` has `hunks: []`, so nothing paints at all (the failure after it). The "WorkerPoolManager: operation canceled because the pool terminated" line was a red herring throughout: it is inside discardRestoreRender's own group, ~0.3s BEFORE this file's group opens, is that file's provider unmounting the pool it created, and prints on every local run too. The precondition assertion added in the previous commit is what proved it, turning a 10.3s mystery into a 0.45ms verdict: 228 | expect(expected?.isPartial).toBe(false); error: expect(received).toBe(expected) Expected: false Received: undefined Fixed at both ends: - Source: the mocking file now captures the real modules before it stubs them and restores both library specifiers in afterAll, so no later file in any run inherits its stubs. This fixes the class for every future DOM test that needs the real renderer, which was the actual leak. - Consumer: the two tests that render against the real @pierre/diffs get their own CI step, the same isolation (and for the same kind of reason) this workflow already gives useFileBrowser.test.tsx. They are removed from the shared list so that step is their single source of truth. The restore above should make this unnecessary; it is not something to bet a green build on from a machine that cannot reproduce the platform behaviour. Verified with a CI-faithful harness: one bun process per step, the exact lists from test.yml, isolated + shared-forward + shared-reverse, 10 iterations with every core saturated, then 6 more after the final split. All green, plus the full suite and typecheck. * docs(test): point the diff-renderer DOM tests at the CI step that actually runs them |
||
|
Michael Ramos
|
c08b188812 |
perf(ui): single Shiki highlighter, palette-matched code blocks, drop highlight.js (#1218)
* perf(build): stub out the dead Oniguruma WASM in every bundle
@pierre/diffs picks its Shiki engine with a runtime ternary:
engine: preferredHighlighter === "shiki-wasm"
? createOnigurumaEngine(import("shiki/wasm"))
: createJavaScriptRegexEngine()
Plannotator pins `preferredHighlighter: 'shiki-js'` (and Pierre's own
default is 'shiki-js'), so the Oniguruma branch never executes. Because
the choice is a runtime ternary, bundlers keep the `import("shiki/wasm")`
edge anyway and inline `@shikijs/engine-oniguruma/wasm-inlined`, a
~622 KB base64 blob, into the single-file HTML builds. The review app
paid for it twice: once on the main thread (via
`highlighter/shared_highlighter.js`) and once inside the `?worker&inline`
Pierre worker.
Alias `shiki/wasm` to a stub that throws if it is ever reached. Wired via
`resolve.alias` rather than a plugin because `resolve.alias` is shared
with Vite's worker build and `plugins` are not.
Highlighting output is unchanged: the JS regex engine was already the one
doing the work. Opting back into 'shiki-wasm' now fails loudly instead of
silently costing every user a megabyte of dead bytes.
apps/review/dist/index.html 19,424,646 -> 18,180,545 (-1,244,101 raw / -463,348 gzip)
apps/hook/dist/index.html 23,032,467 -> 22,410,416 (-622,051 raw / -233,485 gzip)
* perf(ui): consolidate code highlighting onto Shiki, drop highlight.js
The app shipped two highlighters. Shiki already tokenised the code-review
diff pane (via @pierre/diffs, JavaScript regex engine); highlight.js
separately coloured markdown fences and review suggestion snippets at
~982 KB minified for a full build of ~190 grammars. That second
highlighter is now gone.
Every call site moves onto `packages/ui/utils/codeHighlight.ts`, a thin
wrapper over Pierre's SHARED Shiki instance:
CodeBlock, Viewer, PlanCleanDiffView markdown fences
InlineMarkdown code-file hover preview
HighlightedCode review suggestion snippets
Reusing Pierre's instance rather than standing up a second fine-grained
one is deliberate. Pierre imports Shiki's full bundle, so every grammar
and theme is ALREADY inlined in the single-file builds: a separate
highlighter with a curated language list would have duplicated a subset
of bytes that are already there. Sharing costs nothing, gives every
language Shiki bundles instead of a shortlist, and — the point of the
change — guarantees fences resolve the exact same theme the diff pane
resolves.
Theming. `SHIKI_THEME_MAP` / `resolveSyntaxTheme` move from
`packages/review-editor/hooks/usePierreTheme.ts` to
`packages/ui/utils/syntaxTheme.ts`; usePierreTheme re-exports them, so
the review editor's imports are unchanged. `useFenceTheme()` feeds the
components and re-highlights on palette or mode change. Code blocks now
follow the active palette across all ~52 themes in both light and dark,
instead of always rendering github-dark and relying on hand-written
`.hljs-*` override stacks to stay legible. Those stacks are deleted:
`packages/editor/index.css`'s light-mode token palette, and
`colorblind.css`'s hand-tuned tokens which existed to APPROXIMATE
@pierre/theme's protanopia-deuteranopia themes that are now simply used.
Behaviour held fixed:
- Language-less fences stay plain text (#1212). No auto-detection
anywhere, including the hover preview, which previously called
`hljs.highlightAuto`. `HighlightedCode` derives its language from
the caller's file path; an unknown extension renders plain.
- `applyHighlight(el, ...)` keeps the imperative `hljs.highlightElement`
DOM contract the annotation layer reaches into, and writes plain text
at final size first so async highlighting causes no layout shift.
Already-attached grammars highlight synchronously — no flicker on
cached highlights.
- It also verifies the rendered text is byte-identical to the source
and falls back to plain otherwise, because annotations address code
blocks by text offset.
- `@plannotator/ui`'s public API is unchanged: the highlighter is a
module-level default like the package's other seams, no new props.
The `hljs` class on fenced `<code>` becomes `pn-code` (it is a
structural hook for blockTargeting, vim navigation and print.css, and it
named a library we no longer ship). `language-*` stays.
apps/review/dist/index.html 18,180,545 -> 17,270,889 (-909,656 raw / -291,921 gzip)
apps/hook/dist/index.html 22,410,416 -> 21,704,434 (-705,982 raw / -238,096 gzip)
Verified the diff pane is untouched: the rendered Pierre shadow-DOM
markup is byte-for-byte identical between an origin/main build and this
one (SHA-256 aa1ee88a…).
* fix(ui): strip stray NUL bytes from the code-highlight source
Two U+0000 bytes slipped into comments in the previous commit, which made
git treat the file as binary. Replaced with spaces; no behaviour change.
* fix(ui): keep code-block annotation marks across highlight swaps
Fenced code is annotated by hand: one `<mark data-bind-id>` inside the
`<code>` element, which `applyHighlight` also owns. Every highlight swap
(palette change, dark/light toggle, or the first async grammar attach
after load) replaces that element's children, so the mark was silently
wiped and nothing put it back. Annotation state, the sidebar panel and
exports were unaffected; the loss was purely visual, and deterministic.
`applyHighlight` now publishes every write through `onCodeHighlightSwap`,
synchronously, immediately after it. `Viewer` subscribes and re-paints the
fence's mark, so a swapped block ends up with BOTH the new theme's tokens
and its annotation. The shared painter (`paintCodeBlockMark`) moves the
token spans into the mark instead of flattening them to text, so creating
an annotation no longer costs a block its colours either.
Being driven by the swap also fixes the cousin race by ordering rather
than timing: share/draft restore runs on a timer after load, and on a slow
machine the first async swap could land after it and wipe the restored
marks per block. A restore that painted before the swap is now
re-established in the same task the swap ran in, and one that runs after
finds the mark already there.
Removal tombstones the id before re-highlighting, because the host drops
the annotation from state a tick later — without it the swap listener
would paint the just-removed annotation back in, and a fence carrying a
second annotation would end up bare.
Also closes the named gap in the WASM coverage: entry-assets only grepped
source, so a future @pierre/diffs bump could reintroduce the inlined blob
through a different import specifier unnoticed. It now greps the built
`apps/{review,hook}/dist/index.html` for the base64 WASM magic, skipping
on an unbuilt checkout and running for real in the CI job that builds the
bundles.
|
||
|
Michael Ramos
|
8396eeb787 |
ci: auto-sync install scripts to their dedicated S3 bucket (#1214)
plannotator.ai serves /install.sh, /install.ps1, and /install.cmd from the plannotator-install-scripts bucket, not the marketing bucket the deploy workflow syncs. Nothing in CI wrote to that bucket, so the served scripts went stale between hand syncs: v0.26.0 shipped while the site still served the July 31 installer, missing --skip-skills, the checkout guard, and the PowerShell < 7.2 fix. This adds a deploy-install-scripts job triggered by changes to the three scripts (plus a workflow_dispatch target), uploading them with content types and invalidating exactly those paths. |
||
|
Michael Ramos
|
7aff70281f |
fix(review): repaint pristine diff immediately on edit-mode Discard (#1209)
writeRestore's combined item write leaves upstream's highlighted render cache serving the stale edited content; clear the live instance's render cache and rerender so the pristine diff paints immediately (plaintext first, then the normal async highlight). Covers Discard, Suggest, finishIfEditing, and the deferred external-teardown restore. Adds a DOM-gated integration test driving the real @pierre/diffs CodeView + EditProvider + worker pool through startEdit -> real Editor.applyEdits -> cancelEdit, asserting the pristine content is back in the shadow DOM within a bounded settle. |
||
|
Michael Ramos
|
e97b703479 |
ci: glob the OpenCode smoke tarball instead of pinning the version (#1203)
The installed-package smoke passed a hardcoded plannotator-opencode-0.25.1.tgz path to the fixture, so every version bump would silently break the job on the bump commit. npm pack writes exactly one plannotator-opencode-*.tgz into RUNNER_TEMP, so a glob resolves to the same file and survives bumps. |
||
|
Michael Ramos
|
10a5104888 |
fix(install): repair the skills checkout guard, add --skip-skills (#1201)
* fix(install): make a failed skills checkout stop reporting success The skills/commands checkout runs in a subshell written as `( set -e; ... ) || checkout_failed=1`. POSIX ignores `set -e` for every command of an AND-OR list except the last, and bash 3.2.57 (what `curl | bash` gets on macOS), bash 5.3, dash, zsh and ksh all carry that suppression into the subshell. The `set -e` was inert. A failed clone therefore ran the whole block anyway, the subshell exited with the status of its trailing `if` (0), `checkout_failed` stayed 0, and the installer printed "YOU'RE ALL SET!" with no skills installed. It also blamed the wrong thing, printing "Tag vX.Y.Z predates the per-agent skill layout" when the real cause was a failed clone. Drop the inert `set -e` and guard the four fetch steps with explicit `|| exit 1`. Everything after the checkout stays best-effort, matching install.cmd, which only checks git clone and lets every xcopy run unchecked. A local cp/mkdir/rm failure must not surface as the "network or git error" message. Verified on bash 3.2.57 in a sandboxed HOME: the failure case now exits 1 with the fetch error and no success banner, and original vs patched success runs produce byte-identical trees (74 paths, 35 files) and identical logs. * feat(install): add --skip-skills opt-out The skills and slash commands come from a sparse `git clone` of the release tag. There was no way to decline that fetch short of --minimal, which also drops the sem sidecar, the agent-terminal runtime, the hooks, and every per-agent config. Anything that installs a tag github.com cannot serve had no option at all. Add --skip-skills to all three installers, following the existing --skip-codex / --skip-gemini / --skip-kiro / --skip-opencode family: CLI flag (-SkipSkills in PowerShell), PLANNOTATOR_SKIP_SKILLS_INSTALL env var, skipInstall.skills config key, resolved flag > env > config. It is not a per-agent switch; it covers every scope the checkout writes (Claude, ~/.agents, OpenCode, Gemini, Kiro), the extras, and the skill-scope cleanup sweeps. Skip means do-not-write: nothing already installed is replaced or removed, and git stops being a hard requirement. The run reports "Skills: skipped (<source>)" and the closing banner no longer claims the /plannotator-* commands are ready, which is the same false-success the checkout guard exists to prevent. Use it in the install-script-smoke job. That job installs a synthetic v9.9.9: the fake curl serves the freshly built binary for any URL, but the skills clone goes to real github.com, where the tag does not and cannot exist. That clone has always failed; it only went unnoticed while the broken guard let the installer exit 0 anyway. The job asserts Codex hook config, not skills, so it opts out rather than ignoring a real error. Both run_installer call sites go through the one function definition. Verified in an env -i sandbox on bash 3.2.57 (what `curl | bash` gets on macOS) with a fake curl and a local stand-in remote. Flag, env var, and config each skip and name their own source; flag beats env=0; env=0 beats config true; an explicit "skills": false stays a veto. Without the flag, pre-change and post-change runs produce byte-identical trees (39 entries) and identical logs. A bad clone URL without the flag still exits 1 with the fetch error and no success banner. A --skip-skills re-run over an existing install leaves all 12 skill and command files byte-identical. The CI step was reproduced locally: both run_installer calls exit 0 and every Codex assertion still passes. * test(install): cover --skip-skills and repoint the pinned source strings scripts/install.test.ts asserts against exact install-script source text, so six assertions broke when --skip-skills landed. Each is repointed at the new string with its intent preserved, not weakened: - The "hook/config writing happens before the git hard-fail" ordering test keeps proving the ordering; it just matches the gate's new conditional form. git being a hard requirement is now a narrower invariant (it applies only when the checkout actually runs), so that is asserted separately rather than dropped. - The skipInstall walk assertions follow codex/gemini/kiro/opencode gaining a skills entry, in install.sh's `for _agent` loop and install.cmd's PowerShell key list. - The three "never remove" sweep assertions follow the Codex stale-skill cleanup gaining its skills-opt-out arm, in all three installers. Add nine tests covering --skip-skills itself in the same style as the per-agent family: flag/switch parsing, PLANNOTATOR_SKIP_SKILLS_INSTALL, skipInstall.skills, and flag > env > config precedence by textual layering, for each installer. Each installer also gets a test that the opt-out bails before the clone and leaves the checkout guard intact (#1201's fix must keep failing a real fetch error), and one that the run reports honestly, never prints the "commands are ready" banner over an empty skills dir, and suspends the extras, the model-invocation rewrite, and the stale-stub sweeps rather than applying them partially. bun test scripts/: 116 pass, 6 skip, 0 fail. Full bun test: 2884 pass, 234 skip, 0 fail. bun run typecheck clean. |
||
|
Michael Ramos
|
d7be3406f6 |
fix(ci): repair the OpenCode 2 installed-package smoke (#1202)
The smoke's wait budgets were sized on a warm macOS dev box (5s to a healthy server, 20s to plugin activation). On a cold Linux runner OpenCode 2 needs longer to boot and has to install the packed plugin plus its whole dependency closure through the fixture's throwaway registry first, so the job has failed on every run since it was introduced. Measured, same opencode2 build and the same packed tarball: macOS, warm caches: healthy 0.8s, plugin activated 6.3s linux/amd64 container: healthy 8.2s, plugin activated 45.3s Raise the budgets to 120s and 300s (overridable via PLANNOTATOR_SMOKE_HEALTH_TIMEOUT_MS / PLANNOTATOR_SMOKE_PLUGIN_TIMEOUT_MS) and give the job a 25 minute backstop. The assertion is untouched: the smoke still requires the plugin registry to report the plannotator plugin. Also make a failure legible and prompt. Each poll gets a per-request timeout so one wedged request cannot swallow the budget, waits report progress, failures carry the elapsed time and the last HTTP status/body, and teardown escalates to SIGKILL and force-closes the registry. The CI failure previously burned five minutes in teardown before printing anything. |
||
|
Michael Ramos
|
6b8723cc57 |
test(review): repair guide virtualization mock and run its tests in CI (#1200)
The AllFilesCodeView lifecycle test stubs '@pierre/diffs/react' with mock.module, which replaces the whole specifier. #1193 added an EditProvider import to AllFilesCodeView, but the stub was never extended, so the file throws at import when run on its own. The three DOM tests #1158 added for Guided Review virtualization were also never registered in the DOM_TESTS=1 CI step, so nothing enforced the 8-viewer mount cap and nothing surfaced the broken stub. |
||
|
Sergiy Dybskiy
|
050dfcda9a |
feat(opencode): add OpenCode 2 plan review adapter (#1194)
* feat(opencode): add OpenCode 2 plan review adapter * fix(opencode): harden V2 review lifecycle * fix(opencode): address V2 review feedback * fix(opencode): update V2 target and isolate tests * test(opencode): assert prompt composition invariants |
||
|
Michael Ramos
|
156761a3fa |
feat(review): author suggestions by editing code in place (experimental, flag-gated) (#1193)
* feat(ui): add experimental editSuggestions setting (cookie-only, default off) Registers the flag in the settings registry and surfaces it under a new Experimental section of the review Display tab. Not synced to server config while experimental; when off, no edit UI renders anywhere. * feat(review): edit-to-suggestion core (adapter wall, derivation, session controller) pierreEditAdapter.ts is the single module allowed to reference @pierre/diffs/edit: the editor chunk loads via dynamic import on first use, types are derived structurally, and the EditProvider factory declines attaches until the module is ready (CodeView retries). deriveSuggestions.ts diffs the session's final contents against the pre-edit new-side content with the diff package and emits one minimal hunk per contiguous changed region, expanding pure inserts/deletes to include an unchanged anchor line so originalCode and suggestedCode stay non-empty. CRLF input is normalized. cloneDiff.ts deep-clones FileDiffMetadata before a session starts; Pierre's editor mutates it in place (additionLines, hunks, editSessionDirty), and the clone is what gets republished when the session ends. useEditSession.ts owns the lifecycle: one file at a time, full-content hydration before entry (partial diffs throw in applyDocumentChange), suggestion creation on completion, and session end as ONE combined item write (edit off plus restored pristine fileDiff with a fresh cacheKey), which is upstream's documented commit pattern and avoids racing CodeView's teardown re-render. persistState is deliberately unused (upstream bug, open PR #1048). Existing annotations are projected to editor severity markers best-effort via onAttach. * feat(review): wire edit-to-suggestion into the plain all-files view (flag-gated) The plain all-files dock panel is the only surface that opts in; Guided Review deliberately stays off because its viewport manager evicts CodeViews beyond ~8 mounted, which would destroy an active editor session. When the flag is off the surface renders byte-identical to before: no EditProvider, no edit props, no header buttons. FileHeader gains the Edit entry button (disabled with a tooltip when content cannot be fetched) and the in-session Editing badge with Suggest/Discard controls. Slot portals republish before React commits state, so header rendering reads the session refs, not state. AllFilesCodeView routes every collapse path through finishIfEditing so a collapse ends the session on our one code path, blocks the annotation toolbar and augmentation applies for the file being edited, and drops session state on fileSetKey remounts (diff switches are user-initiated; in-progress edits are discarded, documented v1 behavior). App.tsx converts derived hunks into ordinary suggestion annotations (type comment with suggestedCode/originalCode, side new, anchored to new-side line numbers), the same shape SuggestionModal produces, so rendering, sidebar, drafts, and feedback export work unchanged. The browser never writes files; the agent applies suggestions. * test(review): cover edit-to-suggestion derivation, clone invariant, adapter wall, and flag-off UI deriveSuggestions: no-op edit, single and multi region edits, whole-line add and delete with anchor expansion, insertion at file start, file emptied, CRLF vs LF, trailing newline. cloneDiff: pristine clone stays byte-identical after the live object is mutated the way Pierre's edit session mutates it. adapterWall: only pierreEditAdapter.ts may reference @pierre/diffs/edit, and only via import type or dynamic import. FileHeader DOM tests (DOM_TESTS=1): zero edit UI without the feature, entry button, disabled tooltip, and the Suggest/Discard session controls; registered in test.yml's DOM step. * fix(review): resolve anchor collisions in suggestion derivation deriveSuggestionHunks could emit overlapping hunks when adjacent regions claimed the same anchor line (a file-start region expanding forward into a line the next region's backward expansion also claimed). An agent applying the suggestions against original line numbers would silently drop one or duplicate code. Regions are now emitted left to right with collision-aware anchoring: backward expansion is preferred but only when the preceding line is not already claimed by the previous hunk; otherwise the region anchors forward (always an unchanged line, and later regions see it as claimed); when both directions are unavailable (previous hunk adjacent and region runs to end of file) the region merges into the previous hunk as one spanning suggestion. A defensive fold enforces the non-overlap invariant (lineStart > previous lineEnd) as a safety net. Tests: unit cases for both reproduced collision shapes plus adjacent regions at file start and end, and a 5000-case seeded fuzz asserting zero overlaps, exact originalCode anchors, and full reconstruction of the edited text under a sequential offset-tracking applier. A 20,000-case run of the same generator reports zero overlaps, zero anchor mismatches, and zero round-trip failures. * feat(review): export originalCode as a Replaces block in feedback Suggestions carried originalCode internally but the export never emitted it, so the applying agent had no way to validate an anchor before applying. Each suggestion now exports a fenced Replaces block (the exact current lines being swapped out) ahead of the existing Suggested code block, for both line and file scoped annotations. SuggestionModal-authored and edit-session-derived suggestions flow through the same single format. A deletion-only suggestion (no suggestedCode) still emits its Replaces block so the anchor stays verifiable. Tests assert the block pairing per annotation, the deletion-only case, and that annotations without originalCode are unchanged. * fix(review): never silently discard a dirty edit session on file-set change fileSetKey changes on sort-order and collapse-default flips as well as diff switches and refreshes, and the remount tears the editor down without a completion callback. A dirty session was dropped silently there, destroying in-progress work, while the file-switch path prompted. The session now keeps the FileContents delivered with each change event (its contents getter is lazy and stays readable after editor cleanup), and handleFileSetChange recovers the last-known document text, derives the suggestions, and prompts to keep them, matching the dirty file-switch pattern. The call site is a post-commit effect, so the synchronous confirm is safe. Declining discards; a clean session or a no-op edit still drops without a prompt. New DOM-gated test (registered in test.yml's DOM step) covers the confirm, decline, clean, and no-op paths through the real hook, plus non-DOM units for the pure recovery helper. * fix(review): give editor markers real range width so they render Single-line annotation markers were projected as collapsed ranges (start and end both at character 0 of the same line); upstream's overlay renderer skips zero-width blocks, so markers never painted. Marker ranges now end at character 0 of the line after the last annotated line (exclusive-end LSP form), covering every annotated line in full; the editor's TextDocument clamps a past-the-end position back to end-of-document. Verified via CDP against a live review server: the severity squiggle renders across the annotated line during an active edit session and the hover popover shows the annotation source and message. Also corrects overstating comments: the flag-off claim that the editor module is never imported (the single-file build inlines dynamic imports, so the namespace exists at page load but stays inert) and the createEditor decline-and-retry framing (the React wrapper throws on an undefined return; the controller awaits the chunk before any item enters edit mode, and onAttach delivers once per editor rather than re-firing across virtualization re-attaches). * polish: drop suggestion card left accent, move Edit to far right of file header Suggestion cards keep the colored SUGGESTION header as their sole identity marker; the green left border on the card body is removed for all suggestion cards (inline and sidebar, including modal-authored ones). The Edit entry button moves to the far right of the file header action row, after the Sem badge and adjacent to the file actions dropdown, so the experimental affordance stays out of the everyday Viewed/Add/Comment cluster. Responsive isVeryTight behavior is unchanged (icon-only at narrow widths). * prototype: edit-session HUD strip below the file header While an edit session is active, a slim strip renders directly below the file header (inside the file card, above the content) carrying the session controls and state: Suggest, Discard, a debounced net change count (for example 3 changes), and the experimental label. The header no longer renders in-session controls; the Edit entry button still lives there when no session is active. The HUD rides Pierre's memoized custom-header slot portal, so the change count is delivered through a small external store (useSyncExternalStore) that useEditSession updates on a 250ms debounce per change event; slot height re-measures via the version-bumped updateItem that session start and end already perform. Prototype for design review; revert this commit to drop the HUD. * fix(review): pierce shadow DOM in file-nav keyboard guards Window-level keydown handlers in SectionsPanel and FileTree guarded only against e.target being an input or textarea. Events from inside the Pierre editor's shadow DOM retarget to the host element, so Home/End/arrow keys typed during an edit session fell through to file navigation and switched the visible file mid-edit. Use composedPath()[0] and isContentEditable, matching the guard AllFilesCodeView already uses. * feat(review): Make annotation selection action in edit sessions Selecting text in a Pierre edit session now shows the editor's Selection Action popover with a single Make annotation button (plain DOM, inline styles, colors via inherited theme tokens so light/dark and theme switches work through the shadow boundary). Clicking it snapshots the selection and opens the app's existing CommentPopover anchored at the button rect; the submitted comment becomes a normal line-scoped CodeAnnotation. Anchoring: the selection lives in the edited buffer, but annotations anchor to the rendered diff's new side, which is the session's pre-edit content. mapEditedRangeToPristine (edit/selectionAnchor.ts) maps the selected line range through diffLines(preEdit, edited) at click time. Unedited regions map exactly; ranges overlapping session edits anchor to the pristine lines those edits replace and are flagged approximate; pure insertions anchor to the adjacent pristine line. Pre-edit coordinates are session-invariant, so the anchor stays correct whether the session completes or is discarded. The captured selection text is stored on the annotation (selectedText, plus selectedTextFromEdits when approximate) and exported in feedback as a Highlighted text block with an honesty note for approximate anchors. New comments are re-projected into editor markers mid-session, and the editor selection collapses after submit so the popover does not reopen over the annotated lines. Entry UX note: the comment entry deliberately lives outside the editor's popover. Focusing an input inside it would blur the editor, collapse the selection, and tear the popover down mid-typing, so the popover only snapshots and hands off to the app-side CommentPopover. Tests: anchor-mapping unit tests (exact mapping through shifts, edited and inserted regions, deletions inside a selection, clamping, CRLF), export coverage for the Highlighted text block, and a DOM-gated popover builder test registered in the CI DOM step. Adapter wall unchanged and passing. * refactor(review): rename Display settings tab to Editor and move edit toggle to top * refactor(review): do not project annotations as editor markers Wavy underlines read as errors, which misrepresents comments. Annotations render in their normal slots below the code instead. --------- Co-authored-by: Michael Ramos <ramos@plannotator.ai> |
||
|
Michael Ramos
|
463f6ed57c |
fix(ui): fall back to legacy copy in insecure browser contexts (#1174)
* fix(ui): fall back to legacy copy in insecure browser contexts navigator.clipboard only exists in secure contexts. Remote mode serves plain HTTP on a non-localhost host, so every bare navigator.clipboard.writeText call threw TypeError and copy buttons silently broke. Add copyTextToClipboard(text): Promise<boolean> to packages/ui/utils/clipboard.ts: it tries the async Clipboard API (guarded against synchronous throws), falls back to the existing copy-event plus execCommand path, reports success as a boolean, and never throws. copyTextWithFallback now returns whether the copy happened and accepts an optional focusOwner; copyTextPreservingFocus keeps its exported signature and behavior unchanged. Route all bare call sites through the helper, preserving each site's UX: Copied states only flip on success, error toasts and console errors remain for the failure case, fire-and-forget sites stay fire-and-forget. GoalSetupSurface gains the fallback and keeps its error surface for the all-strategies-failed case. Add DOM-gated unit tests for the helper and register them in CI. Closes #1173 * fix(ui): address clipboard fallback review findings Review follow-ups for the insecure-context clipboard fallback: 1. Tag the fallback textarea with data-clipboard-fallback and whitelist it in PopoutDialog's ANNOTATION_SELECTORS so the transient focus shift during a fallback copy no longer closes popout dialogs (TablePopout copy buttons, CodeFilePopout copy contents). 2. Widen AnnotationPanel's onQuickCopy prop to Promise<void | boolean>. A false resolution now suppresses the Copied flash; void resolution stays success so existing hosts keep today's behavior. The editor quick-copy site returns the helper's boolean. 3. In copyTextWithFallback, only flag the copy-event path as success when clipboardData was present and setData actually ran, and only when execCommand also reported success. A null clipboardData no longer calls preventDefault, so the textarea retry still runs. 4. Add tests pinning that the fallback runs synchronously when navigator.clipboard is absent (execCommand fires before the call returns, keeping it inside the user-gesture window), that a copy event without clipboardData is not treated as success, and that the fallback textarea carries the PopoutDialog focus-out marker and is removed after the copy resolves. 5. GoalSetupSurface surfaces the real writeText rejection message when the Clipboard API exists but fails and the fallback also fails; the generic unavailable message is reserved for the API-absent case. |
||
|
Michael Ramos
|
93b66e0ab2 |
feat(cli): add safe uninstall lifecycle (#1170)
* feat(cli): add safe uninstall lifecycle * fix(uninstall): harden cleanup and add Windows QA * fix(uninstall): detach Windows self-delete worker * fix(uninstall): preserve PowerShell worker syntax * fix(uninstall): harden purge and host recovery * fix(uninstall): revalidate purge boundary * fix(uninstall): unlink managed link entries safely |
||
|
Michael Ramos
|
d53cbfb373 |
fix(annotate): enforce archive read-only surfaces (#1171)
* fix(annotate): enforce archive read-only surfaces * fix(archive): close remaining read-only leaks |
||
|
Michael Ramos
|
8f84852f97 |
fix(review): surface partial GitLab comment submissions (#1164)
Surface partial GitLab submission outcomes and preserve narrowed, duplicate-safe retries across dialog reopen and same-tab refresh. Follow-up for an explicit blocked-recovery escape: #1166. |
||
|
Oscar Mederos
|
d06f96decb | fix(ui): focus and select existing search text (#1152) | ||
|
Ben Newman
|
6b542da8b9 |
feat(annotate): extend per-file version diff to folder sessions (#1105)
* refactor(annotate): extract per-file version history into a shared helper Move the single-file annotate-history pipeline (slug derivation, saveToHistory, previous-version lookup, degrade-on-error) out of the Bun-specific annotate server and into packages/shared/annotate-history.ts, built on node:fs/node:path/node:crypto only so other runtimes can vendor it unmodified. annotate.ts now calls computeAnnotateHistory() instead of inlining the pipeline; behavior for single-file sessions is unchanged. * feat(annotate): extend per-file version history to folder annotate sessions Eligible folder files served through /api/doc now get snapshotted into the same version history the single-file flow uses, and their doc responses carry the same previousPlan/versionInfo/diffCurrent fields /api/plan already returns for single-file sessions. The pipeline runs lazily on first open and is memoized per resolved absolute path for the life of the server, so reopening a file never re-snapshots it. Eligibility mirrors the single-file source-save gates: a local file under the session's folder root, markdown-branch documents only (.md/.txt, not HTML, not a Turndown-converted doc), under the existing 2MB annotatable-file cap, and gated by the same annotateHistory config toggle. Storage failures degrade to a plain render (never a gate on the request) via the same try/catch computeAnnotateHistory already wraps. /api/plan/version and /api/plan/versions gain an optional path (+ base) query param so folder sessions can ask for a specific file's history; the slug is always derived server-side from the resolved, containment-checked path — never accepted from the client, since it gets joined unsanitized into a filesystem path. Omitting path keeps today's single-session-binding behavior unchanged. * test(annotate): cover folder annotate version history Adds a new describe block exercising the folder-mode history pipeline added in the previous commit: first-open snapshot + same-session memoization, storage-level dedupe, cross-mode slug continuity with the single-file flow, first-ever-open field shape, the config toggle, an ineligible (HTML) file type, degrade-on-unwritable-history-dir, and the path-parameterized version endpoints (including containment rejection and the no-path fallback). * feat(ui): add a docKey seam to usePlanDiff for per-document resets usePlanDiff's diff-base state (diffBasePlan, diffBaseVersion, versions, ...) was seeded once from its constructor args and only ever synced later via a "still falsy" guard - fine for a single root document, but switching to a different document (a different previousPlan/versionInfo) would silently keep the previous document's diff base around instead of adopting the new one's. Add an optional docKey param identifying which document the current previousPlan/versionInfo belong to. When it changes between renders, reset diffBasePlan/diffBaseVersion/versions (and in-flight loading/selecting flags) to the newly-provided values. Omitting docKey (or keeping it stable) preserves exactly today's one-time-hydration behavior, so the root document's call site is unaffected until it opts in. No caller passes docKey yet - this is purely additive. * feat(ui): carry a per-document version-diff baseline through useLinkedDoc /api/doc now returns previousPlan/versionInfo/diffCurrent for eligible folder files (same shape /api/plan already returns for single-file sessions). Extend LinkedDocLoadData with those fields and carry them through the same activate/cache/back lifecycle annotations and markdown already use, so a document's diff baseline: - is captured once when the document is first opened - persists in the per-filepath cache across back()/re-open, instead of being lost or needing a re-fetch - resolves cache-first via the new resolveDiffBaseline helper, gated on whether a baseline was ever captured (versionInfo presence) rather than truthiness of previousPlan - a document at its first-ever version legitimately caches previousPlan: null, which is a resolved fact, not a cache miss The hook exposes the active document's baseline as diffPreviousPlan/ diffVersionInfo, both null when no document is active or the active one has no eligible history (every non-folder linked doc, since /api/doc never populates these fields for those). Not yet consumed by App.tsx - purely additive. * feat(editor): render folder-doc version diffs via the active document Folder annotate's version-diff UI (inline PlanDiffViewer blocks, the +N/-M badge, and the Version Browser) was root-document-coupled: usePlanDiff was fed only the root's previousPlan/versionInfo, and every render site keyed off linkedDocHook.isActive to blank out the badge/version tab whenever any linked or folder document was open. Wire the two new per-document seams together instead: - Feed usePlanDiff the active document's own previousPlan/versionInfo/ filepath (falling back to the root document's when none is active), using the document's filepath as usePlanDiff's new docKey so switching documents resets the diff base instead of inheriting the previous one's. - Add per-document fetchers (fetchVersion/fetchVersions with &path=<filepath>) so selecting a base version or listing versions targets the active document's own history, not the session-bound bare endpoints. - Replace the root-only versionInfo/showVersionsTab reads with the active document's, so the Version Browser now reflects whichever document is on screen (previously it kept showing the root document's versions while a linked doc was open). - Drop the blanket "linkedDocHook.isActive ? null/false : ..." suppression at the Viewer callsite and in DocBadges - planDiffStats/hasPreviousVersion already resolve to the active document's own (possibly absent) diff data, so the badge now shows for folder docs with history and stays hidden for every other document exactly as it did before. Root-document behavior (single-file, plan, review, HTML surfaces) is unaffected: none of those ever set a docKey or have an eligible document history, so they fall through to the same defaults as before. * feat(pi): extend per-file version history to folder annotate sessions Mirrors the Bun runtime's folder annotate history support (packages/server/annotate.ts + reference-handlers.ts) in the Pi Node server: - Vendor the shared annotate-history helper (deriveAnnotateHistorySlug, computeAnnotateHistory) from packages/shared into generated/ via vendor.sh, and delegate the single-file version-history pipeline in serverAnnotate.ts to it instead of the hand-duplicated inline block. Behavior for single-file sessions is unchanged. - Eligible folder files served through /api/doc now get snapshotted into the same version history the single-file flow uses, and their doc responses carry the same previousPlan/versionInfo/diffCurrent fields /api/plan already returns. The pipeline runs lazily on first open and is memoized per resolved absolute path for the life of the server, so reopening a file never re-snapshots it. - /api/plan/version and /api/plan/versions gain an optional path (+ base) query param so folder sessions can ask for a specific file's history; the slug is always derived server-side from the resolved, containment-checked path (resolveAllowedDocPath in reference.ts) — never accepted from the client. * test(pi): cover folder annotate version history Adds apps/pi-extension/server/annotate-history.test.ts, the Node mirror of packages/server/annotate.test.ts's folder-history describe block: first-open snapshot + same-session memoization, cross-mode slug continuity with the single-file flow, the config toggle, an ineligible (HTML) file type, degrade-on-unwritable-history-dir, and the path-parameterized version endpoints (including containment rejection and the no-path fallback). History writes land in the real ~/.plannotator data dir rather than a per-test PLANNOTATOR_DATA_DIR override: generated/storage.js caches its data directory in a module-level constant at first import, so a per-test env var override taken after that point silently no-ops. Each test uses its own unique project namespace instead, same approach as the Bun-side suite. * ci: run docKey/linked-doc DOM tests in CI usePlanDiff.test.tsx and useLinkedDoc.test.tsx use the test.skipIf(!hasDom) pattern but were never added to the DOM_TESTS step, so they silently skipped under CI's plain `bun test` and never actually ran. * refactor(annotate): drop diffCurrent from the folder /api/doc path diffCurrent equals the document's own markdown and the client never reads it off /api/doc — it only exists on /api/plan for legacy single-file shape parity, which is untouched. Stop merging it into folder /api/doc responses and stop retaining it in the per-launch folder history memo (Bun and Pi), and drop the now-unused field from LinkedDocLoadData. - packages/server/reference-handlers.ts: new FolderAnnotateHistory type (AnnotateHistoryResult minus diffCurrent); applyDocOptions no longer copies diffCurrent onto the response - packages/server/annotate.ts: the folder memo now stores/returns only slug/previousPlan/versionInfo - apps/pi-extension/server/reference.ts + serverAnnotate.ts: mirrored changes for the Pi runtime - packages/ui/hooks/useLinkedDoc.ts: removed the unused diffCurrent field from LinkedDocLoadData * test(annotate): stop leaking history dirs; update diffCurrent expectations The folder annotate history tests (Bun and Pi) minted a fresh project namespace per test but never cleaned up, leaving hundreds of directories under the real ~/.plannotator/history over repeated runs. Track every minted project and remove its history directory in afterAll — this also covers the stray non-directory artifact the "unwritable data dir" test deliberately plants inside its own project's history dir, since removing the project dir recursively takes it with it. Also update the two assertions that expected diffCurrent on the folder /api/doc response: that field is no longer propagated on the folder path (see the preceding diffCurrent-removal commit), so both now assert its absence instead. * fix(ui): remember per-document diff-base selection across navigation usePlanDiff reset diffBasePlan/diffBaseVersion to the newly-provided document's defaults on every docKey change. That discarded a manually selected base version when navigating away from a document and back (e.g. root -> linked doc -> root), regressing behavior upstream relied on keeping (nothing reset the selection before this seam existed). Track each docKey's selection in a ref-held Map (keyed by docKey, including null for the root document) and restore it on return instead of re-seeding defaults; a key visited for the first time still seeds from its own initialPreviousPlan/versionInfo exactly as before, and selections never leak between distinct keys. Adds two DOM-gated tests: restoring a manual selection after a detour to another document, and confirming distinct docKeys don't leak into each other. * fix(annotate): match folder history eligibility to the single-file plain-text set The folder /api/doc history gate was a hardcoded /\.(md|txt)$/i in both runtimes, so any other annotatable plain-text file (.mdx, .yaml, .json, .toml, ...) opened via a folder session silently skipped snapshotting — breaking the cross-mode continuity this feature advertises (a .yaml with an existing single-file version thread showed no diff when opened via its folder). Reuse the canonical predicate instead: isAnnotatableTextPath (ANNOTATABLE_TEXT_REGEX in @plannotator/core/annotatable), the exact set the single-file pipeline snapshots. HTML stays deferred and .env stays excluded, both by that same definition. Tests extended in both runtimes: .mdx mints on first open, .yaml single-file history serves as the folder baseline, .env mints nothing, .html unchanged. * feat(ui): label the folder diff badge with its baseline The in-file version-diff badge in annotate/folder sessions shows +N/-M against the file's last-reviewed snapshot, while the git badges in the file tree count uncommitted-vs-HEAD — same numbers, different baselines. Give the badge an optional baseline suffix and tooltip override (PlanDiffBadge baselineLabel/baselineTooltip, threaded through DocBadges, Viewer, and StickyHeaderLane) and have annotate mode pass 'since last review' / 'Changes since you last reviewed this file'. Plan review passes nothing and renders byte-identically to before. DOM tests cover both the labeled and the unchanged default rendering. * fix(editor): exit diff view when the active document loses its baseline Follow-up to the per-document diff baselines: with diff view active on file A, opening a history-less file B left isPlanDiffActive latched on — the diff viewer could not render for B, but the stale flag hid the annotation toolstrip and sticky header until the user pressed Escape. Auto-exit the diff view whenever the active (non-HTML) document has no baseline. The --render-html surface is explicitly gated out: its diff view is driven by htmlDiffHtml with usePlanDiff fed nulls, so hasPreviousVersion is always false there and auto-exiting would kill the HTML diff toggle. Plan review is unaffected — the root document's baseline never goes false mid-session. DOM tests cover the exit, the keep-active document switch, the HTML gate, and the no-baseline activation snap-back. --------- Co-authored-by: Michael Ramos <mdramos8@gmail.com> |
||
|
Michael Ramos
|
467c11b29f | fix: discover symlinked workspace repositories (#1060) | ||
|
renovate[bot]
|
969f5b2141 |
chore(deps): update github actions (#593)
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> |
||
|
Michael Ramos
|
977f4ce582 |
fix: final QA sweep fixes — file-browser cap priority, OpenCode project scoping, spotlight Alt-Alt dismiss
Fixes for the three-agent adversarial sweep findings:
- reference-handlers.ts: seed the user's modified/untracked files BEFORE
the bulk walk — the 5000-file cap (
|
||
|
Michael Ramos
|
8350299bce |
ci: register 5 orphaned DOM-gated test files in the DOM_TESTS batch
QA audit found four DOM-gated test files added since v0.22.0 (srcdoc,
InlineMarkdown.resolveLinkedDoc, MarkdownDiff.frozen,
MarkdownEditor.extensions) that self-skip under plain `bun test` and were
never added to the DOM_TESTS batch — so the mounted-seam contracts for the
published markdown-editor/atomic-editor packages ran nowhere in CI. Also
registers components/sidebar/FileBrowser.test.ts (pre-existing gap; its new
filtering tests from
|
||
|
Michael Ramos
|
e20fd97be7 |
Consumer enablement for @plannotator/ui (0.24.0) (#1017)
* feat(ui): AnnotationPanel renderCardFooter + readOnly host props Per-card footer slot for host reply/resolve UI (clicks inside don't select the card); readOnly hides delete/edit on all card kinds. Both optional, both no-op by default — Plannotator unchanged. * feat(ui): bless 6 exports into the strict-consumer surface TableOfContents, ResizeHandle, useResizablePanel, useActiveSection, useScrollViewport, utils/annotationHelpers — verified strict-clean and backend-free (useResizablePanel persists via the storageBackend seam); added to the gate and the HANDOFF supported-imports table. * feat(ui): Viewer allowImages + readOnly host props allowImages threads to both CommentPopover sites (popover already gated its attach affordance; Viewer never exposed the knob). readOnly suppresses every composer entry point — selection toolbar (highlighter 'enabled'), pinpoint, global comment, attachments, checkbox toggles — while existing annotations still render and select. Both default to today's behavior. * fix(ui): lazy-import Viewer/CommentPopover in the consumer test (DOM-less bun test crashed on web-highlighter) * feat(ui): strict-consumer gate gains verbatimModuleSyntax + noUnusedLocals/Parameters Fixed the 24 violations the flags surfaced across the supported-import graph: type-only imports (verbatimModuleSyntax) and dead imports/locals. Consumers with stricter tsconfigs no longer have to relax them. * feat(ui): opt-in content-verifying annotation restore verifyRestoredContent on useAnnotationHighlighter: after a meta-based fromStore restore, the painted text is checked against originalText (whitespace-normalized). Mismatch -> highlight removed, text-search fallback re-anchors; if that fails too, onRestoreMismatch(annotation, restoredText) fires and nothing is painted. Default off — today's trust-the-positions behavior. Workspaces hit this live after document drift; correctness upgrade for every consumer. * chore(ui): 0.24.0 — HANDOFF consumer-enablement notes + version bump |
||
|
Michael Ramos
|
070d9a5f6d |
Make the document UI reusable as published building blocks (#957)
* docs(adr): revert failed document-ui cutover, add ADR 004 with corrected reuse plan The document-ui extraction/cutover (ADRs 002/003) was an AI-driven rewrite that broke the app; the code was reverted. Add ADR 004 as the source of truth: share @plannotator/ui as published building blocks for the Workspaces app, keep Plannotator's app unchanged, gate on human-verified parity. Banner the reverted ADRs and point AGENTS.md/CLAUDE.md at 004 so future agents don't rebuild the mess. * docs(adr): add verified document-ui extraction plan, supersede draft inventory 36-agent verification of the reuse inventory: confirmed the /api coupling but found the draft missed Viewer's transitive backend call, the cookie settings layer, 3 React contexts + identity singleton, SSE transports, and harder packaging blockers. Adds the verified per-subsystem extraction plan with a parity guardrail on every step; flags the draft inventory as superseded. * docs(adr): add document-ui extraction roadmap + parity checklist Phase 0-7 execution roadmap (safety net -> packaging -> foundation seams -> rendering -> navigation -> comments -> extras -> publish) and the reusable 'did it break?' parity checklist run after every step. Both enforce the law: move + decouple, never rewrite; Plannotator's experience cannot change. * build(ui): packaging unblock for external install (Phase 1) — no runtime change Phase 0: captured parity baseline (typecheck/test/build + shipped-bundle hashes). Phase 1 packaging fixes to packages/ui, metadata only: - add phantom dompurify ^3.3.3 dep (imported in sanitizeHtml/aiChatFormat, was undeclared) - align diff ^8.0.3 -> ^8.0.4 with root - add peerDependencies (react, react-dom, tailwindcss, tailwindcss-animate); keep as devDeps - add files allowlist (excludes tests); remove dead tsconfig @plannotator/shared alias Verified byte-identical: typecheck pass, 1620 tests pass/0 fail, all 3 builds OK, shipped plan+review bundle hashes unchanged from baseline. Remaining Phase 1 blocker (@plannotator/ai + @plannotator/shared workspace:* deps) deferred pending a publish-vs-inline decision; logged in worklog. * feat(ui): make image URL resolution host-overridable (Phase 2, seam 1) getImageSrc now delegates to a module-level resolver defaulting to the verbatim Plannotator /api/image logic; add setImageSrcResolver/resetImageSrcResolver so a host (Workspaces) can resolve images via its own backend. All 5 consumers and the signature unchanged. Verified: default URLs byte-identical, typecheck pass, 1620 tests pass/0 fail, builds OK. No Plannotator behavior change. * feat(ui): make settings storage backend host-overridable (Phase 2, seam 2) storage.ts cookie impl is now the default 'cookieBackend'; add setStorageBackend/ resetStorageBackend so a host (Workspaces) can persist settings via its own storage. getItem/setItem/removeItem delegate to the active backend; the ~24 consumers and literal plannotator-* keys are unchanged. Verified: swap works, typecheck pass, 1620 tests pass/0 fail, builds OK, theme persists across reload. * feat(ui): make MarkdownEditor theme mode host-supplyable (Phase 3) Add optional mode? prop; mode now mode ?? resolvedMode. Plannotator passes no mode (App.tsx:4261) so it keeps using ThemeProvider's resolvedMode unchanged. A host without ThemeProvider can supply mode directly. Verified: typecheck pass, 1620 tests/0 fail, builds OK, App.tsx untouched. * feat(ui): allow hosts to opt out of code-path validation (Phase 3) Viewer gains optional disableCodePathValidation? threaded to a new disabled? arg on useValidatedCodePaths; when set, the /api/doc/exists probe is skipped. Default undefined for Plannotator => validation stays on, /api/doc/exists fires exactly as today. Verified: typecheck pass, 1620 tests/0 fail, builds OK, App.tsx untouched. Also logs Phase 3 workflow outcome + remaining scroll/docfetch pieces. * feat(ui): make code-file hover preview fetch host-overridable (Phase 3) Add DocPreviewFetcher seam (default = verbatim /api/doc fetch) + setDocPreviewFetcher/resetDocPreviewFetcher; route handleMouseEnter through it, useCallback deps unchanged. No caller overrides it => Plannotator fetches /api/doc identically. typecheck pass, 1620 tests/0 fail, builds OK. * feat(ui): ship ScrollViewportProvider with the library (Phase 3 scroll) Add render-transparent ScrollViewportProvider (createElement, keeps .ts) so the scroll-viewport context travels with @plannotator/ui instead of living only in App.tsx. Rewire App.tsx provider tags (3-line delta); identical tree/value/ position, sidebar TOC still reads the MAIN viewport. Fix stale OverlayScrollbars doc-comment. typecheck pass, 1620 tests/0 fail, builds OK, eyeball: TOC tracks. * fix(ui): disabled code-path validation should keep links clickable (self-review) The Phase-3 disabled branch set ready=true with an empty map, which makes gateCodePath demote every code link to plain text. Leave ready=false so the no-validation fallback renders links optimistically. No Plannotator impact (never disables). Logs Phase 3 completion + reusability note. typecheck pass, 1620 tests/0 fail, builds OK. * feat(ui): make file-tree backend host-overridable (Phase 4) Lift useFileBrowser's three backend wires (load-dir fetch, obsidian-vault fetch, and the SSE live-watch effect moved VERBATIM) into an injectable FileTreeBackend with default + setFileTreeBackend/resetFileTreeBackend, same pattern as the image /storage seams. useFileBrowser() stays zero-arg; default fetch/SSE URLs identical. Sidebar confirmed noop (zero backend wires, already reused by review-editor). Verified: useFileBrowser.test.tsx passes 6/0 UNMODIFIED (DOM_TESTS=1), typecheck pass, 1620 tests/0 fail, builds OK, App.tsx untouched, manual eyeball (annotate adr/: tree loads, file-switch works, new file appears live via SSE). Plannotator byte-unchanged. Logs two pre-existing bugs found during testing (not regressions). * docs(adr): research + synthesis + spec for Phase 5 (comments/annotations/drafts) Five-probe code research of the comment system. Key finding: most comment UI is already portable (panel/popover/toolbar/highlighter prop-driven; review-editor already reuses the hooks). Phase 5 narrows to 3 seams — draft transport (+ the 3-party generation protocol), external-annotation transport (SSE->polling, move verbatim), and identity/authorship — plus 2 non-extraction items: renderer coupling (document as a contract) and replies/threading (defer as a new feature). * docs(adr): accept ADR 005 — make comments/annotations/drafts host-overridable (Phase 5) Three seams (identity, draft transport, external-annotation transport), each defaulting to today's behavior; renderer coupling documented as a contract; replies/threading deferred as a new feature. Locks in the recommended choices from the Phase 5 spec/synthesis. * feat(ui): make annotation identity host-overridable (Phase 5 seam 1) Add IdentityProvider + setIdentityProvider/resetIdentityProvider in identity.ts; getIdentity/isCurrentUser now delegate to a module-level provider defaulting to today's ConfigStore tater behavior. The ~9 author-stamp sites and 2 (me)-badge sites delegate with zero call-site edits. No caller overrides => Plannotator byte-unchanged. typecheck pass, 1620 tests/0 fail, builds OK. * feat(ui): make draft persistence transport host-overridable (Phase 5 seam 2) Add DraftTransport (load/save/remove) + getDraftTransport/setDraftTransport/ resetDraftTransport in useAnnotationDraft.ts, default = today's /api/draft fetches verbatim. useCodeAnnotationDraft reads getDraftTransport() live. The generation pre-increment, 500ms debounce, keepalive retry-gate, and pagehide/visibilitychange flush stay in the hooks; getDraftGeneration() still escapes to the host. save rejects-on-failure so the gated retry is preserved. No caller overrides => Plannotator byte-unchanged. shared/draft.test.ts 10/0, annotationDraftPersistence 13/0, typecheck pass, 1620 tests/0 fail, builds OK. * feat(ui): make external-annotation transport host-overridable (Phase 5 seam 3) Add ExternalAnnotationTransport<T> (subscribe/getSnapshot/CRUD) + setters in useExternalAnnotations.ts; default = today's SSE->polling wire moved verbatim into createDefaultTransport. The reducer (applyEvent), fallback-once gate, 500ms poll, versionRef scoping, optimistic-before-await, and [enabled] gate stay in the hook. A host (Workspaces) can implement the same event contract over Durable Objects. No override caller => Plannotator byte-unchanged. external-annotations test green, typecheck pass, 1620 tests/0 fail, builds OK. Logs Phase 5 completion. * docs(adr): research + synthesis + spec for Phase 6 (versions, settings, sharing, AI) Five-probe code research. Most of the four subsystems is already portable; the real work is 5 seams (version fetchers + vscode-diff, config write-back, obsidian detect, save-to-notes, AI transport) + 1 CSS move (block/raw diff classes from the app shell into the package's theme.css). Fragile do-not-touch: the AI SSE reader loop + epoch guards, and configStore debounce/deepMerge. Five Plannotator-only pieces (OpenInApp, HooksTab, useUpdateCheck, useAgents/useAgentJobs) stay home. * docs(adr): accept ADR 006 — make extras (versions/settings/sharing/AI) host-overridable (Phase 6) Five seams + one CSS move, each defaulting to today's behavior. AI reader loop + epoch guards and configStore debounce/deepMerge stay verbatim. Five Plannotator- only pieces stay home. Locks the recommended choices from the Phase 6 spec. * feat(ui): make version fetchers + vscode-diff host-overridable; move diff CSS into package (Phase 6 versions) usePlanDiff gains optional fetchers (default /api/plan/version(s), error asymmetry kept: selectBaseVersion alerts, fetchVersions silent). PlanDiffViewer gains optional onOpenVscodeDiff (default /api/plan/vscode-diff). Relocate .annotation-highlight* + .plan-diff-* block/raw CSS from editor/index.css into ui/theme.css (next to .plan-diff-word-*) so the diff/highlights are self-styling from the package. Verified: relocated CSS gone from index.css, present in shipped bundle (33x), diff renders identical; typecheck pass, 1620 tests/0 fail, builds OK, App.tsx untouched. * feat(ui): make config write-back + obsidian-detect host-overridable (Phase 6 settings) configStore.setServerSync(fn) injects only the terminal POST /api/config; the 300ms debounce, deepMerge batching, singleton, and eager cookie reads stay verbatim. Settings gains optional onDetectObsidianVaults (default /api/obsidian/vaults), with the [obsidian.enabled] effect dep + auto-select-first-vault verbatim. No override caller => Plannotator unchanged. typecheck pass, 1620 tests/0 fail, builds OK. * feat(ui): make save-to-notes host-overridable (Phase 6 sharing) ExportModal gains optional onSaveToNotes (default = verbatim POST /api/save-notes); showNotesTab = isApiMode && !!markdown kept byte-for-byte. Sharing utils already parameterized (noop). No override caller => Plannotator unchanged. typecheck pass, 1620 tests/0 fail, builds OK. * feat(ui): make Ask AI transport host-overridable (Phase 6 ai) useAIChat gains a module-level AITransport (session/query/abort/permission) + setAITransport/resetAITransport, default = the five /api/ai/* fetches verbatim. The SSE reader loop, epoch/createRequest guards, and the supersede-abort position inside createSession stay untouched. Capabilities + provider-resolution stay host-owned in App.tsx. No override caller => Plannotator unchanged. ai.test.ts 97/0, typecheck pass, 1620 tests/0 fail, builds OK. * docs(adr): log Phase 6 completion (4 seams + diff CSS move) * docs(adr): research + synthesis + spec for Phase 7 (carve @plannotator/core + publish) Carve a browser-safe @plannotator/core: move the ~15 pure shared modules in, extract types from the 3-4 node-bound ones (config/storage/workspace-status) so nothing duplicates, shim @plannotator/shared so Plannotator's 99 import sites stay unchanged, re-point @plannotator/ui to depend only on core, move wideMode.ts, then publish core+ui (source-only). shared + ai stay private. Open: registry, versions, CI job. Publish is the one outward-facing step — confirm before pushing. * docs(adr): fold configurePlannotatorUI() front door + precompiled CSS into Phase 7 spec Add the single typed configure() facade over the 9 global host-override setters (zero-risk, additive) and an optional precompiled CSS bundle (smooths the Tailwind-in-shared-lib wrinkle) to the Phase 7 publish scope. Both make the published surface nicer to consume; neither touches Plannotator. * docs(adr): lock Phase 7 publish decisions + carry over review fixes Decided: ship JS as source (single internal consumer on controlled stack, no build to maintain, no dist drift); precompiled CSS now REQUIRED (the @source glob is fragile under pnpm symlinks); core CI typecheck node-free; pin ui->core exact. Recorded the interrogation's carried-over Phase-5 code fixes (useExternalAnnotations split-transport + fallbackRef reset, per-seam override tests, configStore loadFromBackend) to do before publish. * docs(adr): ADR 007 — carve @plannotator/core, complete settings provider, publish Locks Phase 7 decisions: public npm; lockstep version at repo 0.21.0 (ui->core pinned exact); JS ships as source + required precompiled CSS; core CI node-free; ai stays unpublished-to-npm. Settings provider completed (loadFromBackend, prefetch +sync) is now IN SCOPE — Workspaces uses the same UI settings stored in its own backend. CI publish job wired but artifacts validated on-branch (pack + dry-run) before merge; first publish gated. Carries the 2 override-path bug fixes + per-seam override tests as pre-publish work. * fix(ui): make external-annotation transport reads consistent + reset fallback on re-enable Two override-path bugs found by the interrogation pass (both unreachable on Plannotator's path; harden the host-override path for a real consumer): 1. Split-transport: the effect captured the transport at mount for subscribe/poll while the CRUD callbacks read the module global live, so a host swapping the transport after mount would split reads and writes across two backends. Capture once in a ref and use it in all four spots. 2. fallbackRef/receivedSnapshotRef were not reset on effect re-run, so an enabled false->true toggle inherited a stale 'already fell back' flag and silently stopped updating. Reset both at the top of the effect. Plannotator unchanged: it never overrides the transport (same default singleton captured) and enabled never toggles (reset is a no-op). typecheck clean; full test suite shows zero delta (1605 pass / 45 pre-existing env failures, identical with and without this change). * docs(adr): align Phase 7 spec with ADR 007 (version 0.21.0 lockstep, CSS required, scope completeness) * feat(core): carve @plannotator/core — move pure modules, extract node-bound types, shim shared (Phase 7 step 1) * feat(ui): depend only on @plannotator/core — re-point all shared/ai imports (Phase 7 step 2) * refactor(ui): relocate wideMode helper to @plannotator/ui/utils (Phase 7 step 3) * feat(ui): add loadFromBackend settings rehydration + configurePlannotatorUI front door (Phase 7 step 4) * build(ui): precompiled styles.css CSS build + madge circular-dep check (Phase 7 step 5) * test(ui): per-seam override tests + configure routing test (Phase 7 step 6) Add one override test per seam (setX(fake)→drive→assert→resetX()) for all 9 seams + loadFromBackend, modeled after the existing seam test pattern. Fix configure.test.ts to defer mock.module() into beforeAll and restore with captured real function references in afterAll so sibling seam test files are not poisoned by spy replacements in the shared Bun worker module registry. * fix(ui): apply Phase 7 review findings — version lockstep + seam consistency - Bump @plannotator/ui to 0.21.0 (lockstep with @plannotator/core + repo, per ADR 007) [was the 1 critical review finding] - useAnnotationDraft: route persistNow/dismissDraft save+remove through getDraftTransport() so all paths read the transport consistently (matches the load path; makes the single-global invariant explicit) - configStore.loadFromBackend: document it must be called BEFORE init() or server values get overwritten - packages/core/tsconfig: add explicit types:[] so the node-free invariant is first-class (verified: planted node:fs still fails TS2882) * docs(adr): Phase 7 implementation plan (workflow-generated, durable artifact) * fix(ui): reconcile #948 with the draft-transport seam + lockstep 0.21.1 Rebased onto origin/main (picks up #948 draft-deletion fix, the 0.21.1 bump, and the #949/#950 editor fix). The rebase auto-merged #948's code-draft logic (hasHadAnnotationsRef, empty-state tombstone, clearTimeout in restore/dismiss) with the Phase-5 transport refactor cleanly — except the empty-state tombstone delete was left as a raw fetch('/api/draft', DELETE). Route it through getDraftTransport().remove() so a host backend tombstones its own stored draft on clear (the #948 guarantee, for hosts). Plannotator unchanged (default transport hits the same endpoint). Bump @plannotator/core + @plannotator/ui 0.21.0 -> 0.21.1 to match main's version (lockstep per ADR 007). Verified: typecheck clean, madge no-cycles, plain suite 1637 pass / 0 fail, #948 draft-clear test 3/0. (The 45 DOM_TESTS failures are the known server/network integration tests that need a real OS env — same set on main, not regressions.) * fix(ui): address review nits — host-path robustness + cleanups - PlanDiffViewer: wrap onOpenVscodeDiff in try/finally so a host opener that throws can't wedge the VS Code button in a permanent loading state (default unaffected) - useExternalAnnotations: (re-)capture the transport inside the effect on enable so a host that installs a transport before enabling annotations is honored, not the stale default — keeps the split-transport fix (effect + CRUD share one ref) - configure.ts: import ServerSyncFn from configStore instead of duplicating the type - repoint the 2 remaining @plannotator/shared test imports to @plannotator/core - AGENTS.md/CLAUDE.md: document the new packages/core package All host-path only — Plannotator behavior unchanged. typecheck clean, no cycles, full suite green. Skipped (not simple/over-engineering): usePlanDiff prop->module-level (design change), Obsidian late-bind, getSnapshot guard (inert), transport <any> (variance). * docs: collapse 29 ADR process docs into one packages/ui/README.md The branch had accumulated ~6,200 lines of ADR scaffolding (6 decisions, 7 specs, 10 research spikes/synthesis, 6 worklogs/roadmaps/plans) for this one effort. Replace all of it with a single concise README that ships with the published package: what @plannotator/ui + @plannotator/core are, why they exist (commercial reuse), how the host-override seams work (configurePlannotatorUI), how a consumer installs/builds, and the one rule (don't reimplement from scratch — add a seam). Repoint the CLAUDE.md banner at the README. No code references the deleted docs; main's pre-existing adr/ docs untouched. * docs(ui): add packages/ui/AGENTS.md guardrail + CLAUDE.md symlink Directory-scoped agent guidance for anyone editing @plannotator/ui: don't rewrite from scratch, add a seam (default = today's behavior, Plannotator byte-for-byte unchanged), core stays node-free, never delete working code until human parity. Points to README.md for the architecture. CLAUDE.md -> AGENTS.md symlink mirrors the repo root convention. * build: remove madge circular-dep check (unmaintained) madge is unmaintained (~3 years stale) and the check was never wired into CI, so it was a dormant script + devDependency on a load-bearing path. Drop it: remove the check:cycles script, the madge devDependency, and .madgerc. The no-cycle invariant still holds by construction — @plannotator/core imports nothing (zero @plannotator deps in its package.json), so any accidental core->shared/ui import fails at publish-time bun pm pack (and review). No automated tripwire, but no stale unmaintained tooling either. * fix(ui): address review — TDZ guard, html-viewer export, doc corrections - useExternalAnnotations: declare unsubscribe as let (not const) + guard calls, so a host transport that fires onError synchronously during subscribe falls back to polling instead of throwing a TDZ ReferenceError (Plannotator's EventSource fires async, never hit) - package.json: add explicit ./components/html-viewer export (dir has index.ts; the ./components/* -> *.tsx wildcard can't resolve it, so external installers would fail) - README: fix configurePlannotatorUI sample keys to the real option names (storageBackend/identityProvider/imageSrcResolver/externalAnnotationTransport) - AGENTS.md: point the Ask-AI mapping at packages/core/agents.ts (shared/agents.ts is a shim now) All publish/host-path/doc only — Plannotator unchanged. (#1 CSS-build font collision deferred to publish-prep — it needs the asset pipeline + files allowlist, not a one-liner.) * build(ui): don't bundle fonts in published styles.css — app loads fonts (review #1) Industry standard for a shared UI package: ship theme + component CSS, let the consuming app load fonts. Drop the @fontsource imports from styles-entry.css (the publish CSS entry); the theme still defines --font-sans/--font-mono, and the app provides those families. Fixes the asset-name collision (every emitted .woff2 was renamed styles.css) and shrinks the published stylesheet 555kB -> 185kB. README documents the two-line @fontsource install. Plannotator unaffected: its apps (editor/review-editor index.css) load fonts via their own entry CSS — styles-entry.css is consumed ONLY by the publish CSS build. * fix(ui): build styles.css on prepack, not prepublishOnly (review #4) prepublishOnly doesn't run for npm pack / bun pm pack / git / file: installs, so the package exported ./styles.css without shipping it. prepack runs on any pack, so the stylesheet is always present. Verified: bun pm pack now emits styles.css. * chore(ui): post-rebase reconciliation — version lockstep 0.21.3, awaitable AI abort seam Rebased onto main (0.21.3). Bump @plannotator/core + @plannotator/ui to 0.21.3 to stay in lockstep with the repo version. Resolve the useAIChat conflict: main added postServerAbort (an awaitable abort that prevents session-busy races) using a raw fetch. Route it through the AITransport seam by making AITransport.abort return Promise<unknown> instead of void, so the host override is honored AND main's await-the-abort behavior is preserved. Update the abort mocks in the seam/configure tests accordingly. * fix(ui): make postServerAbort never reject regardless of AI transport The await site in ask() relies on postServerAbort resolving so a superseding query can proceed. main's original guaranteed this with its own .catch on the fetch; routing through the AITransport seam delegated that guarantee to the transport. Restore it at the call site (Promise.resolve(...).catch) so a host override that rejects — or returns void at runtime — can't throw out of ask(). * fix(ui): address review — core import, abort sync-throw, snapshot guards - useAIProviderConfig: import Origin from @plannotator/core/agents (was the only ui file still importing @plannotator/shared); drop the masking shared/* path alias from ui/tsconfig.json so a stray shared import now fails typecheck. The hook is part of the published surface — a standalone install had no @plannotator/shared to resolve. - useAIChat.postServerAbort: defer the transport call into .then so a host abort that throws *synchronously* also can't reject (the .catch only caught async). - useExternalAnnotations: default getSnapshot returns null (skip) on a malformed 200 instead of coercing to []/0, so it can't clear annotations or reset the version cursor — restoring the pre-seam behavior. * feat(ui): add upload + identity-editable seams for host backends Two override points the Workspaces app needs that had no seam: - UploadTransport (utils/upload.ts): image attachments hardcoded POST /api/upload with no override. Add a setX/resetX/getX seam (default = today's /api/upload, verbatim) and route AttachmentsButton through it. Workspaces sends bytes to its R2 asset API and returns the content-addressed URL. - IdentityProvider.isEditable() (utils/identity.ts): the Settings rename/regenerate controls wrote to the cookie store, bypassing a host identity provider — so a host with server-owned identity could split one user across two author names. Add an optional isEditable() (default true) and hide the rename controls when a host returns false. Plannotator's cookie identity stays editable — unchanged. Both wired into configurePlannotatorUI(); seam tests added; configure routing test covers uploadTransport. HANDOFF.md updated with the Workspaces seam mapping from the repo research (asset layer, identity, realtime, no-AI-infra, the Me display-name backend follow-up). README publish command corrected to bun pm pack + npm publish. * refactor(ui): capture sessionId synchronously in postServerAbort Self-review: the deferred .then read sessionIdRef.current a microtask after the guard checked it. Capture the id synchronously so the abort always targets the session current at call time and there's no double-read. * fix(ui): address review — seed host store, browser-safe timer type, harden abort - configStore.loadFromBackend: seed the host StorageBackend with resolved defaults for keys it lacks. The constructor runs at module load (before a host installs its backend), so its default-seeding writes went to the cookie backend; without this a fresh host store was never populated and generated defaults (e.g. displayName) regenerated every reload. [P1, host path] - Viewer.tsx: replace NodeJS.Timeout with ReturnType<typeof setTimeout> (2 refs) so a browser-only consumer compiling the published source doesn't need @types/node. Matches the pattern already used in configStore. [P1, published path] - useAIChat: harden the create-session supersede abort the same way as postServerAbort, so a host transport that throws can't surface an unhandled rejection. No impact on Plannotator (default self-catches). [nit] - .gitignore: correct stale 'prepublishOnly' comment to 'prepack'. [nit] Plannotator behavior unchanged (it never calls loadFromBackend; the timer/abort changes are behavior-preserving). Strengthened configStore seam test to assert first-run seeding. typecheck clean, 1773 pass / 0 fail. * refactor(ui): single-source the never-reject abort via safeAbort helper Self-review: the hardened abort pattern (defer into .then + .catch so a host transport that throws can't reject) was duplicated across postServerAbort and the create-session supersede site — the exact drift the review flagged. Extract a module-level safeAbort(sessionId) so both call sites share one hardened implementation and can't diverge again. Behavior unchanged; reads aiTransport at call time so a late override is honored. * chore(ui): post-rebase version lockstep to 0.21.4 Rebased onto main (0.21.4, adds markdown math #878 + parser hardening). Bump @plannotator/core + @plannotator/ui to 0.21.4 to stay in lockstep with the repo. katex (main's math dep) merged into ui; typecheck clean, 1810 pass / 0 fail. * docs(ui): consumer-lens handoff hardening + ADR 005 - HANDOFF.md: add supported-imports allowlist vs unsupported (hardcoded /api/*) list; document the annotation anchor schema, reattachment order, and untested stale-anchor degradation; state that the markdown editor cannot take CM6/Yjs extensions yet and the plan of record; note AI avoidability re-verified post-rebase; fix stale 0.21.3 ref. - adr/decisions/005: record the publish-as-packages decision (packages over copy/vendor, core/ui split, seam-singleton pattern + SSR revisit condition, the law, lockstep publish model). * fix(ui): make shipped source strict-TS clean for consumers + seam type barrel Consumers compile the published TS source with their own compiler options, and strict mode failed with 35 errors inside the package: - settings.ts: satisfies SettingDef<unknown> is contravariantly illegal under strictFunctionTypes (33 errors) — use SettingDef<any> - useDismissOnOutsideAndEscape: RefObject<HTMLElement> rejects React 19's useRef<T>(null) refs — widen to HTMLElement | null - globals.d.ts: declare *.png / *.webp modules, referenced from each asset-importing component so any consumer program that includes one gets the ambient declarations Also unscatter the seam contract types: configure.ts re-exports every seam type next to configurePlannotatorUI, and ServerSyncFn is now exported from config/index.ts (it was unreachable through the exports map). Verified: standalone Vite consumer importing the full supported surface passes tsc --noEmit under full strict (was 35 errors). * fix(ui): keep KaTeX fonts out of published styles.css (back to ~187KB, was 1.6MB) Main's math PR imports katex/dist/katex.min.css in theme.css; the publish build (Vite lib mode) force-inlines all 60 KaTeX math fonts as data URIs, ballooning styles.css to 1.6MB (977KB gzip) and breaking the package's consumer-owns-fonts policy. Alias the katex stylesheet to an empty stub in vite.css.config.ts only — theme.css stays untouched (no rebase surface) and Plannotator's own apps, which import theme.css directly, still bundle KaTeX as before. Hosts that render math load katex.min.css themselves (bundler import, CDN tag, or self-hosted copy per HANDOFF.md), which also gets them lazy font loading. Verified: fresh build is 186.9KB / 30.8KB gzip with zero @font-face data URIs; consumer vite build CSS drops 1.66MB -> 200KB. * docs(ui): HANDOFF corrections from adversarial consumer review - Math rendering section: KaTeX css/fonts excluded from styles.css by design; three one-time host setup options (self-hosted recommended, CDN tag, bundler import) - styles.css size claim corrected (~187KB / ~31KB gzip) + strict-TS guarantee documented (verified against a standalone consumer) - AI-avoidability claim made precise: configure.ts statically imports useAIChat for its setter; unused AI code tree-shakes to zero (bundle- verified) — the runtime claim holds, the static wording was wrong - Loud warning on the loadSettingsFromBackend ordering footgun: configuring before hydration seeds generated defaults into the host backend and nothing re-runs hydration - DraftTransport.load() tombstone-generation contract spelled out - Seam-type barrel documented on the configure row; 'everything is importable' softened (some components/*.ts don't resolve via the *.tsx wildcard); stale diff stats refreshed * docs(ui): math setup pointer in README + pnpm caveat on the katex bundler-import option * fix(ui): lazy settings resolution — zero cookies on a configured host The configStore resolved all settings eagerly in its constructor, at module import — before a host's configurePlannotatorUI() could install its StorageBackend — writing 17 plannotator-* cookies (including a generated identity) onto the host origin. Resolution now runs lazily on first settings access (get/set/init/loadFromBackend): by then the host backend is live, so the initial reads AND default-seeding writes route through it. A configured host gets zero cookies, ever. Plannotator unchanged: same resolution, same cookie seeding, same values — on first settings read (same page load) instead of at import. New configStore.lazyInit.seam.test.ts proves the contract from a fresh module graph; full suite + consumer strict tsc green. * chore(ui): post-rebase version lockstep to 0.22.0 * fix(ui): round-2 review batch — dedupe asset declarations, CI seam tests, strict consumer gate, doc corrections - components/types.d.ts: drop the *.png/*.webp declarations that globals.d.ts now owns — both shipping was a duplicate-identifier error for any consumer with skipLibCheck: false - untrack packages/ui/styles.css (generated by prepack, gitignored; got scooped into the carve commit during the rebase by git add -A before the ignore entry existed in the replay) - CI: the DOM test step now runs ALL packages/ui tests, so the seam contract tests (AI/draft/external-annotations/file-tree/inline- markdown) actually execute in CI instead of skipping - new packages/ui/tsconfig.strict-consumer.json wired into root typecheck: type-checks the supported-import surface under full strict, so the consumer strict-TS guarantee can't silently rot - HANDOFF: rot-proofed the diff stat, strict guarantee now cites the CI gate, CDN katex pinned-version wording, theme-vs-styles.css caveats (theme still imports KaTeX + needs Tailwind), Viewer required props, Yjs plan-of-record updated to the atomic-editor fork - README: @source fallback wording (build entry isn't shipped) * test(ui): make the lazy-resolution seam test deterministic The test asserted lazy resolution on the module singleton and relied on its test file getting a fresh module graph — an isolation assumption that doesn't hold under all bun test orderings (CI failed with zero observed reads because another file had already resolved the store). Test the contract on a fresh instance instead: ConfigStore is exported as @internal ConfigStoreForTest, the spy backend is installed before construction, and the test asserts construction reads nothing while the first get() resolves and seeds through the live backend. Deterministic by construction. * test(ui): poll for the debounced reconnect refetch instead of a fixed sleep The reconnect-refresh assertion waited a fixed 150ms against the SSE watcher's 120ms debounce — a 30ms margin that slower CI runners lose, flaking 'refreshes after an SSE ready event from reconnect'. The watched logic is unchanged (verified byte-identical to main's inline version — the seam only relocated it into the default watchTrees and added the onChange indirection). Poll for calls.length===2 up to 1s so the pass/fail is hardware-independent. * test(ui): poll the committed tree state, not the fetch call count Prior fix polled calls.length===2, but the fetch call is counted one tick before its result commits to React state — so the poll exited early and the next assertion (dirs[0].tree === reconnectedTree) lost the race on slow CI (toEqual failure). Poll on the committed tree itself, which is exactly what the assertion checks: now the only way to fail is a genuine no-refresh, not a timing margin. * test(ui): give the reconnect-refetch poll a 10s ceiling + 20s test timeout A CI runner was measured at 6x normal speed (1676ms for a ~275ms test), blowing through the 1.5s poll ceiling before the 120ms debounce fired — same commit passed on a faster runner. Raise the poll to ~10s and set an explicit 20s test timeout (bun's 5s default would otherwise kill the poll). Root cause is load, not logic: this timing-sensitive test only started flaking when the CI DOM step was broadened to run the whole ui suite in one process. * ci: run the file-browser DOM test isolated; scope the DOM step to DOM files Root-causes the intermittent 'refreshes after an SSE ready event from reconnect' failure. The round-2 change ran the ENTIRE ui suite under DOM_TESTS=1 to catch the seam contracts; that load intermittently starved the test's 120ms real-timer debounce so the reconnect refetch never fired (observed failing after a full 10s poll — not a margin issue). The hook logic is byte-identical to main, and main runs this test in its own process (green for months). Fix at the CI layer, not the test: run useFileBrowser.test.tsx isolated (matching main), and run the seam contracts + remaining DOM-gated tests as an explicitly-scoped light batch. The test file is reverted to main verbatim (today's timing-poll experiments dropped). Follow-up issue to file: the underlying re-subscription race the load exposed. |
||
|
renovate[bot]
|
903290e451 |
chore(deps): update github actions (#791)
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> |
||
|
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 |
||
|
Michael Ramos
|
f24bb435d3 | Fix live file tree startup latency (#935) | ||
|
Juan Patten
|
1287bfa5d8 |
Disable bunfig autoload for release binaries (#937)
Add Bun's compile autoload guard to every release binary build so distributed executables do not inherit bunfig.toml from the caller's current directory. Extend the binary smoke test to launch --help from a temporary directory with an invalid preload entry, matching the crash reproduction. Update the Codex sandbox manual compile path so locally rebuilt binaries use the same guard. Co-authored-by: Codex <codex@openai.com> |
||
|
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. |
||
|
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
|
||
|
Michael Ramos
|
8c947c5419 |
Add Amp plugin integration (#803)
* Add Amp plugin integration * Use official Amp logo on landing page * Tighten landing agent selector * Default landing selector to Claude Code * Stabilize server ready handoff test * Create ready handoff directory before writing * Stabilize server ready handoff tests * Fix Amp command cancellation and cwd * Preserve Plannotator browser handling for Amp * Fix Amp review edge cases * Add PowerShell installer smoke coverage |
||
|
Michael Ramos
|
7db5e9b8d9 |
Fix Windows Pi shim spawning (#792)
* Fix Windows Pi shim spawning * Fix Pi smoke process cleanup * Kill Windows Pi process trees |