The skill launcher and `impeccable install` both fail closed when a
release binary's `.sha256` sidecar cannot be fetched or carries no hash:
they refuse rather than cache an unverified binary. The npm shim did not.
It only compared when a hash was present, so a 404, an empty sidecar, or
a truncated one all wrote the payload straight into
`~/.impeccable/bin/<version>/` and exec'd it.
It now refuses in the same cases, with wording that matches the launcher,
and writes nothing until the hash matches, so a refusal leaves the cache
dir empty. IMPECCABLE_BIN and the optional-dependency lookup are
untouched: neither downloads.
tests/cli-shim.test.mjs runs the real shim against a throwaway HTTP
server and covers missing, empty, and mismatched sidecars, plus the
matching-sidecar and IMPECCABLE_BIN paths. The two refusal cases fail
against the old shim.
Co-Authored-By: Claude Code <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vau2X53xGTjjTCXWMVBoNY
Brings the leak guard from #718 onto the branch and makes its guarantee hold
for the Rust engine instead of the Node scripts it was written against.
Conflicts and how each was resolved:
- tests/live-poll-stream.test.mjs, tests/live-server.test.mjs,
tests/live-target-context.test.mjs (modify/delete): kept deleted. They drove
skill/scripts/live-server.mjs, which does not exist here; the verb behavior
they covered is the oracle's job now. Their entries came out of
test-suites.mjs along with the rest of main's live list, which is Node-script
coverage this branch already retired.
- scripts/test-suites.mjs: took main's two new entries that still apply,
process-group.test.mjs into core and live-server-leak.test.mjs into live, plus
the infra trigger patterns for the three new scripts/lib modules. Dropped
main's pin.test.mjs (no such file here).
- package.json: kept test:cleanup, dropped test:cli-e2e (no cli-e2e suite here).
- scripts/run-tests.mjs: rewritten to hold both sides rather than picking one.
From #718: the createGroupShutdown state machine, the per-suite run-id marker
env, the post-suite leak check, and --cleanup. From 47f18713: the per-command
wall-clock cap with its per-suite wallClockMs override and
IMPECCABLE_TEST_WALL_CLOCK_MS, plus the killed-by-signal report. The two agree
on the detached process group, so they compose: the cap SIGKILLs that group
when a command wedges, the shutdown handler ends it on a signal, and both now
sweep for leaked servers before exiting. #718's handler replaces the old raw
signal forwarding, which sent one signal and never escalated.
- tests/live-e2e/session.mjs: kept both sides. The binary-driven boot
(runEngineSync, requireEngineBin, engineEnv) stands, with armLiveServerReaper
at module scope and trackServerChild around the fixture dev server.
Ported to the rest of the branch:
- tests/oracle/lib.mjs arms the reaper and tracks the daemon child. Its daemon
steps spawn live-server detached, so a SIGKILLed oracle run used to strand
one; buildInvocation already inherits process.env, so the marker reaches it.
- tests/live-server-leak.test.mjs now boots the engine binary through
tests/lib/engine-bin.mjs and skips cleanly without one.
No crate change was needed. The daemon spawn does env_clear().envs(env) against
Io::stdio()'s env, which is std::env::vars(), so the detached Rust process
carries the parent environment and the markers reach it. Verified against a
real --background daemon: found by run id and by repo marker, not found by an
adjacent checkout's marker. CLAUDE.md now says so, since narrowing that env
would make the guard silently blind.
Verified with a fresh cargo build --release -p impeccable:
- IMPECCABLE_BIN=... bun run test green end to end: core 90, oracle 1 (zero
unreviewed differences), detector 1, live 159 (157 pass, 2 skipped),
framework 186, plugin-e2e 4. Zero servers left.
- SIGKILL repro against impeccable live-server --background: 1 daemon up, 0
after with the reaper, 1 surviving with parent pid 1 under
IMPECCABLE_NO_TEST_REAPER=1. bun run test:cleanup then kills exactly that one.
- The leak test fails under IMPECCABLE_NO_TEST_REAPER=1 and passes with it.
- IMPECCABLE_E2E_ONLY=vite8-react-plain bun run test:live-e2e 4/4.
- bun run build green.
AI assistance: prepared by Claude Code under pbakaus's direction.
Co-Authored-By: Claude Code <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vau2X53xGTjjTCXWMVBoNY
* Tests: stop the harness leaking live-server processes
Nothing owned a live server past the exit paths JavaScript can observe. The
live unit tests spawn the server as a direct child and stop it with an HTTP
/stop plus proc.kill() inside an after() hook; the e2e session and the
target-context tests boot it through `live-server --background` / live.mjs,
which spawns a detached, unref'd daemon that only the `stop` verb ever ends.
A POSIX child does not die with its parent, and a detached daemon is orphaned
to pid 1 from birth, so any exit that skipped teardown (a node:test timeout, a
SIGKILL of the runner, a Ctrl-C, an assertion that threw before the hook) left
the server listening on a fixed live-suite port for good. scripts/run-tests.mjs
did not compensate: it used blocking spawnSync, so no signal handler could run;
it left suite commands in its own process group with nothing that could kill
that group; and it never checked afterwards whether anything survived. Days of
local runs accumulated 197 orphans on one machine, the oldest four days old,
until `bun run test:live` could not claim its ports.
The fix is structural rather than a cleanup sweep bolted on the end, and it is
deliberately implementation-agnostic so it holds for the Node scripts here and
for the Rust `impeccable live-server` on rust-swap:
- tests/lib/live-servers.mjs. armLiveServerReaper(), called once at module
scope by every test file that starts a server, stamps the process env with a
unique marker, installs exit and signal handlers, and spawns a detached
reaper holding a pipe to the process. SIGKILL the process and the pipe closes,
the reaper wakes on EOF and kills the servers carrying that marker. That is
the one case no in-process cleanup can reach. trackServerChild() also
registers direct children (live servers and fixture dev servers) so the
ordinary exits are a cheap kill by handle.
- scripts/lib/live-server-processes.mjs. The scan and kill primitives, shared
by the reaper and the runner. Processes are matched by the environment marker
the harness exported, never by name or port, so a sweep can only ever reach a
server this repo's tests started.
- scripts/run-tests.mjs. Each suite command now runs as its own process-group
leader with SIGINT/SIGTERM/SIGHUP forwarded to the group, and after every
suite the runner checks for live servers carrying that suite's run id. A
survivor is killed and fails the run, so the next leak surfaces in the run
that caused it instead of on a laptop days later. IMPECCABLE_SKIP_LEAK_CHECK=1
bypasses it. `bun run test:cleanup` sweeps leftovers from earlier runs.
- tests/live-server-leak.test.mjs pins the guarantee: it boots a real server
under a process it then SIGKILLs, and fails if the server outlives it. With
IMPECCABLE_NO_TEST_REAPER=1 the test fails, which is what makes it a
regression test rather than a tautology.
Verified: bun run test:live green with zero survivors; scoped live-e2e
(vite8-react-plain) matches pristine main test for test; the SIGKILL repro goes
from 2 orphans to 0; SIGINT and SIGKILL of the runner itself both leave nothing
behind; bun run build green.
Fixes#717
AI assistance: prepared by Claude Code under pbakaus's direction.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vau2X53xGTjjTCXWMVBoNY
* Review fixes: scope the sweep to whole env entries only
Five review findings on #718, all in the matching layer that decides which
processes a sweep may touch.
The repository-path fallback is gone (Greptile P1). `bun run test:cleanup`
passed REPO_ROOT to findLiveServers, which then also matched any live-server
command line under the checkout, marker or not. A developer running
`impeccable live` in this repo has exactly that command line, so the cleanup
could have killed their own session. The PR promised matching on the exported
environment marker and nothing else; now it does. The cost is that a server
from a run predating the marker is no longer found and has to be killed by
hand, which is the right trade.
Environment entries are compared whole on macOS and BSD (Greptile P1). `ps -E`
flattens the environment into the command column, and that line was searched
with a plain substring test, so IMPECCABLE_TEST_REPO=/work/impeccable also
matched /work/impeccable-copy and one checkout's cleanup could reach a
neighbouring checkout's servers. envLineHasEntry() now requires the marker to
start an entry (line start or whitespace) and to end one (line end, or
whitespace followed by the next KEY=), which is the same whole-entry
comparison the Linux /proc branch already did. Six unit tests cover it,
including the adjacent-path negative case, and a live probe against real
`ps -E` output confirms an exact repo matches while /work/impeccable-copy and
a run-id prefix do not.
The SIGKILL regression test now skips on win32 with a stated reason (Copilot).
The reaper is a POSIX mechanism and armLiveServerReaper() does not arm it
there, so the test asserted a guarantee Windows does not make yet.
Signal exits use the shell convention 128 + signum in both the runner and the
test helper (Copilot, two threads). SIGHUP returned 143; it is 129. Read from
os.constants.signals rather than a hand-written table.
Verified: leak test 7/7 (2 guard, 5 matcher); bun run test:live 895 tests, 0
fail, 0 survivors; scoped live-e2e (vite8-react-plain) 3 pass / 1 fail,
matching pristine main; SIGKILL repro 3 servers up, 0 after; bun run build
green.
AI assistance: prepared by Claude Code under pbakaus's direction.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vau2X53xGTjjTCXWMVBoNY
* Review fix: make marker values opaque so the matcher has no ambiguous case
Greptile's follow-up P1 on the parser was right, and the parser was the wrong
place to answer it. envLineHasEntry ended an entry at "whitespace followed by
the next KEY=", so a checkout path that extended another one with whitespace
plus a KEY=-shaped token still defeated it, which is exactly the ambiguity the
docblock admitted to. A format that cannot be parsed unambiguously should not
be handed ambiguous input.
So the fix is at the source: no marker value is a path any more. IMPECCABLE_TEST_REPO
now carries repoMarker(), the first 16 hex characters of the sha256 of the
checkout's real path, and the runner and the cleanup command both compute it
the same way from REPO_ROOT. Two checkouts whose paths share a prefix get
unrelated hashes, so a substring cannot arise in the first place, and every
spelling of one checkout (trailing slash, `.` segment, symlink, /private
prefix) resolves to one marker. The run id is now repoMarker plus 8 random
bytes of hex, and the process id p<pid> plus the same, both from a
whitespace-free alphabet.
With every value fixed-alphabet, envLineHasEntry needs only "starts an entry
and ends at whitespace or line end". The KEY= lookahead is gone and so is the
documented unresolvable case. assertMarkerValue keeps the invariant honest: it
refuses any value outside [A-Za-z0-9_-] with a message that says to hash it,
so a future caller that passes a path gets a loud error instead of a silent
mismatch. The readable path is still available for a human reading `ps -E`
output, exported separately as IMPECCABLE_TEST_REPO_PATH, which nothing
matches on and the docblock says so.
Matcher tests: the space-in-value case is gone, since that value can no longer
exist. Added a strict-prefix case (a longer hash-shaped value starting with the
marker), an adjacent-checkout case asserting the two hashes do not even share a
prefix, a symlink/trailing-slash case against real directories, an alphabet
check on all three generators, and one asserting assertMarkerValue throws.
Verified: leak test 10/10; bun run test:live 898 tests, 0 fail, 0 survivors;
scoped live-e2e (vite8-react-plain) 3 pass / 1 fail, matching pristine main;
SIGKILL repro 1 server up, 0 after; bun run build green. A probe against real
`ps -E` output with a hashed marker: this checkout 1 match, its trailing-slash
spelling 1, an adjacent checkout 0, exact run id 1, a run-id prefix 0.
AI assistance: prepared by Claude Code under pbakaus's direction.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vau2X53xGTjjTCXWMVBoNY
* Review fixes: async group shutdown, and a Windows-safe symlink test
Two Cursor Bugbot findings, both real.
killCurrentGroup busy-waited on alive(child.pid) after sending SIGTERM, which
could never work. A dead child stays a zombie until its parent reaps it, the
parent here is the runner, and the runner reaps through libuv when the event
loop runs. The spin blocked the very loop that would have done the reaping and
then read the unreaped zombie as alive, so every SIGINT, SIGTERM and SIGHUP
burned the full 2s grace and ended in a needless SIGKILL. There is no waitpid
from JavaScript that sees through this, so the wait is now asynchronous and
keyed on the child's own exit event. The logic moved to
scripts/lib/process-group.mjs: trackChildExit exposes the exit as a flag and a
promise, stopGroup races that promise against the grace period and escalates to
SIGKILL only if it loses, and killGroupSync stays synchronous for
process.on('exit'), where nothing can be awaited, so it sends SIGTERM then
SIGKILL without pretending to wait. A second Ctrl-C now skips the grace period
entirely rather than queueing behind it.
Measured on a real SIGINT to a running live suite: 2027ms before, 34ms after.
tests/process-group.test.mjs pins both halves, including the escalation path
against a child that traps SIGTERM, which is not otherwise reachable from a
registered suite.
The repoMarker symlink test called symlinkSync with no type, which throws EPERM
on Windows without Developer Mode. It now passes 'junction' there and 'dir'
elsewhere, the same shape tests/concept-seed.test.mjs uses, and the
trailing-slash and dot-segment cases split into their own test so they keep
running on every platform regardless.
Merged origin/main (through #716) to re-level the branch.
Verified: leak and process-group tests 16/16; bun run test:live 900 tests, 0
fail, 0 survivors; scoped live-e2e (vite8-react-plain) now 4/4, with the
orphaned-session test that #716 fixed passing in 7.2s; bun run build green.
AI assistance: prepared by Claude Code under pbakaus's direction.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vau2X53xGTjjTCXWMVBoNY
* Review fix: a second Ctrl-C must reach the group the first one is stopping
Cursor Bugbot caught a bug I introduced with the async shutdown, and it is the
same class of leak this PR exists to close. The signal handler cleared
currentChild before awaiting stopGroup, so a second Ctrl-C read a null handle:
killGroupSync did nothing, process.exit walked away from the SIGKILL escalation
still in flight, and because the suite is spawned detached it kept running
after the runner was gone. Impatience with a stuck suite produced exactly the
orphan the change is supposed to prevent.
The shutdown state machine moved into scripts/lib/process-group.mjs as
createGroupShutdown, which holds the group in `stopping` for as long as it is
being ended rather than dropping the only reference to it. A second signal
kills that handle and leaves; process.on('exit') looks at `current` or
`stopping`, so the last-resort path reaches a group mid-shutdown too. The
runner keeps no shutdown state of its own now, which is what made the bug
possible to write in the first place.
The extraction is what makes it testable: `exit` is injectable, so
tests/process-group.test.mjs can drive two signals at a stubborn child that
traps SIGTERM and assert the group dies in under 2s against a 30s grace. Point
that test at the old logic (killGroupSync on the cleared reference) and it
hangs out the full grace and fails, which is the check that it pins something
real. Five cases in all, including the exit-handler path and the no-child case.
Verified: process-group 10/10, live-server-leak 11/11; real double SIGINT to a
running live suite exits in 24ms with zero group members and zero servers left;
bun run test:live 900 tests, 0 fail, 0 survivors; scoped live-e2e
(vite8-react-plain) 4/4; bun run build green.
The core suite wedged twice locally in tests/build-phase.test.mjs, the
pre-existing unbounded-spawnSync hang noted in the PR description that
rust-swap's 47f18713 fixes. Unrelated to this change: CI is green on both Node
versions, and process-group.test.mjs passes inside that batch.
AI assistance: prepared by Claude Code under pbakaus's direction.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vau2X53xGTjjTCXWMVBoNY
---------
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Upstream sha 9736a9f6e9, the part of it that
lives in `cli/bin/commands/skills.mjs` rather than `pin.mjs`.
`copy_provider_commands` mirrors `copy_provider_skills` for a provider's
compiled `commands/` dir: project scope writes `<root>/<configDir>/commands`,
user scope writes the config dir OpenCode actually scans
(`OPENCODE_CONFIG_DIR` -> `XDG_CONFIG_HOME/opencode` -> `~/.config/opencode`),
and a pre-#406 global install at `~/.opencode/commands/` loses exactly the
files just written while siblings, symlinked dirs and home-rooted git repos
are left alone. It runs on install, on the reinstall refresh, on update, and
on link, which is the only path that can deliver the bridge to a linked
install.
`is_up_to_date` now compares the bundle's command files too, so an install
whose skills match but whose bridge is missing or drifted refreshes instead of
reporting success while the slash command stays absent. Only bundle-shipped
files are compared, so a pinned shortcut never affects freshness.
`tests/copy-provider-commands.test.js` arrived with the merge importing the
deleted `cli/bin/commands/skills.mjs`; its scenarios are ported to
`crates/skills/tests/provider_commands_tests.rs` (project scope, the three
user-scope dir resolutions, the legacy migration and its two guards, a
provider with no commands dir, and the four `isUpToDate` command-awareness
cases), and the file is removed and deregistered.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vau2X53xGTjjTCXWMVBoNY
Textual merge only. The JS engine and skill scripts stay deleted on this
branch; every behavior change they carried is ported to the Rust crates in
the commits that follow.
Conflict resolutions:
- skill/SKILL.src.md, skill/reference/new-work.md: main's new wording, with
the branch's launcher invocations kept in place of `node <script>.mjs`.
- scripts/test-suites.mjs: registers main's new build-tooling tests
(copy-provider-commands, root-commands-sync, opencode-commands) and leaves
the tests for deleted JS modules deregistered.
- Every modify/delete conflict under cli/engine, cli/bin/commands,
skill/scripts/*.mjs and tests/ for deleted modules keeps the deletion.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vau2X53xGTjjTCXWMVBoNY
The detector is open source. The rules it ships were already public in this
repo's git history and in every npm tarball of the JS engine, so a closed
binary bought nothing it could keep; the moat is the service (the catalog,
the labs, the review pipeline), not the check functions. Keeping them behind
a prebuilt archive cost a C-ABI, an exact toolchain pin, a build-time
download, a second release to order ahead of every engine release, and a
serde layer that had to serve two encodings.
Deleted
- crates/core/src/ffi.rs, crates/core/build.rs, crates/core/tests/boundary.rs
and the shim modules under src/checks and src/browser.
- crates/foundation/src/boundary.rs and the postcard dependency.
- DETECTOR_VERSION, scripts/check-detector-release.mjs and its test, the
check:detector-release script, the detector gate and
IMPECCABLE_SKIP_DETECTOR_CHECK in scripts/release.mjs.
- scripts/lib/detector-bundle.mjs and tests/detector-bundle.test.mjs (the
vendoring path for the closed browser bundle).
- scripts/build-browser-detector.js and the build:browser script (a stub
since the JS engine left the tree).
- xtask's detector-archive subcommand and its public-repo lookup.
Came back
- crates/core is now the rule logic itself: every check_* / scan_*, the
browser adapters, the visual-contrast decisions. It re-exports foundation
as before, so no consumer changed. Its vectors dispatcher is the union of
both id tables again, and tests/vectors.rs replays the frozen vectors
straight through it.
- crates/wasm and crates/xtask join the workspace. cargo xtask bundle builds
the in-page bundle from browser-bundle/ plus the wasm core, writes
dist/, refreshes the tracked crates/live/assets/detect-antipatterns-
browser.js, and writes extension/detector/. bun run build:extension runs
it instead of downloading.
- crates/live/assets/detect-antipatterns-browser.js is tracked again; live
mode embeds it and serves it as /detect.js.
- Serde is back to plain derives: no is_human_readable branch in
js::json_number, derived Serialize for Rgba and BrowserFinding with their
skip_serializing_if attributes.
- profile.release has lto = "fat" again; rust-toolchain.toml is plain
stable plus the wasm32 target. The rust, rust-windows and oracle CI jobs
lose continue-on-error and can be required.
Verified
- cargo build --workspace --all-targets: clean, no warnings.
- cargo test --workspace: 346 pass, 0 fail (the 8 boundary tests are gone
with the boundary).
- cargo build -p impeccable-wasm --target wasm32-unknown-unknown --release: ok.
- cargo xtask bundle && cargo xtask bundle --check: reproducible; the
regenerated bundle is committed (it differs from the archived one, which
was built with a pinned rustc and lto = false).
- cargo build --release -p impeccable: no linker warnings, 12.5 MB (the
same source at lto = false is 13.1 MB).
- oracle: 795 pass, 0 fail, 0 accepted deltas, 0 missing goldens.
- bun run build, bun run build:extension, web-ext lint (0 errors,
8 warnings), bun run test: 363 + 80 + 1 + 1 + 133 + 180 + 4 pass, 0 fail.
- impeccable detect --no-config --json tests/fixtures/antipatterns: 128.7 ms
median of 5.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vau2X53xGTjjTCXWMVBoNY
Move Codex and .agents skill versions under metadata while keeping all version readers compatible with legacy top-level frontmatter.\n\nAI assistance: prepared with Codex under maintainer direction.
`bun run build:extension` was broken on this branch: it still imported the
deleted JS engine (cli/engine/registry/antipatterns.mjs,
scripts/lib/browser-detector-bundle.js).
The shipped shell now matches the new design. The content script only
snapshots the DOM; an extension-owned offscreen document runs the
WebAssembly rule core over that snapshot, so the scanned page's CSP no
longer matters. That replaces the old approach of injecting a JS rules
bundle into the page. New files: extension/offscreen/offscreen.html, plus
the "offscreen" permission and a 'wasm-unsafe-eval' extension_pages CSP in
the manifest.
The manifest version stays at 1.3.3. The shell's own manifest carried
2.0.0; feature branches never bump versions, so the bump is a release step.
The five generated detector pieces (core.js, core_bg.wasm, snapshot.js,
overlay.js, antipatterns.json) are vendored at build time into the
gitignored extension/detector/ by the new scripts/lib/detector-bundle.mjs,
which resolves them the same three ways crates/core/build.rs resolves the
native archive: IMPECCABLE_DETECTOR_LIB/extension-detector/, the
~/.impeccable/detector/<DETECTOR_VERSION>/ cache, then a checksum-verified
download of detector-browser-bundle.zip from the detector release.
antipatterns.json is no longer regenerated here.
The zip packaging is unchanged. The Firefox variant still builds so
`web-ext lint` keeps covering the shared shell, but it cannot scan: Gecko
has no chrome.offscreen API. The build prints a one-line warning saying so.
Also here: a referenced-path check that fails the build when the manifest
or the service worker points at a file that is not in extension/, a
resolver unit test wired into the core suite, and the detector rule count
in the READMEs synced to the 61 the vendored registry carries.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vau2X53xGTjjTCXWMVBoNY
Add a first-class OpenCode command bridge across builds, installs, updates, linked installs, and pinned shortcuts. Preserve current provider behavior while backfilling missing or drifted command files.\n\nAI assistance: contributor and maintainer work used AI tools as disclosed in the PR discussion and commits.
The engine no longer lives in a separate repo. `crates/` is a snapshot of the
open crates (foundation, core, common, context, live, hook, skills, comp,
comp-verbs, html, browser, detect, cli) plus `Cargo.lock`, taken as a git
archive of the engine repo at the commit that finished the boundary split.
None of that repo's history comes with it, and none of it should: the closed
half stays private.
The closed half is the rule engine. It ships as a prebuilt native archive per
target, `libimpeccable_detector.a`, published as a `detector-v<X>` GitHub
Release on this repo. `crates/core/build.rs` resolves and links it three ways:
`IMPECCABLE_DETECTOR_LIB=<dir>` for a local detector build, else the
`~/.impeccable/detector/<version>/<target>/` cache, else a download verified
against its `.sha256` sidecar. `crates/core` is a thin shim over a three-symbol
C ABI; nothing above it knows the boundary exists.
What changed versus the engine repo copy:
- Every crate manifest moves from `license-file.workspace` to
`license.workspace` (this workspace declares Apache-2.0), and the workspace
gains the `postcard` dependency the boundary encoding needs.
- The launcher contract test reads `skill/scripts/impeccable{,.cmd}` instead of
a sibling `launcher/` dir, and `engine_binary` downloads from
`github.com/pbakaus/impeccable/releases/download/engine-v<version>/` instead
of the retired dist repo. No oracle golden carried the old URL, so no
re-recording was owed.
- The tests that hunted for a public repo through `IMPECCABLE_PUBLIC_REPO`,
`../impeccable-second` or a hardcoded home directory now resolve the root as
`CARGO_MANIFEST_DIR/../..`, because they are in it. The env var stays as an
override for an out-of-tree checkout.
- The in-page bundle (`detect-antipatterns-browser.js`, 2 MB of generated wasm
glue) is no longer tracked. `crates/core/build.rs` resolves it beside the
archive, hands the path to `impeccable_core::browser::IN_PAGE_BUNDLE_JS`, and
live mode serves that. `scripts/check-detector-release.mjs` now requires it
and its `.sha256` in a detector release.
- The live crate embeds `skill/scripts/live-browser*.js` and
`modern-screenshot.umd.js` directly rather than through vendored copies, so
the binary and the installed skill cannot drift.
- `crates/browser/assets/` (an unused second copy of the bundle) is gone.
- `tests/lib/engine-bin.mjs` also accepts `target/release/impeccable`, so a
plain `cargo build --release -p impeccable` is enough to run `bun run test`.
Verified with the archive from a local detector build: `cargo test --workspace`
267 pass, oracle 795 pass / 0 fail / 0 missing, `bun run build` clean, the
default suite green, and the launcher's `engine-probe` handshake answering
through `skill/scripts/impeccable`.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vau2X53xGTjjTCXWMVBoNY
The engine binaries move from the impeccable-dist channel to this repo's own
GitHub Releases (tag engine-v<ENGINE_VERSION>), and the closed detector the
engine links arrives as detector-v<DETECTOR_VERSION> releases on the same
repo. This commit wires the public side for that; the crates themselves land
in the next commit.
- Launcher (sh + cmd), npm shim, fetch-engine and check-engine-release now
download from github.com/pbakaus/impeccable/releases/download/engine-v<X>/.
- release.mjs gains `engine`: verifies ENGINE_VERSION against the platform
package pins and the detector release, tags, pushes; release-engine.yml
builds the five targets and publishes. check-detector-release.mjs is the
matching release-order guard (with tests).
- Root Cargo.toml (workspace, lto = false with the reason), rust-toolchain.toml
(exact pin), DETECTOR_VERSION, /target ignored.
- CI: rust + rust-windows jobs and an oracle job that replays the goldens
against a source build, warn-only until the first detector release exists;
ci-test-plan exposes a `rust` output.
- docs/ENGINE.md (the crate map and the closed-detector mechanism) and the
CLAUDE.md engine, release-order and rules sections.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vau2X53xGTjjTCXWMVBoNY
The four comp-fidelity scripts (comp-spec, comp-diff, font-match, build-phase)
and their six libs are ported into the impeccable-engine binary. This removes
the last Node .mjs from the skill: `git ls-files skill/scripts | grep '\.mjs$'`
now returns nothing.
- reference/new-work.md, reference/visualize.md, and the asset-producer /
finish-reviewer agents now invoke `{{scripts_path}}/impeccable <verb>` instead
of `node <script>.mjs`.
- Deleted the ten ported .mjs and the four JS unit tests that imported them
(their behavior is now covered by the engine's Rust tests and the oracle);
removed those files from scripts/test-suites.mjs.
- Added oracle cases (comp-*, font-match-*, build-phase-*) over a comp-basic
workspace, recorded from the engine binary; the deterministic outputs are
byte-identical to the JS the scripts left behind.
- docs/CLI-CONTRACT.md documents the four verbs, the CDP font rendering, and
the runtime-resolved (never-committed) font-index catalog.
The font-index catalog JSON stays shipped in the skill (data/font-index.json);
the engine resolves it at run time and never vendors it.
Prepared with AI assistance (Claude Code).
Two suites could hang forever and never print a tally, because the one
mechanism that could interrupt the wedged work was missing on both paths.
Hang 1 (bun run test / build-phase.test.mjs): the test's run() helper
spawned every child with spawnSync and no timeout. spawnSync blocks the
test worker's thread, so node's --test-timeout (an event-loop timer)
cannot interrupt a child that wedges (a fork/exec blocked on OS resources
under concurrency, a gate's comp-diff grandchild, or a stray browser
launch). Bound every child with spawnSync timeout + killSignal SIGKILL so
a wedge becomes a fast, named failure the next test survives.
Hang 2 (bun run test:skill-behavior): runTurn called generateText with no
client-side deadline, so a stalled provider stream kept the fetch (and the
whole node process) alive past the per-test timeout, producing no tally.
Attach a real AbortSignal (default 840s, under the 900s per-test cap):
on expiry the fetch aborts, the turn throws, and the scenario
fails-and-continues. The unref'd timer is cleared on completion.
Runner backstops: run-tests.mjs now spawns each command as a detached
process-group leader and enforces a per-suite wall-clock cap that SIGKILLs
the entire group (workers, grandchildren, browsers) on expiry, with
SIGINT/SIGTERM forwarded so Ctrl-C still reaps the tree. The core node
batch gets a finite --test-timeout (180s); skill-behavior gets a 60min
group cap. Env overrides: IMPECCABLE_TEST_WALL_CLOCK_MS,
IMPECCABLE_SKILL_BEHAVIOR_TURN_TIMEOUT_MS,
IMPECCABLE_BUILD_PHASE_RUN_TIMEOUT_MS.
Proof: bun run test green twice (~60s); scoped claude-sonnet-5
skill-behavior sweep terminates with a tally (20 tests, ~32min) where the
840s abort caught a wedged redesign turn and the sweep continued instead
of hanging.
Prepared with AI assistance (Claude Code).
The launcher, npm shim, and `impeccable install` all resolve the engine
binary for the pinned ENGINE_VERSION, so a skill/CLI release or a rust-swap
merge published ahead of the engine release + platform packages dead-ends
every install path. Add a mechanical guard:
- scripts/check-engine-release.mjs: verifies all five dist binaries +
.sha256 and the five @impeccable/cli-<os>-<arch> npm platform packages
exist for the pinned ENGINE_VERSION; names missing assets, exits non-zero.
Honors IMPECCABLE_DOWNLOAD_BASE.
- release.mjs: hard-fails release:skill and release:cli when assets are
missing; extension is exempt (vendored WASM detector, no engine exec).
- CI engine-release-ready job: runs the check, continue-on-error with a
loud ::warning until the first engine release exists (flip to false then).
- CLAUDE.md Releases: documents the enforced ordering.
Prepared with AI assistance (Claude Code).
IMPECCABLE_BUNDLE_ENGINE=1 opts in to staging the engine binaries into the
dist skill copies. Bundling every target into every provider copy put
dist/universal.zip near 340 MB, past the 25 MB Cloudflare Pages file cap
that impeccable install downloads through.
Prepared with AI assistance (Claude Code).
Unit tests of the deleted Node scripts and the JS detector are removed;
their behavior is pinned by tests/oracle goldens (frozen JS behavior plus
reviewed deltas) and the engine's own tests. tests/oracle.test.mjs replays
the corpus against the binary (IMPECCABLE_BIN or skill/scripts/bin/<target>/,
via tests/lib/engine-bin.mjs) and skips cleanly without one; the framework
fixture sweep drives live-inject, live-wrap, and detect-csp through the
binary the same way. record.mjs learns --bin. The function-level vectors
under tests/oracle/vectors/calls are committed as the frozen snapshot they
can no longer be regenerated from. Suites: core trimmed to build and
transformer tests, oracle added to the default run, detector/live reduced to
packaging and reference checks, the live-e2e helper tests move to the opt-in
live-e2e lane pending its retarget, cli-remote-e2e is an empty placeholder.
Prepared with AI assistance (Claude Code).
readSourceFiles no longer copies cli/engine into the skill; the scripts
payload is the launcher (executable bit preserved through dist, plugin/, and
universal.zip), impeccable.cmd, VERSION (synced from ENGINE_VERSION on every
build), the page JS, and command-metadata.json. Hook manifests call
`<scripts>/impeccable hook` behind an existence guard (Codex adds a
commandWindows sibling calling impeccable.cmd; Cursor runs hook-before-edit;
GitHub keeps the git rev-parse form; Grok mirrors Claude); the Node probe and
systemMessage notice are gone. build:release fetches the pinned engine for
every target (lenient) and stages bin/<os-arch>/ into the dist skill copies
after root harness dirs and plugin/ were synced, so git-delivered trees stay
launcher-only. The detection-rule count check reads the vendored
extension/detector/antipatterns.json and is skipped when absent.
build:browser is a stub; the codex prefix rewrite leaves
`{{scripts_path}}/impeccable` alone.
Prepared with AI assistance (Claude Code).
skill/scripts keeps command-metadata.json and the page JS; every .mjs entry
point, lib/, and live/ are gone (the binary owns those verbs). Adds the POSIX
launcher, impeccable.cmd, VERSION (copied from the new root ENGINE_VERSION),
scripts/fetch-engine.mjs (bun run fetch:engine) to pull the pinned binary
into skill/scripts/bin/<os>-<arch>/, and gitignores that bin dir.
Prepared with AI assistance (Claude Code).
Makes plugin instructions execute the installed plugin copy, safely quotes paths containing spaces, removes the broad Node pre-approval, and ships guarded generated plugin output.\n\nAI-assisted maintainer conflict resolution, review, and validation by Codex under maintainer direction.
Resolves the embed helper with a filesystem path, reports fallback accurately, and adds regression coverage while leaving generated harness output to the post-merge sync.\n\nAI-assisted maintainer repair, review, and validation by Codex under maintainer direction.
Make policy and merge-conflict blockers age as contributor work, keep maintainer-policy decisions out of ready state, auto-close regular contributors unless explicitly exempted, and mark new or reopened issues for triage.
AI-assisted change: implemented and validated by Codex under maintainer direction.
The generated browser bundle is rebuilt from the merged engine sources in the next commit's build step (both branches had regenerated it).
AI-assisted (Claude Code).
Read waiver config from every live root (appRoot, contextRoot, repoRoot),
so monorepo projects whose config lives at the repo root reach the
overlay; serialize served roots and page identities repo-relative there.
Resolve each page URL to its actual serving file via the inject config's
resolved page list before applying file-scoped waivers; ambiguous URLs
keep the conservative common-ancestor fallback (PR #645 review
discussion r3840011436).
Honour detector.ignoreFiles: a wholly waived page now scans to zero
findings in the overlay, matching the CLI and the edit hook.
Guard the resolver call so a throwing resolver degrades to an unfiltered
scan instead of breaking the detect toggle.
Match design-system-color waivers by color value across hex and rgb()
spellings, and stop extracting font values for bounce-easing findings,
mirroring extractFindingIgnoreValue. Regenerate the browser bundle.
AI-assisted change: reviewed, planned, and implemented with Claude Code
under maintainer direction.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collapse the nightly alternate plan into the shared event routing while preserving every GitHub output and schedule behavior. Strengthen the nightly characterization for all deterministic suites.\n\nAI assistance: prepared by OpenAI Codex under maintainer pbakaus's standing scheduled-refactor authorization.
The live overlay's detect scan ran unfiltered: requestDetectScan() posted
only { scanId }, so detector.ignoreRules and detector.ignoreValues in
.impeccable/config.json reached impeccable detect and the edit hook but
never the surface a designer actually watches.
The server now serializes the project's detector waivers into the /live.js
prelude (window.__IMPECCABLE_PROJECT_IGNORES__), read per request through
hook-lib's readConfig so config.local.json wins and edits land on the next
tab reload. A new script part, live-browser-ignores.js, resolves that
config against the page URL when a scan starts: ignoreRules suppress
outright, wildcard ignoreValues suppress their rule in the files their
globs name, and the remaining entries ride along as disabledValues for the
detector to match on each finding's own value. The detector bundle applies
those where the findings are assembled, since the overlay draws its own
markers from the collected findings.
Scope resolution mirrors cli/lib/impeccable-config.mjs deliberately: the
same glob dialect (globToRegex, including {a,b} alternation), the same
path-suffix matching as findingMatchesScopedIgnoreFile, and the same
refusal to apply an unscoped wildcard entry. The served-root prefixes that
bridge project-relative globs and site-relative URLs come from the inject
config's own files globs, never from the ignore globs; deriving them from
the ignore globs lets one entry scoped to prototype/library/** lend its
prefix to every page and suppress site-wide, which looks like success
because the numbers go down.
Known gaps, recorded in the detector comment: the motion value extractor
is not mirrored, so a value-scoped bounce-easing waiver only matches when
the finding carries ignoreValue directly, and design-system-color matches
on the normalized string without the CLI's color-equality fallback.
Tests: unit tests for the resolver part (stale globals, string ignoreRules,
malformed entries, directory URLs, percent-escapes, glob metacharacters,
the roots trap), an extension-mode puppeteer test that disabledValues
suppress exactly the waived findings, and the live-browser regression pin
now asserts the new scan config shape instead of { scanId }.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
font-match / catalog index
- Index schema 2 adds a third render, 48c (48px cap, ALL CAPS text). Caps headline crops have no x-height band and ranked against mixed-case renders as barcode faces; they now route to the caps render.
- Non-text families (barcodes, redacted, flow, dingbats, effect faces) are excluded from candidates.
- The distance adds a gross width and weight gap (log ratio of advance and ink density) so a face 50% wider or 35% lighter cannot rank first on run-length detail alone; the index stores those readings.
- Multi-line all-caps crops vote on x-height across lines: one line's crossbars no longer give the crop a spurious x band.
- With no browser, --rank records the catalog's nearest face (source catalog, size estimated) so the spec gate can close; the NEXT line and new-work.md say not to install a browser or hand-write a choice.
- font-match stamps the choice it writes; the spec gate refuses a chosen face it did not write (sessions typed Arial Narrow into spec.json to pass).
- IMPECCABLE_NODE_MODULES lets a harness lend a playwright.
comp-spec
- A region note that describes painted material (diagram, drawing, photo, texture...) under a code kind is refused at the spec unless codeDrawn is set: the exploded carburetor filed as chrome is a plate about to be redrawn in SVG.
build-phase
- The control ink-box veto applies only when the comp's ink is a discrete element and the build's box is too; a full-width bar told one session six times that 1376x87 was 1382x102 with no edit able to move it.
AI-assisted (Claude Code).
lib/font-fingerprint.mjs replaces the three-number fingerprint with
size-invariant shape features (x-height ratio, stroke contrast, stem width,
run-length quantiles, roundness, serif signal, width spread) and a
noise-normalized distance; family recall on a held-out self-test rose from
13% to 72% top-5. data/font-index.json carries the whole Google Fonts
catalog (3,092 faces at two cap sizes, 707 KB); font-match --rank fingerprints
the comp crop, takes the 25 nearest faces from the index (plus the model's
own names), renders them at the comp's cap height, ranks by the same
distance, and prints a proof sheet and the CSS to use. scripts/build-font-
index.mjs rebuilds the index at release time.
AI-assisted (Claude).
Co-Authored-By: Claude <noreply@anthropic.com>
font-match.mjs fingerprints a text region's lettering in the comp (cap
height, glyph advance per cap = width class, ink fraction = weight class,
tracking), renders candidate faces at that cap height in a headless
browser (yours plus a shortlist for the width class), and ranks them by
distance with a proof sheet; the spec gate refuses to close until the lead
text region is measured and ranked. generate-image --plate keys ink-on-
ground plates to alpha (chroma) so the drawing sits on the page's own
ground; the plates gate scores keyed plates composited over the region's
ground. comp-diff reports each region's ink box; the hero gate names a
control whose box height or width differs from the comp. The hero
instruction copies the comp's words verbatim; rewording is a stated
decision after the hero passes.
Driven by a human review of the r3 side-by-sides: face width and weight,
plate ground, control row height, and content substitution.
AI-assisted (Claude).
Co-Authored-By: Claude <noreply@anthropic.com>
Dependency-free PNG codec, perceptual metrics (structure / color / detail /
bands), side-by-side + heatmap + per-region crops, a measured spec from the
approved comp (grid overlay, sampled palette, plate list), and a phase state
machine whose spec / plates / hero gates run the diff instead of asking the
model to remember the image.
AI-assisted (Claude).
Co-Authored-By: Claude <noreply@anthropic.com>
Grok was classified as GitHub Copilot, so the design hook skipped every
edit with no-file-path and never ran Stop. Normalize toolInput/sessionId
and treat Stop additionalContext as the Grok product.
Prepared with AI assistance.
Co-authored-by: Cursor <cursoragent@cursor.com>
Claude Code now folds multi-edit behavior into Edit, so keep generated and repaired hook manifests aligned with the current Edit and Write tools. Grok keeps its compatibility matcher unchanged.
AI assistance was used to implement and validate this change.
Centralize provider fixture, hook, and agent-launch contracts while preserving provider-specific verification behavior. Reuse the shared CLI argument parser and characterize the public usage contract.
AI-assisted: prepared by Codex under pbakaus's scheduled architecture-refactor authorization.
Centralize the shared Claude-compatible PostToolUse and Stop schema while preserving every provider-specific matcher, path, notice, and timeout.\n\nAI assistance: Codex prepared this behavior-preserving refactor under pbakaus's scheduled architecture-simplification authorization.
findDesignRoot stopped at the first package.json boundary, so every
design-system rule silently abstained for files inside monorepo
workspaces. The walk now continues past a workspace boundary to the
monorepo root that owns it, recognized the same way context.mjs does
(declared workspace globs, or a marker file beside apps/ or packages/
children). A nested repo with its own .git, a workspace-owned
DESIGN.md, and non-monorepo projects keep their existing behavior.
Written with AI assistance (Cursor); reviewed by maintainer.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Simplify local detector dispatch
Centralize HTML-versus-text file routing for stdin, directory, and direct-file scans. Add CLI characterization coverage for both stdin paths.
Prepared with AI assistance under maintainer pbakaus's standing scheduled-refactor authorization.
* Strengthen detector dispatch characterization
Put the HTML-only finding in a linked stylesheet so the text engine cannot satisfy the static-engine assertion.
Prepared with AI assistance under maintainer pbakaus's standing scheduled-refactor authorization.
* Take every themed list in an entry, not the first one
A long changelog entry is grouped into themed lists behind cf-group labels, and
the extractor stopped at the first one. skill-v4.0.0 shipped 6 of its 19
bullets that way, and v4.1.0 would have shipped 6 of 21.
This is the same shape as the bounded-search fix one commit earlier: the
extractor treated "found a list" as "found the notes". It now collects every
cf-items list inside the entry's own article and joins them, so grouping an
entry for readability cannot silently truncate its release notes.
Written with AI assistance (Claude Code).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Name the malformed case separately
An entry that opens a cf-items list and never closes it inside its article
matched nothing, and the failure said the entry had no list of its own. That is
a different repair, and the message sent you looking for the wrong thing.
Written with AI assistance (Claude Code).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Release: skill v4.1.0, CLI v3.6.0, extension v1.3.2
Skill 4.1.0: the build path becomes a recorded setting with a per-round
toggle, the direction round routes challengers by verdict, surface rounds deal
structure, and critique delivers its report and its close.
CLI 3.6.0: contrast findings stop assuming white when the ground cannot be
read, waivers scope to the element that carries them, and Hermes Agent and
Antigravity install natively.
Extension 1.3.2: no source change, but the bundled engine is rebuilt at
release, so the same 59 rules ship with the false-positive work behind them.
Chrome and Firefox from the one manifest.
Harness output regenerated with build:release, which is what the version
validator checks against the manifests.
Written with AI assistance (Claude Code).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Bound release-note extraction to the entry it names
Every v4.0.x skill release shipped v4.0.0's notes. The extractor took the
first `<ul class="cf-items">` after the version header with no upper bound, and
the v4.0.1 through v4.0.4 entries wrote their bullets in a `cf-entry-list`
instead, so the search ran past all four and landed in v4.0.0. Nothing failed,
because finding a list somewhere was treated as success.
The search now stops at the entry's own `</article>` and fails with the reason
when the entry has no readable list, which is the case the old code silently
published its way through. The changelog side is fixed in impeccable-site,
where those five entries now use `cf-items` like the other 46: `cf-entry-list`
also had no CSS at all, so their bullets were rendering unstyled on the
changelog page.
Written with AI assistance (Claude Code).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Live mode scaffolds each Svelte variant with a props script that annotated the
declaration:
/** @type {{ title: string; }} */
let { title } = $props();
A JSDoc `@type` written directly before a value is also JSDoc's cast syntax,
and esrap 2.3.3, the printer Svelte emits JS through, moves that annotation
onto the template's own declaration:
var /** @type {{ title: string; }} */ (h1) = root();
`var (h1) = ...` does not parse. The .svelte source is valid, the compile
succeeds, and the failure lands in the browser's dynamic import as "Unexpected
token '('": the variant never mounts and the session shows nothing. `@typedef`
carries the same shape without being a cast, so both builders emit that.
This is not test-only. Every Svelte variant we generate carried the construct,
so live mode was broken for any user whose install resolved esrap 2.3.3.
Svelte declares `esrap: ^2.2.12`, so a fresh install takes it; this repo's
lockfile pins 2.3.0, which is why unit tests stayed green while the fixture,
which installs into a temp dir, did not.
Two reasons the existing pre-publish guard could not have caught it, now
recorded next to it:
- `compileCheckVariants` compiles with `generate: false`, so there is no
emitted JS to inspect.
- `loadSvelteCompiler` resolves the compiler through createRequire, which
Svelte's export map routes to a prebuilt CJS build. A dev server imports
`src/compiler`, and only that path runs the app's installed printer. The
guard was checking a different compiler than the browser runs.
The new suite therefore imports the compiler as ESM and asserts the emitted
JavaScript parses, rather than pinning the comment style: a future printer that
mangles some other construct fails it too. The first draft used createRequire
and reported green against the exact input that breaks in a browser, which is
the mistake worth not repeating.
Verified against svelte 5.56.9 with esrap 2.3.3. Full live-e2e sweep green,
26 fixtures.
Written with AI assistance (Claude Code).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`initialized natural build` looked like a third defect on main: sonnet began
implementation before the attended concept checkpoint, three runs in a row.
It is flaky, not broken, and the measurement setup was the larger problem.
A run that stops to put the concept to the user before building takes about
579s on sonnet. A run that skips the checkpoint and fails the assertion
finishes in 130-200s. The suite capped each test at 300s, so the thorough path
was killed as a timeout and the hasty path was graded as a result: the cap was
selecting for the behavior the scenario exists to forbid. Raised to 900s, with
the reasoning recorded next to the number so it is not trimmed back as a
mystery constant.
The baseline is corrected accordingly: the scenario is flaky (1 of 4), not
failing, and readers are told to check a duration against the cap before
calling a slow failure a behavioral one.
Written with AI assistance (Claude Code).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review on #576 caught document.md:71 splicing {{ask_instruction}} after
"then", which is the same defect this branch set out to fix. Rendered for
Codex it produced "Show the user the existing file, then STOP and use Codex's
structured user-input/question tool...". The line now starts a new sentence.
The comment added to PROVIDER_PLACEHOLDERS asserted the contract without
enforcing it, which is exactly how four reference files shipped the splice in
the first place. validateAskInstructionSites() in build.js now checks every
call site and fails the build on a mid-sentence interpolation, and the comment
points at the gate instead of asking authors to remember.
Prepared with AI assistance (Claude Code).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The critique report and the AskUserQuestion call shipped in one assistant
message, so the report stayed hidden until the user answered the picker and
the command read as if it had never run.
Reorder critique's persistence steps so the temp-file cleanup runs after the
report and trend line are sent. That cleanup now ends the message carrying the
report, leaving the questions to open a fresh one. Both critique.md and
overdrive.md state the constraint and why it exists, so the ordering is not an
unexplained sequence a model can optimize away. Overdrive additionally moves
its direction descriptions inside the question options, where the user is
actually reading them.
Also fix the ask_instruction splices. The placeholder is a complete sentence,
but five call sites spliced it mid-sentence and shipped text like "stop and
STOP and call the AskUserQuestion tool to clarify. before expanding it". Every
call site is now sentence-initial and the twelve lowercase provider values are
capitalized to match, with a comment in utils.js pinning the contract.
Record a workflow-contract baseline for the current model lineup. The two
failures seen while validating this change are pre-existing: bolder refinement
fails on deepseek-v4-flash identically with bolder.md reverted to HEAD, and
redesign replaces DESIGN is flaky on assertions driven by new-work.md, which
this change does not touch.
Prepared with AI assistance (Claude Code).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Resolve {{scripts_path}} in the agent bodies Codex ships
Three code paths emit an agent body: the degraded fallback reference, the
.toml nested inside the skill for Codex, and the native agent file. Only the
nested .toml skipped placeholder substitution and rule-marker stripping, so
the codex and .agents dists shipped `node {{scripts_path}}/embed-prompt.mjs`
verbatim in the asset producer, and every caller had to substitute the token
itself at load time.
All three now render through renderAgentBody(), and the new regression test
asserts a runnable embed-prompt command on each emitted surface plus a
synthetic agent proving markers and placeholders resolve in the nested .toml.
Prepared by an AI agent (Claude Code) under pbakaus's instruction.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Give the finish reviewer's screenshots one fixed address
The Input Contract asked for "desktop and mobile screenshot paths captured by
the parent" and named none, so each session invented a filename and the
verdict pass went looking for a recapture that was never written there. Two
reviewer passes burned on that in the eval runs.
The parent now captures and recaptures to .impeccable/review/desktop.png and
.impeccable/review/mobile.png, and the reviewer reads those two first,
treating a brief-named path as the fallback for a parent that wrote elsewhere.
Prepared by an AI agent (Claude Code) under pbakaus's instruction.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Lead Setup with the base directory the runtime reports
The rendered claude and codex skills opened with
`node .claude/skills/impeccable/scripts/context.mjs`, a project-relative path
that resolves in this repo and in nothing a user installs: a personal or
plugin install puts the scripts outside the project entirely. The working form
was already in the text, parenthesized, after the one that fails.
Setup now leads with `node <skill-base-dir>/scripts/context.mjs` and says once
that the base directory resolves every scripts-path command in the skill and
its references, leaving the project-relative path as the fallback for runtimes
that report no base directory.
Prepared by an AI agent (Claude Code) under pbakaus's instruction.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Answer the Copilot review: brittle model assertion, missing review dir
Assert that {{model}} resolved rather than that it resolved to "GPT", which
belongs to PROVIDER_PLACEHOLDERS and can change without touching what the test
guards. And have the parent create .impeccable/review/ when the harness does
not, so a fresh project's first capture has somewhere to land.
Prepared by an AI agent (Claude Code) under pbakaus's instruction.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Make the review-screenshot contract directory-based, not web-viewport-named
Two amendments to the recapture contract from review feedback:
1. The canonical location is the directory .impeccable/review/, one file
per captured viewport; desktop.png and mobile.png are the web case,
not the contract. Baking web-viewport names into the reviewer's spec
would have hardened a web assumption into paths that a native
(ios/android/adaptive) build cannot honestly write.
2. Precedence restored to explicit-beats-convention: paths the calling
brief names are authoritative when the files exist; the canonical
directory is where the reviewer looks when the brief names none or a
named path is missing. This avoids stale canonical files from an
earlier run silently winning over fresh explicit paths. The observed
failure (the verdict round inventing a round-stamped filename) stays
fixed: recapture happens over the same files, and invented filenames
are still called out as pointing at nothing.
Assisted-by: Claude Code
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Paul BakausandClaude Code
4nibhalandGitHub
Abdul WahabandGitHub
oleg kovalandGitHub
