skills/backnotprop__plannotator
1
0
Fork 0
mirror of https://github.com/backnotprop/plannotator.git synced 2026-09-14 14:17:26 +08:00
Code Issues Packages Projects Releases Wiki Activity
Files
feat/annotate-submit-with-note
backnotprop__plannotator/.github/workflows/test.yml
T
Michael Ramos 1cab9dd9a8 feat(review): mark files viewed as you scroll past them (#1430) 
* feat(review): mark files viewed as you scroll past them

Reviewers reading the all-files diff top to bottom had to check every file
off by hand. Now a file marks itself viewed when the reviewer MOVES ON from
it, after its content was actually on screen long enough to have been read.
Arriving at a file never marks it; leaving it downward does.

- All-files surface: a file marks when the reader scrolls past it (its
  successor has reached the viewport top, so it genuinely scrolled out above)
  and has accumulated at least 1000ms as the reported reading file. Dwell is
  cumulative per diff snapshot, so bouncing between two files still accrues,
  while a momentum flick to the bottom marks nothing. The last file, which can
  never scroll out above, marks on reaching the end of the diff.
- Single-file panel: opening a file never marks it; navigating away after the
  same dwell floor does. Keyboard file navigation drives the same panel
  switches, so keyboard-only parity is automatic.
- Collapsed cards never mark. Generated files seed collapsed, so nobody
  reviews a lockfile by scrolling past its folded header.
- Un-viewing a file suppresses auto-view for it until it is marked viewed by
  hand again. That set rides the review draft as an additive optional field.
- Inert inside the Guided Review takeover and on a commit detour, where the
  files on screen are not the change under review.
- A viewed file whose patch changes under a refresh loses its checkmark, but
  only while auto-view is on, so the off state stays byte-identical to today.
- PR sessions batch the marks into one /api/pr-viewed request rather than one
  per file.

The setting is reviewAutoViewed, cookie-only and on by default, with two off
switches: Settings > Git and a row in the file-list gear popover. The first
time auto-view actually fires, a toast says so and offers Turn off; using
either switch consumes that one-time notice.

The decision core is pure and clock-injected (utils/autoViewed.ts), the
binding is a hook (hooks/useAutoViewed.ts), and AllFilesCodeView only gains
one optional emission callback on the rAF path it already runs. No server
changes in either runtime.

AI-assisted (Claude) under maintainer direction.

* fix(review): scope auto-mark-viewed to the transitions it was meant for

Four review findings on the auto-mark-viewed branch.

Rule 5 fired on EVERY applied diff switch, not just the staleness refresh.
The review app funnels every transition through one apply path, so entering
the Commits detour (the rail auto-opens HEAD), switching base branch, and
toggling hide-whitespace all un-viewed files whose per-path patch text
legitimately differs, which contradicts both Rule 4's "a commit detour is
inert" and Rule 5's own rationale. The apply path now goes through
resolveDiffSwitchUnviews, which requires the caller to opt in
(`contentRefresh`) and re-checks the identity of the diff on top of that:
same selection, same base, and never a commit-family type on either side.
Only the staleness refresh and the post-fetch base refresh opt in. The pure
delta resolver is unchanged. A source-level test pins which call sites may
opt in, since that is where the guarantee actually lives.

The at-bottom branch fired on the mount tick. A diff shorter than the
viewport is at-bottom from the very first report, and that report is the
mount seed, so the file on screen marked itself about a second later with
zero interaction and fired the first-time toast at a motionless page. It now
requires a real scroll event on the current file set.

Staging a file marked it viewed without clearing auto-view suppression,
unlike v, the header button and the tree row, so a file the reviewer
un-viewed and later staged stayed permanently off-limits to auto-view.

Dwell accrued while the setting was off, so enabling mid-read could mark the
current file instantly on time the reviewer spent with the feature
deliberately disabled. Disabled is now fully inert: the clock does not
accrue, and enabling starts a fresh one rather than replaying the gap.

AI-assisted (Claude) under maintainer direction.

* chore: refresh pinned guide viewer manifest after merging main
2026-08-31 09:13:49 -07:00

672 lines
33 KiB
YAML
Raw Permalink Blame History

name: Test
on:
workflow_dispatch:
pull_request:
branches:
- main
push:
branches:
- main
permissions:
contents: read
jobs:
test:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
with:
persist-credentials: false
- uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0
with:
bun-version: 1.3.14
- name: Install dependencies
run: bun install --frozen-lockfile
- name: Check release version consistency
run: bun run check:release-version
- name: Generate Pi extension shared copies
run: bash apps/pi-extension/vendor.sh
- name: Type check
run: bun run typecheck
# The extension has its own tsconfig (Node libs, no DOM) and is not part
# of the root typecheck, so nothing was checking it. A type error in the
# panel cookie seed shipped that way; this is the step that would have
# caught it.
- name: Type check VS Code extension
run: bun run --cwd apps/vscode-extension lint
- name: Run tests
run: bun test
# The file-browser watcher test is timing-sensitive (a 120ms SSE debounce
# asserted with real timers). Run it in its own process — matching the
# historical config — so it isn't destabilized by sharing a process with
# the rest of the ui suite (heavy load intermittently starves the debounce
# timer and the reconnect refetch is missed). See useFileBrowser.test.tsx.
- name: Run file-browser DOM test (isolated)
run: DOM_TESTS=1 bun test packages/ui/hooks/useFileBrowser.test.tsx
# These two render against the REAL @pierre/diffs, because the defects
# they cover (a stale render cache served across a cacheKey collision;
# an empty stub card) only exist inside its renderer. A sibling in the
# list below, AllFilesCodeView.lifecycle.test.tsx, calls
# `mock.module('@pierre/diffs', ...)` with a hunk-less getSingularPatch
# and `processFile: () => null` — and bun's module mocks are process
# global with no restore, so in one shared process the real renderer is
# not guaranteed to still be there by the time these load. That leak
# reproduced on the Linux runner and not on macOS. Their own process
# removes the coupling outright, same reasoning as the isolated step
# above.
- name: Run diff-renderer DOM tests (isolated, real @pierre/diffs)
run: >-
DOM_TESTS=1 bun test
packages/review-editor/components/DiffViewer.fullContentSwap.test.tsx
packages/review-editor/components/DiffViewer.oversizedStub.test.tsx
packages/review-editor/components/DiffViewer.binaryNotice.test.tsx
packages/review-editor/components/DiffViewer.headerControls.test.tsx
# Seam contracts + the remaining DOM-gated tests. Scoped to the DOM files
# (not the whole ui suite) to keep this process light. Files get isolated
# globals because theme, storage, and DOM test state must not leak between
# suites when Bun schedules them differently across platforms.
- name: Run UI seam-contract + DOM tests
run: >-
DOM_TESTS=1 bun test --isolate
packages/ui/markdownEditorFidelity.test.tsx
packages/ui/annotationDraftPersistence.test.tsx
packages/ui/codeAnnotationDraftPersistence.test.tsx
packages/ui/components/html-viewer/srcdoc.test.ts
packages/ui/components/html-viewer/htmlPinpointProtocol.test.tsx
packages/ui/components/html-viewer/htmlLiveProtocol.test.tsx
packages/ui/components/html-viewer/HtmlViewer.vimHud.test.tsx
packages/ui/components/html-viewer/HtmlViewer.bridgeAsset.test.tsx
packages/ui/components/Viewer.vimMode.integration.test.tsx
packages/ui/hooks/useVimSelection.test.tsx
packages/ui/utils/codeHighlight.test.ts
packages/ui/utils/htmlChrome.test.ts
packages/ui/utils/inputMethod.test.ts
packages/ui/utils/vimNavigation.test.ts
packages/ui/utils/clipboard.test.ts
apps/vscode-extension/src/vscode-theme.test.ts
packages/ui/utils/vimScroll.test.ts
packages/ui/components/InlineMarkdown.resolveLinkedDoc.test.tsx
packages/ui/components/MarkdownDiff.frozen.test.tsx
packages/ui/components/MarkdownEditor.extensions.test.tsx
packages/ui/components/ThemeProvider.favicon.test.tsx
packages/ui/components/CommentPopover.skillReferences.test.tsx
packages/ui/components/SkillReferenceMenu.placement.test.tsx
packages/ui/components/sidebar/FileBrowser.test.ts
packages/editor/editableDocumentsHook.test.tsx
packages/review-editor/components/ReviewSubmissionDialog.ui.test.tsx
packages/review-editor/components/AnnotationToolbar.placement.test.tsx
packages/review-editor/dock/panels/ReviewPROverviewPanel.ui.test.tsx
packages/review-editor/components/FileHeader.edit.test.tsx
packages/review-editor/edit/useEditSession.recovery.test.tsx
packages/review-editor/edit/discardRestoreRender.test.tsx
packages/review-editor/edit/selectionActionPopover.test.ts
packages/review-editor/components/AllFilesCodeView.lifecycle.test.tsx
packages/review-editor/components/AllFilesCodeView.generated.test.tsx
packages/review-editor/components/AllFilesCodeView.autoViewed.test.tsx
packages/review-editor/components/DiffViewer.compactTouchSelection.test.tsx
packages/guide-viewer/GuideSectionCard.test.tsx
packages/guide-viewer/GuideView.test.tsx
packages/guide-viewer/GuideViewportManager.test.tsx
packages/review-editor/components/guide/GuideExportButton.test.tsx
packages/review-editor/hooks/useReviewSearch.test.tsx
packages/review-editor/hooks/useAutoViewed.test.tsx
packages/review-editor/hooks/useCallFlowAnalysis.test.tsx
packages/review-editor/hooks/useCallFlowInstall.test.tsx
packages/review-editor/hooks/useCallFlowAutoInstall.test.tsx
packages/review-editor/dock/panels/ReviewCallFlowPanel.install.test.tsx
packages/review-editor/dock/panels/ReviewCallFlowPanel.raw.test.tsx
packages/review-editor/components/CallFlowFileBadge.interaction.test.tsx
packages/review-editor/components/CallFlowTreeView.interaction.test.tsx
packages/review-editor/components/PanelChrome.test.tsx
packages/review-editor/components/FileTree.search-placement.test.tsx
packages/review-editor/components/FileTreeNode.controls.test.tsx
packages/ui/components/Settings.analysis.test.tsx
packages/ui/components/AnnotationPanel.props.test.tsx
packages/ui/components/Viewer.consumer.test.tsx
packages/ui/components/Viewer.codeBlockHighlightSwap.test.tsx
packages/ui/components/ImageAnnotator/ImageAnnotator.shortcuts.test.tsx
packages/ui/utils/undoHistory.dom.test.ts
packages/ui/utils/codeBlockMark.test.ts
packages/ui/components/InlineMarkdown.seam.test.tsx
packages/ui/components/ImageThumbnail.seam.test.tsx
packages/ui/hooks/useAnnotationHighlighter.test.tsx
packages/ui/hooks/annotationRestoreVerify.test.tsx
packages/ui/hooks/useAnnotationDraft.seam.test.tsx
packages/ui/hooks/useExternalAnnotations.seam.test.tsx
packages/ui/hooks/useAIChat.seam.test.tsx
packages/ui/hooks/useFileBrowser.seam.test.tsx
packages/ui/hooks/usePlanDiff.test.tsx
packages/ui/hooks/useLinkedDoc.test.tsx
packages/ui/components/DocBadges.test.tsx
packages/editor/planDiffAutoExit.test.tsx
packages/editor/App.archiveReadOnly.test.tsx
packages/editor/App.htmlChrome.test.tsx
packages/ui/components/HtmlSurfaceControls.test.tsx
packages/ui/components/AnnotationPanel.unanchored.test.tsx
packages/ui/hooks/useHtmlRefresh.test.tsx
packages/ui/hooks/useSharing.contentRevision.test.tsx
packages/ui/hooks/useSharing.shortUrlLifecycle.test.tsx
packages/ui/components/AnnotationToolbar.commentOnly.test.tsx
packages/ui/components/CommentPopover.quickLookGood.test.tsx
packages/editor/actionsLabelMode.test.ts
packages/ui/webmcp/useToolset.seam.test.tsx
packages/ui/components/AnnotationPanel.inReplyTo.test.tsx
packages/editor/webmcp/useDocumentWebMcp.toggle.test.tsx
packages/editor/components/AppHeader.webmcpIndicator.test.tsx
packages/ui/components/MathBlock.firstPaint.test.tsx
packages/ui/components/DiagramBlock.lazyRetry.test.tsx
opencode-v2:
name: OpenCode 2 installed package
runs-on: ubuntu-latest
# The smoke's own budgets (health 120s, plugin activation 300s) are the real assertion;
# this is only a backstop so a genuinely wedged runner cannot idle for hours.
timeout-minutes: 25
steps:
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
with:
persist-credentials: false
- uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0
with:
bun-version: 1.3.14
- name: Install dependencies
run: bun install --frozen-lockfile
- name: Install OpenCode 2
# zizmor: ignore[adhoc-packages] This integration test requires the exact
# globally installed CLI. Owner: maintainers; reassess a locked fixture
# by 2027-08-11.
run: npm install --global @opencode-ai/cli@0.0.0-next-16775
- name: Build OpenCode plugin assets
run: bun run build:review && bun run build:hook && bun run build:opencode
# This is the only job with the single-file bundles on disk, so it is the
# only place the built-artifact assertions in entry-assets can actually
# run (they skip on an unbuilt checkout). Chief among them: no inlined
# WebAssembly survived the bundle.
- name: Assert built bundles ship no inlined WASM
run: bun test tests/entry-assets.test.ts
- name: Pack OpenCode plugin
working-directory: apps/opencode-plugin
run: npm pack --ignore-scripts --pack-destination "$RUNNER_TEMP"
- name: Run installed-package smoke
# Glob the tarball so this job survives version bumps; npm pack wrote
# exactly one plannotator-opencode-*.tgz into RUNNER_TEMP above.
run: >-
bun run --cwd apps/opencode-plugin smoke:v2 --
"$(command -v opencode2)"
"$RUNNER_TEMP"/plannotator-opencode-*.tgz
guides-show:
name: guides.show viewer (budgets + pinned manifest)
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
with:
persist-credentials: false
# The merge commit plus both parents, so the path gate below can diff
# the PR against its base without fetching the whole history.
fetch-depth: 2
# Path gate. The viewer build only reads apps/guides-show and the packages
# it bundles, so an unrelated PR skips the install and the build. Anything
# this cannot resolve (a push to main, a manual run, a checkout that is not
# the PR merge commit) runs the job: skipping is the risky direction.
- name: Does this change touch the viewer's inputs?
id: gate
run: |
set -eu
run=true
# HEAD is the PR merge commit, so HEAD^1 is the base tip and diffing
# the two is exactly the PR's changes. Requiring HEAD^2 confirms this
# really is the merge commit before trusting that.
if [ "$GITHUB_EVENT_NAME" = "pull_request" ] \
&& base=$(git rev-parse --verify --quiet 'HEAD^1') \
&& git rev-parse --verify --quiet 'HEAD^2' >/dev/null; then
changed=$(git diff --name-only "$base" HEAD)
if grep -Eq '^(apps/guides-show/|packages/(guide-viewer|ui|review-editor|core)/)' <<< "$changed"; then
run=true
else
run=false
fi
fi
echo "run=$run" >> "$GITHUB_OUTPUT"
- uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0
if: steps.gate.outputs.run == 'true'
with:
bun-version: 1.3.14
- name: Install dependencies
if: steps.gate.outputs.run == 'true'
run: bun install --frozen-lockfile
# The release workflow (guides-show-deploy.yml) refuses to publish when
# the viewer build blows its size budgets or the checked-in manifest no
# longer matches it, but that gate fires at the tag. The manifest pin is
# compiled into the release binaries independently of the viewer deploy,
# so a budget or manifest break has to surface on the PR that causes it.
# Regenerate with `bun run --cwd apps/guides-show build:viewer &&
# bun run --cwd apps/guides-show sync:manifest`.
- name: Build viewer
if: steps.gate.outputs.run == 'true'
run: bun run --cwd apps/guides-show build:viewer
- name: Size budgets
if: steps.gate.outputs.run == 'true'
run: bun run --cwd apps/guides-show check:budgets
# The viewer build is deterministic across platforms at the pinned Bun and
# lockfile (macOS and Linux produce identical hashes from a clean install),
# but a drifted local environment silently builds different bytes: a bun
# store with stale duplicate package versions reproduced exactly that. This
# Linux build is the authority the pin must match, so publish its manifest;
# when the check below fails and a fresh local regen still disagrees,
# download this artifact to apps/guides-show/dist/viewer/manifest.json, run
# `bun run --cwd apps/guides-show sync:manifest`, commit the result, and
# rebuild your environment (rm -rf node_modules && bun install --frozen-lockfile).
- name: Publish built viewer manifest for pin regeneration
if: always() && steps.gate.outputs.run == 'true'
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v6.0.0
with:
name: guide-viewer-manifest
path: apps/guides-show/dist/viewer/manifest.json
retention-days: 7
- name: Checked-in manifest matches this build
if: steps.gate.outputs.run == 'true'
run: bun run --cwd apps/guides-show check:manifest
uninstall-windows:
# Run the filesystem and path logic under real Windows path semantics.
# The tests inject temporary homes and process boundaries, so this job
# never touches the runner's actual agent installations or user data.
name: Uninstall lifecycle (Windows)
runs-on: windows-latest
steps:
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
with:
persist-credentials: false
- uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0
with:
bun-version: 1.3.14
- name: Install dependencies
run: bun install --frozen-lockfile
- name: Run focused uninstall tests
run: bun test packages/server/uninstall.test.ts apps/hook/server/cli.test.ts
pi-extension-ai-runtime-windows:
# Exercises the Pi extension's Node/jiti server mirror on Windows with an
# npm-style `pi` shim pair. This catches regressions where `where pi`
# resolves the extensionless shim before pi.cmd and the Ask AI provider
# crashes before the plan review UI opens.
name: Pi extension AI runtime (Windows)
runs-on: windows-latest
steps:
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
with:
persist-credentials: false
- uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0
with:
bun-version: 1.3.14
- uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0
with:
node-version: 24
- name: Install dependencies
run: bun install --frozen-lockfile
- name: Generate Pi extension shared copies
shell: bash
run: bash apps/pi-extension/vendor.sh
- name: Run workspace symlink tests
run: bun test packages/server/review-workspace.test.ts --test-name-pattern discoverWorkspaceRepoPaths
- name: Build Pi AI runtime smoke
run: bun build scripts/smoke-pi-extension-ai-runtime.ts --target=node --outfile "$env:RUNNER_TEMP/pi-ai-runtime-smoke.mjs"
- name: Run Pi AI runtime smoke
run: node "$env:RUNNER_TEMP/pi-ai-runtime-smoke.mjs"
install-cmd-windows:
# End-to-end integration test for scripts/install.cmd on real cmd.exe.
# The unit tests in scripts/install.test.ts are file-content string checks
# that run on Linux and never exercise cmd's delayed-expansion parser or
# the embedded `node -e` Gemini merge — exactly where issue #506 lived.
# This job runs install.cmd end-to-end on Windows with a seeded ~/.gemini
# settings.json fixture so the Gemini merge path actually executes and
# any regression of #506 (or similar cmd-parser bugs) fails CI.
name: install.cmd (Windows integration)
runs-on: windows-latest
steps:
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
with:
persist-credentials: false
- name: Seed fake ~/.gemini/settings.json with pre-existing hook
shell: pwsh
run: |
New-Item -ItemType Directory -Force -Path "$env:USERPROFILE\.gemini" | Out-Null
# Fixture mirrors the shape of a real Gemini settings.json (top-level
# `hooks.BeforeTool` array plus unrelated sibling keys) but uses only
# obviously-fake values. Must NOT contain the literal string
# "plannotator" anywhere — install.cmd's Gemini block is gated on
# `findstr /c:"plannotator"` returning non-zero and would otherwise
# skip the merge entirely.
$fixture = @'
{
"theme": "ci-fixture-theme",
"hooks": {
"BeforeTool": [
{
"matcher": "ci-fixture-existing-matcher",
"hooks": [
{
"type": "command",
"command": "ci-fixture-existing-command",
"timeout": 1000
}
]
}
]
},
"general": {
"ciFixtureSentinel": true
}
}
'@
Set-Content -Path "$env:USERPROFILE\.gemini\settings.json" -Value $fixture -NoNewline
- name: Run install.cmd end-to-end # zizmor: ignore[misfeature]
# CMD is the subject of this regression test; replacing the shell would
# invalidate it. Owner: maintainers; reassess by 2027-08-11.
shell: cmd
# v0.17.1 is pinned intentionally. This test needs a real binary
# to download so it can exercise install.cmd end-to-end — SHA256
# verification, skills sparse-checkout, and (critically) the
# embedded `node -e` Gemini merge path that was the site of
# issue #506. Using `latest` would couple the Windows regression
# test to whatever version is currently released, so a bad
# release would retroactively break CI on every branch.
#
# v0.17.1 was the current release when the test was added and is
# locked in place by GitHub Immutable Releases. If you ever need
# to bump it (e.g. this version becomes too old to represent the
# install flow we care about), verify the replacement has:
# - plannotator-win32-x64.exe attached
# - plannotator-win32-x64.exe.sha256 attached
# - the install.cmd and packages/shared/ layout your branch
# under test expects to find in apps/skills/
# A missing or mismatched release asset surfaces as a curl 404
# in this step with no obvious connection to the test's purpose.
run: scripts\install.cmd v0.17.1 --skip-attestation
- name: Verify Gemini settings.json was merged correctly
shell: pwsh
run: |
$path = "$env:USERPROFILE\.gemini\settings.json"
if (-not (Test-Path $path)) { throw "settings.json missing after install" }
# Must still parse as JSON after the merge (regression for #506,
# where cmd's delayed expansion corrupted the embedded node script
# and left settings.json in a broken state).
$s = Get-Content $path -Raw | ConvertFrom-Json
# The plannotator hook must have been added.
$plannotatorEntries = $s.hooks.BeforeTool | Where-Object {
$_.matcher -eq 'exit_plan_mode'
}
if (-not $plannotatorEntries) {
throw "plannotator hook was not added to BeforeTool"
}
$planCmd = $plannotatorEntries[0].hooks | Where-Object {
$_.command -eq 'plannotator'
}
if (-not $planCmd) {
throw "plannotator command entry missing inside the new hook"
}
# The pre-existing (fixture) hook must have survived the merge.
# The original buggy JS was `if(!s.hooks.BeforeTool)s.hooks.BeforeTool=[]`
# which — after cmd ate the `!` — wiped existing arrays. The fix
# (`s.hooks.BeforeTool = s.hooks.BeforeTool || []`) must preserve them.
$fixtureHook = $s.hooks.BeforeTool | Where-Object {
$_.matcher -eq 'ci-fixture-existing-matcher'
}
if (-not $fixtureHook) {
throw "pre-existing hook was wiped — merge clobbered existing data"
}
# Unrelated top-level keys must survive the merge.
if ($s.theme -ne 'ci-fixture-theme') {
throw "unrelated top-level field 'theme' was mangled"
}
if ($s.general.ciFixtureSentinel -ne $true) {
throw "unrelated top-level field 'general' was mangled"
}
Write-Host "✓ Gemini settings.json merge verified (issue #506 regression guard)"
- name: Attestation pre-flight rejects v0.17.1 on real cmd.exe
shell: pwsh
run: |
# Regression guard: the main feature of this PR (three-layer
# verification opt-in + MIN_ATTESTED_VERSION pre-flight +
# injection-safe $env:-based PowerShell version comparison) had
# no runtime coverage on Windows because the previous
# integration step passes --skip-attestation.
#
# We can't test the SUCCESS path (valid attested release)
# because v0.17.1 is the current latest and it predates the
# release.yml attestation step. Until the first post-merge
# release exists, the only realistic end-to-end test is the
# REJECTION path: invoke install.cmd with --verify-attestation
# against v0.17.1 and assert the pre-flight rejects with
# exit != 0 and stderr containing "predates".
#
# This exercises on a real cmd.exe:
# - setlocal enabledelayedexpansion parser under the guard
# - three-layer resolution reaching the CLI flag layer
# - the :~1 substring (instead of :v= global substitution)
# - the pre-release tag detection (negative — v0.17.1 is stable)
# - the PowerShell shell-out with $env:TAG_NUM / $env:MIN_NUM
# (injection-safe — the previous interpolation would have
# allowed arbitrary PS execution via --version)
# - the `[version] -ge` comparison returning false
# - the "predates" error message block
$installedBinary = "$env:USERPROFILE\.local\bin\plannotator.exe"
# Capture the currently-installed binary's hash BEFORE running
# the rejection test. The earlier Gemini-merge integration step
# installed v0.17.1 at this path; we use the captured hash as
# a baseline so we can prove the rejected invocation left it
# untouched (no wasted download, no overwrite).
if (-not (Test-Path $installedBinary)) {
throw "Expected $installedBinary to exist from the earlier install.cmd step, but it's missing. Cannot baseline the preservation check."
}
$baselineHash = (Get-FileHash $installedBinary -Algorithm SHA256).Hash
$baselineWriteTime = (Get-Item $installedBinary).LastWriteTime
$stderrFile = New-TemporaryFile
$p = Start-Process -Wait -PassThru -NoNewWindow cmd `
-ArgumentList '/c','scripts\install.cmd v0.17.1 --verify-attestation' `
-RedirectStandardError $stderrFile.FullName
$stderr = Get-Content $stderrFile.FullName -Raw -ErrorAction SilentlyContinue
Remove-Item $stderrFile.FullName -ErrorAction SilentlyContinue
if ($p.ExitCode -eq 0) {
throw "install.cmd v0.17.1 --verify-attestation should have been rejected by the MIN_ATTESTED_VERSION pre-flight, but exited 0. stderr: $stderr"
}
if ($stderr -notmatch 'predates') {
throw "install.cmd rejected with exit $($p.ExitCode), but not via the pre-flight guard. Expected 'predates' in stderr, got: $stderr"
}
# Assert the pre-flight ran BEFORE any download / install step.
# If the binary's hash or mtime changed, something downloaded
# and moved a new file into place — meaning the pre-flight
# rejection happened late (after the download step) instead
# of early (before it). Catches future regressions that re-
# introduce the post-download pre-flight pattern.
$postHash = (Get-FileHash $installedBinary -Algorithm SHA256).Hash
$postWriteTime = (Get-Item $installedBinary).LastWriteTime
if ($postHash -ne $baselineHash) {
throw "Binary at $installedBinary was overwritten during the rejected --verify-attestation run. Baseline SHA256 $baselineHash, post SHA256 $postHash. The pre-flight must run before any download."
}
if ($postWriteTime -ne $baselineWriteTime) {
throw "Binary at $installedBinary had its LastWriteTime modified during the rejected --verify-attestation run. Baseline $baselineWriteTime, post $postWriteTime."
}
Write-Host "✓ MIN_ATTESTED_VERSION pre-flight rejected v0.17.1 via the expected code path"
Write-Host "✓ Installed binary was preserved (SHA256 and LastWriteTime both unchanged)"
- name: Verify Claude Code slash command files contain the shell-invocation prefix
shell: pwsh
run: |
# Claude Code commands are deprecated in favor of skills: the
# installer no longer writes ~/.claude/commands files AT ALL (the
# core skills in ~/.claude/skills provide the slash commands). The
# old `echo ^!` escape regression class is structurally gone — no
# command content is generated by the installer anymore. Guard the
# NEW contract: a fresh install must not create plannotator command
# files.
$cmdDir = "$env:USERPROFILE\.claude\commands"
foreach ($file in @("plannotator-review.md", "plannotator-annotate.md", "plannotator-last.md")) {
$path = Join-Path $cmdDir $file
if (Test-Path $path) {
throw "Installer wrote a deprecated Claude command file: $path. Commands were replaced by skills; the installer must not generate them."
}
}
Write-Host "✓ No deprecated Claude Code command files were written (skills are the slash commands now)"
# Gemini slash commands — `.toml` files with `!{plannotator ...}` invocation
# syntax, now COPIED VERBATIM from the sparse checkout (apps/gemini/commands
# at the pinned tag) instead of being generated by echo escapes. The earlier
# integration step seeded `~/.gemini/settings.json`, so install.cmd's Gemini
# block fired. The `!{plannotator` content check verifies the verbatim copy
# actually delivered intact files (and would catch a regression back to
# generated content that mangles the `!`).
$geminiDir = "$env:USERPROFILE\.gemini\commands"
foreach ($file in @("plannotator-review.toml", "plannotator-annotate.toml")) {
$path = Join-Path $geminiDir $file
if (-not (Test-Path $path)) {
throw "Expected Gemini command file missing: $path"
}
$content = Get-Content $path -Raw
if ($content -notmatch '!\{plannotator') {
throw "Gemini command file $file is missing the '!' shell-invocation prefix. Content: $content"
}
}
Write-Host "✓ Both Gemini slash command files contain the '!' prefix"
- name: Unknown flag is rejected with non-zero exit
shell: pwsh
run: |
# Regression guard for the review finding that install.cmd silently
# reinterpreted typoed flags as version strings. A leading-dash token
# that doesn't match a known flag must now produce a non-zero exit
# AND emit "Unknown option:" on stderr — the latter is the real
# discriminator between the guard triggering and some other failure
# mode (network, gh auth, pre-PR release without an attestation)
# that also happens to exit non-zero.
#
# `--verify-attesttion` below is INTENTIONALLY MISSPELLED. Do not
# "correct" it during a typo sweep — the valid spelling is a real
# flag and would bypass this guard. The stderr assertion below
# would catch the drift, but the comment is the first line of
# defense for future maintainers.
$stderrFile = New-TemporaryFile
$p = Start-Process -Wait -PassThru -NoNewWindow cmd `
-ArgumentList '/c','scripts\install.cmd --verify-attesttion' `
-RedirectStandardError $stderrFile.FullName
$stderr = Get-Content $stderrFile.FullName -Raw -ErrorAction SilentlyContinue
Remove-Item $stderrFile.FullName -ErrorAction SilentlyContinue
if ($p.ExitCode -eq 0) {
throw "install.cmd should have rejected --verify-attesttion but exited 0. stderr: $stderr"
}
if ($stderr -notmatch 'Unknown option:') {
throw "install.cmd exited $($p.ExitCode) but not via the unknown-flag guard. Expected 'Unknown option:' in stderr but got: $stderr"
}
Write-Host "✓ Unknown flag rejected with exit code $($p.ExitCode) via the unknown-flag guard"
install-ps1-windows:
# End-to-end integration test for scripts/install.ps1 on real PowerShell.
# This specifically covers PLANNOTATOR_DATA_DIR tilde expansion because
# installer config lookup happens before any shared runtime code is loaded.
name: install.ps1 (Windows integration)
runs-on: windows-latest
steps:
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
with:
persist-credentials: false
- name: Run install.ps1 end-to-end
shell: pwsh
run: .\scripts\install.ps1 v0.17.1 -SkipAttestation
- name: Config opt-in resolves PLANNOTATOR_DATA_DIR tilde paths
shell: pwsh
run: |
$configDir = Join-Path $env:USERPROFILE "plannotator-ci-config"
New-Item -ItemType Directory -Force -Path $configDir | Out-Null
Set-Content -Path (Join-Path $configDir "config.json") -Value '{ "verifyAttestation": true }' -NoNewline
$installedBinary = "$env:LOCALAPPDATA\plannotator\plannotator.exe"
if (-not (Test-Path $installedBinary)) {
throw "Expected $installedBinary to exist from the earlier install.ps1 step, but it's missing."
}
$baselineHash = (Get-FileHash $installedBinary -Algorithm SHA256).Hash
$baselineWriteTime = (Get-Item $installedBinary).LastWriteTime
$env:PLANNOTATOR_DATA_DIR = '~\plannotator-ci-config'
$stderrFile = New-TemporaryFile
$p = Start-Process -Wait -PassThru -NoNewWindow pwsh `
-ArgumentList '-NoProfile','-ExecutionPolicy','Bypass','-File','.\scripts\install.ps1','v0.17.1' `
-RedirectStandardError $stderrFile.FullName
$stderr = Get-Content $stderrFile.FullName -Raw -ErrorAction SilentlyContinue
Remove-Item $stderrFile.FullName -ErrorAction SilentlyContinue
if ($p.ExitCode -eq 0) {
throw "install.ps1 should have found config.json via PLANNOTATOR_DATA_DIR=~\plannotator-ci-config and rejected v0.17.1, but exited 0."
}
if ($stderr -notmatch 'predates') {
throw "install.ps1 exited $($p.ExitCode), but not via the expected attestation pre-flight. Expected 'predates' in stderr, got: $stderr"
}
$postHash = (Get-FileHash $installedBinary -Algorithm SHA256).Hash
$postWriteTime = (Get-Item $installedBinary).LastWriteTime
if ($postHash -ne $baselineHash) {
throw "Binary at $installedBinary was overwritten during the rejected config-driven verifyAttestation run."
}
if ($postWriteTime -ne $baselineWriteTime) {
throw "Binary at $installedBinary had its LastWriteTime modified during the rejected config-driven verifyAttestation run."
}
Write-Host "✓ PLANNOTATOR_DATA_DIR tilde expansion found config.json"
Write-Host "✓ Config-driven verifyAttestation rejected before download/install"
Reference in New Issue View Git Blame Copy Permalink