Repository navigation
Refuse stale Visimer installs up front and fix the thin-hook flake (#6186) - #44
Merged
Merged
Conversation
…6186) * Add the U-push-helper slice spec Six push-helper and Visimer defects from the gate-placement lane register: Visimer's push slot is not standalone, so an uninstalled or stale Visimer reads as a code defect (observed: class defect, exit 1); Visimer's round entry checks presence only; Visimer's PostCSS config search reaches the repository root; the thin hook's 5437 equality case reds on this host when its pipes stay at 512 bytes; the push reads changed paths through git's quoting; and chat-to-edit's check:fast push row starts no script. The spec records the evidence, the decisions, an acceptance check per item and the commit order. * Record the pipe probe behind the thin-hook acceptance checks The probe writes the frozen 5437 helper's --push-script-tests answer into a real pipe with only PIPE_BUF bytes free. Without a plugin manifest the scratch's answer is 578 bytes and arrives cut at byte 512, ending where every witness log ends; with a manifest it is 216 bytes and arrives whole. The spec's A4.1 now names the half the probe can show; the broken-pipe half at the hook's line 360 rests on the witness logs. * Refuse a push up front when Visimer's install is missing or stale Visimer has its own lockfile and workspace, but its push slot was not flagged standalone, so the up-front install refusal skipped it and the push dispatched its typecheck against whatever node_modules held. An uninstalled or stale Visimer then ended as a defect with a typecheck remedy instead of not-run with the install command. The slot is now standalone: a Visimer change with no node_modules, or with a node_modules/.pnpm/lock.yaml that differs from its pnpm-lock.yaml (including a present but empty node_modules), refuses before any slot runs, exit 77, naming pnpm --dir public/visimer install. A current install runs pnpm run typecheck from public/visimer. Selection is unchanged. Tests: new push cases for the missing, stale, empty and current installs, run red against the unchanged helper (each ended exit 0 with the typecheck dispatched) and green after. The push fixtures give Visimer an untracked node_modules, and the stub pnpm now records the directory each script ran in so assertions see a cwd dispatch. The parity test names Visimer's dispatch form as a change instead of re-recording; the standalone-set pin and the runbook entry name Visimer. Verified with node --test over check-pre-push, -plan, -mode, -contract, pre-push-thin-hook and gate-metrics tests. Story: US-001 * Refuse Visimer's round when its install is stale against its lockfile Visimer's check:agent entry used to refuse only when node_modules/.modules.yaml was missing, so an install made from an older pnpm-lock.yaml ran the round against dependencies the lockfile no longer names. The entry now applies the push helper's rule with its own copy of the comparison (the entry ships to the public mirror and cannot import root code): no node_modules is "not installed"; a pnpm-lock.yaml with a missing or byte-different node_modules/.pnpm/lock.yaml is "stale"; both print one NOT-RUN line naming `pnpm install` in Visimer's root and exit 77. OK Marketing's entry keeps its presence check, since its install is the root's and the root lockfile changes with any workspace. The round entry contract in scripts/check-agent.test.mjs gives each entry its own install shape and adds Visimer's stale cases (a differing .pnpm/lock.yaml, and a node_modules holding only a stray state file) plus a run when there is no lockfile to compare. Before the entry change the differing-lockfile case ran the script and exited 0; after it, all 38 round entry cases and the whole file (195 tests) pass. Visimer's entry on this worktree's real install runs `pnpm -r run test` and passes, and `pnpm -r typecheck` passes from public/visimer. The runbook's NOT-RUN catalogue states the new refusal. Story: US-002 * Pin Visimer's PostCSS config search at Visimer's root Vite 5.4 resolves a PostCSS config by walking up from each package root, so every Visimer package's search left Visimer and read the repository root. An empty config at public/visimer stops the search there; the mirror's public/visimer/** include ships it. Verified: with a root postcss.config.mjs that throws, pnpm run test in public/visimer/packages/core failed "Failed to load PostCSS config (searchPath: .../public/visimer/packages/core): planted: root PostCSS config was read", exit 1. With this file added and the plant kept, 7 test files and 84 tests passed, exit 0; the plant was then removed. From public/visimer, pnpm -r test (core 84, monaco 7, codemirror 6, dom 22) and pnpm -r typecheck pass. Story: US-003 * Give the thin hook's vintage scratches a plugin manifest makeScratch now writes .claude-plugin/marketplace.json into every vintage scratch, registering one plugin source the scratch's change does not touch, as the branch vintage 5437 models had. Without it the frozen 5437 helper's --push-script-tests answer carried the scratch path twice and ran past 512 bytes, so on a host whose new pipes stay at 512 bytes the answer was cut at exit and the frozen hook's printf | grep -q pipeline broke with a Broken pipe line that differed between the direct and the thin run. With the manifest the answer is a plugin skip line, names no host path and fits in one PIPE_BUF write. No frozen fixture, timeout, retry or assertion changes. Verified: pipe512-probe.py against kept 5437 scratches (record added beside it): before, 512 of 586 bytes arrived through a pipe with PIPE_BUF (512) free; after, 216 of 216. node --test scripts/pre-push-thin-hook.test.mjs passes 147/147 on this host, including all seven per-vintage equality cases and both 5582 version-skew cases. Story: US-004 * Read the push's changed paths unquoted and print control characters as JSON getChangedFiles now reads git diff --name-only -z and splits on NUL with no trimming, as the root round does, keeping --no-renames where it passed it. Before, git's quoting reached every row: a path with a byte of 0x80 or more or a control character arrived wrapped in quotes and escapes, so it matched no slot prefix, read edge or plugin root. A changed path is now printed through one renderer that leaves it as it is unless it holds a control character (U+0000 to U+001F, U+007F), which it prints as a JSON string: selection reasons, the root-verification and plugin triggers, the mirror leak-scan and changelog reasons, the extra-check reasons, and command text in the human plan. Verified: new cases in scripts/check-pre-push.test.mjs were red against the unchanged helper (an Open Knowledge source named qa071-café.ts selected no Open Knowledge slot; a qa071-café.sh fixture under a declared read edge answered root skip; tab-and-newline paths selected nothing) and pass now, with qa071-plain controls passing both times. node --test over check-pre-push, check-pre-push-plan (276/276) and check-pre-push-mode, check-pre-push-contract, pre-push-thin-hook (231/231). The rendering assertions' own red could not be produced: the scratch-mutation probe refuses this file. Story: US-005 * Stop dispatching chat-to-edit's check:fast, which typechecks nothing chat-to-edit's check:fast is pnpm -r --filter=./apps/** --if-present typecheck, and none of its apps defines typecheck, so the push row started no script while claiming a typecheck. Its slot now declares no typecheck (typecheckScript null, declaresNoTypecheck true), and the planner adds an owned-elsewhere typecheck row only for a slot that names an owner, so a slot with neither gets no typecheck row. This is the closure #6087's exception named in its reopen trigger, so the contract row slot.private-chat-to-edit.check-fast and its exception go, the pinned exception list is Discord's two rows again, and CHANGES.md records why. The SUBTREES wiring guard admits a declared absence and still fails an absent field, a null with no owner or declaration, a declaration that is not true and an empty name (new must-fire cases). The parity replay drops chat-to-edit's check:fast as a named change; expected.json is not re-recorded. Verified: a new plan test (a chat-to-edit change plans and runs only its format:check) was red against the unchanged helper on the check:fast row and green after. node --test over check-pre-push, -plan, -contract and -mode (362/362), check-pre-push-contract-homes (52/52), and the suites that copy the helper or contract into fixtures (nul-bytes, ok-changeset-length, hook-context, push-records: 249/249; thin hook and check-agent: 342/342); pnpm check:pre-push-contract validates 71 rows. Story: US-006 * Record the final helper's Visimer refusal on the real tree A temporary commit touching only public/visimer/README.md, checked by the helper with its parent as the base and push records off: with Visimer's node_modules moved aside the push ended not-run, exit 77, toolchain-missing, naming pnpm --dir public/visimer install and dispatching nothing; with the install back (lockfiles byte-equal) it ran pnpm run typecheck in public/visimer and passed. The temporary commit was dropped afterwards. The pipe-state sampler has seen only 65536-byte pipes so far, so the 512-byte-state run of the 5437 copies is still unobserved; the record says so and the sampler is re-read before the PR body. Story: US-007 * Correct the spec's account of an uninstalled Visimer at the push Run directly, the helper ends defect with a typecheck remedy. Through the hook's hand-off, QA observed the same red reported without blocking, because Visimer Validation runs on the change: the push is accepted as pass with the typecheck counted as a reported red, still naming no install. The typecheck run also leaves node_modules/.pnpm-task-run-state-v1 behind, the present-but-empty node_modules the earlier witnesses hit. The spec now states both readings and the side effect. * Pin the absent-root reason on standalone typecheck rows Two regression tests for QA-026. A standalone root absent from the checkout (Visimer via a sparse checkout, Discord never checked out) must skip its typecheck row with the same "is not present in this checkout" reason its scripts rows give, in both the push output and the --select-only JSON plan. Today the typecheck row falls through to "script not defined in <dir>/package.json", which is false: the root is absent and its package.json does define the script. These tests fail until the helper decides absence per slot. * Skip an absent standalone root's typecheck as absent QA-026: with public/visimer sparse-checked-out of the working tree and a commit touching it, the push skipped Visimer and passed but printed "public/visimer typecheck - script not defined in public/visimer/package.json, skipping", and the --select-only plan row carried the same reason. Visimer's package.json does define typecheck; the root was absent. The slot row builder decided absence inside the loop over a slot's scripts only. Visimer's slot has no scripts (its typecheck is its only row), and the typecheck row is built after that loop by a chain that never looked at absence, so an absent root's empty manifest fell into the "script not defined" branch. Discord had the same false typecheck line beside its correct absent-root lines for format:check and test. Absence is now decided once per slot and the typecheck chain skips an absent standalone root with the same reason and warning its scripts rows give, ahead of the undefined-script branch. --no-typecheck keeps its existing row; selection, dispatch, install refusal and selected operations are unchanged, so the plan-parity fixture still holds. Open Knowledge's typecheck row is owned by check:pre-push and never reached the false branch. Non-standalone root workspace members are outside SPEC D1's skip contract and are not changed here. * Pin that an unreadable standalone install refuses as not run An install probe that cannot read the install it checks must end the push, or Visimer's round, as an environment refusal: exit 77, one column-0 NOT-RUN line naming the path it could not read and a remedy, and at the push one not-run result marker. Today both probes rethrow every read error other than ENOENT, so the run crashes with a stack trace and exit 1, which the push records as a defect. The push cases run the real helper against each standalone slot that shares standaloneInstallProblem (Visimer, Open Knowledge, Discord). The entry cases run Visimer's real round entry. Each case covers five states: lock.yaml unreadable, pnpm-lock.yaml unreadable, node_modules unsearchable, lock.yaml a directory, and .pnpm a file. A current-install run first shows the fixture reaches the probe. The refusal must not call the install stale or missing, since reinstalling does not make an unreadable file readable. Permission cases skip as root, and every case restores the modes it changed. These tests fail until the probes classify unreadable installs. * Pin the refusal header and a mixed unreadable and stale refusal The 15 push cases for an unreadable standalone install now also require the single refusal header to name the read failure. Today's header says the root is not installed or its install is stale, which is false when the install is present but cannot be read, so a repair that keeps it fails. A disjunctive header that includes the read failure passes. A new case covers one refusal for two roots: Open Knowledge with a directory where node_modules/.pnpm/lock.yaml belongs, and Discord with a stale install, both selected by a root package.json change. It requires exit 77, no stack trace, no slot run, one header naming the read failure, each root named with its own cause, one NOT-RUN line whose why names Discord as stale and Open Knowledge as neither missing nor stale and whose remedy carries Discord's install command and addresses Open Knowledge, and one not-run marker whose reason code is set and is not toolchain-missing. Which code wins when unreadable and stale roots are refused together is left to the fix, since both name a true cause. Both fail on the current helper at the stack-trace assertion: the probe still throws before it renders anything. * Pin that an unreadable install's remedy names what to repair The refusal for an install the probe cannot read must say how to restore it (the runbook's NOT-RUN contract), and the install command alone does not: pnpm reports "Already up to date" and leaves a mode-000 node_modules/.pnpm/lock.yaml, a mode-000 pnpm-lock.yaml or a directory in place of lock.yaml exactly as they were. The unreadable-install tests accepted such a remedy, since they checked only that it was non-empty. Each push case now requires the NOT-RUN remedy to name the root's own pnpm-lock.yaml or node_modules, whichever the unreadable path lies under; the Visimer entry cases require the same relative to its root; the mixed refusal requires public/open-knowledge/node_modules. A remedy naming node_modules for an unreadable pnpm-lock.yaml fails too, since removing node_modules and reinstalling cannot clear that state. No wording or verb is pinned. Production is unchanged and these cases still fail first at their stack-trace assertion. * Pin that an unreadable install offers no install-only instruction The refusal for an install that cannot be read must not leave the install command standing alone in the instructions a person follows. The runbook's fix for this refusal sends them to the per-root lines, and the install command alone does not clear any unreadable shape. At the push, every stderr line that offers a root's install command must also name the root's repair object (its pnpm-lock.yaml or its node_modules), across the 15 slot and shape cases and the mixed refusal. At Visimer's round entry, every sentence that offers the install must name it too. The checks are label-free, so a fix may keep or change the "Install it:" label as long as the instruction names what to repair. The mixed refusal also requires the shared locator to find Discord's stale-root install offer, so a change to the printed install command reddens that test rather than leaving the loop checking nothing. * Refuse an unreadable standalone install as not run, not a crash The push helper's install probe and Visimer's round entry compared pnpm-lock.yaml with node_modules/.pnpm/lock.yaml and rethrew every read error other than ENOENT. An install the probe could not read (a mode-000 file, an unsearchable node_modules, a directory where the lockfile belongs, or a file where .pnpm belongs) escaped as an uncaught EACCES, EISDIR or ENOTDIR, and the push recorded it as a defect with exit 1. Each probe now classifies every outcome of its two reads. ENOENT keeps its meaning, and any other error becomes an unreadable verdict that carries the path the probe tried and the error code. The push helper renders it as exit 77 with one NOT-RUN line, the reason code toolchain-unreadable, and a "Fix it:" line that names what to repair: "make <dir>/pnpm-lock.yaml readable" for the lockfile, otherwise "make <dir>/node_modules readable (chmod -R u+rwX <dir>/node_modules), or remove <dir>/node_modules and run <install>". The install command alone does not clear these states, so "Install it:" is printed only under a missing or stale root. The probe serves the Open Knowledge, Visimer and Discord slots. Visimer's entry gets the same rule with its own prose line and exit 77. Missing and stale refusals print the same bytes as before. The runbook catalogues toolchain-unreadable, the precedence for mixed refusals, and the Fix step for an unreadable root. * Pin that a stale refusal names whether the installed lockfile is missing or differs The push helper's stale refusal prints "(its node_modules/.pnpm/lock.yaml differs)" for every stale install, including the present-but-empty node_modules the Visimer witnesses hit and Discord's no-recorded-state shape, where that file does not exist. Visimer's round entry already names each cause. The no-recorded-state and empty-node_modules cases now expect "is missing", and the lockfile-mismatch case expects "differs". Run against the current helper, the two missing-cause cases fail on the message and the mismatch case passes. Story: phase 8 do-now (catch-slop S1) * Name the cause in the push helper's stale-install refusal A stale verdict now records whether node_modules/.pnpm/lock.yaml is missing or differs from pnpm-lock.yaml, and the refusal line names it, as Visimer's round entry already does. Before, every stale install read "(its node_modules/.pnpm/lock.yaml differs)", including the present-but-empty node_modules the Visimer witnesses hit, where that file does not exist. The class, reason code, NOT-RUN line and install remedy are unchanged, and pr-smart's hook classifier keys on "node_modules is stale", which stays. The pinning cases from the previous commit go from red to green; the whole check-pre-push.test.mjs passes 240/240. Story: phase 8 do-now (catch-slop S1) * Cover the DEL arm of the push's control-character path render The render quotes a changed path holding U+0000 to U+001F or U+007F, but the suite's only control-character name exercised the first range. A new case pushes a Visimer path holding U+007F and expects the plan's selection reason to print it as one JSON string. Not verified: that this assertion would fail if the U+007F arm were dropped. The only route to that red is a scratch mutation of scripts/check-pre-push.mjs, which the repository's probe script refuses for this file, and that refusal binds every route. Story: phase 8 do-now (no-skimp, renderPath U+007F) * Drop a stale comment from the standalone-flag wiring test The comment described a runScript() dispatcher and a node_modules-present skip guard that the push helper no longer has: a selected standalone root without a current install is refused up front, and runScript() was removed. This branch had extended the comment to name Visimer. The assertion messages and the helper's own SUBTREES header state the contract. Story: phase 8 do-now (catch-slop S2) * Bring the spec in line with what QA and the follow-up pass changed D1 now says the anchor's absent-root skip covered only a slot's scripts rows, and that the branch decides absence once per slot (QA-026). D5 records the declaresNoTypecheck field the wiring test needs. A new D6 records the two outcomes added late: an install the probe cannot read is refused as not run with toolchain-unreadable (QA-064), and the stale refusal names whether the installed lockfile is missing or differs. C1 describes the default Visimer fixture as built, with acceptance rows for the absent and unreadable cases; C5 lists the U+007F case. The Open Knowledge executor is recorded as compliant for missing and stale installs but not unreadable ones, and section 6 carries that and the red proofs the probe refusal leaves unrun. Story: phase 8 do-now (spec deviations) * Escape C1, separator and bidi characters in push path output Reading changed paths with -z dropped git's octal quoting of bytes 0x80 and up, and the renderer escaped only C0 controls. A path holding a C1 control such as CSI, a line or paragraph separator or a bidi override now printed raw in selection reasons and command text. The renderer now prints such a path as a JSON string with each of those characters (and DEL) as a \u escape. * Check chat-to-edit's no-typecheck declaration against its manifests declaresNoTypecheck was read only by the wiring test, so an app under private/chat-to-edit that later added a typecheck script would be skipped at the push with every check green. The wiring test now fails the declaration while any package.json under the slot, other than its root manifest, defines typecheck. * Bring the push-helper spec in line with the renderer and declaration checks D4 now names the characters the renderer escapes and records that the round's renderers still print them raw. D5 and C6 describe the declaration check, and the status line states what is open at merge. * Escape the root all and root note path lines the push prints The root all line and the root note lines in the push's script-test answer quoted their path with plain JSON.stringify, so a changed root test or script named with a C1, separator or bidi character reached push stderr raw inside the quotes. Both now go through the same escaping as every other printed path, including the notes root-test selection composes. The escaped set is now written once: the quoting decision and the escaping inside the quotes read the same pattern, so they cannot drift apart. * List chat-to-edit manifests from the repository root, NUL-delimited The declaration test listed manifests with git ls-files from the process cwd and split on newlines, so it failed from any directory but the root and would drop a path git quotes. It now pins cwd to the repository root, reads with -z and resolves each read against the root, as the sibling root tests do. * Say where the chat-to-edit declaration is checked and how root lines render D5 claimed an app that adds typecheck could not pass the push unnoticed, but no read edge selects the declaration test for a chat-to-edit manifest change, so it runs only in CI. D4 and section 6 now describe the single escaped set and the escaped root lines, and section 6 records the missing read edge for the lead. * Escape root run reasons and the selection load error the push prints The root run line carried root-test selection's reason as composed, with plain JSON.stringify, so a read-edge match on a changed path holding a bidi or C1 character printed it raw. The root all line for a selection that fails to load did the same with the error message. Both now go through escapeUnsafe. The test fixture names that held raw U+202E and U+2028 bytes are spelled with \u escapes. * Say the push escapes selection reasons and load errors too * Escape the plugin load error and unknown-scope text the push prints The plugin trigger's load error quotes the marketplace manifest's JSON parse message, and the unknown-scope reason quotes git's error, so both can carry a bidi or C1 character from file content or a ref name. Both now go through escapeUnsafe like the root all load error. * Say the push escapes error text on plugin run lines too GitOrigin-RevId: b2651aa899ee0041ff2375e0a96dc82d3884c339
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Copybara-translated 1 Inkeep OSS change. Rebase-merge this PR so the prepared commit lands directly on public main.