Skip to content

release: ship vinci/worker in the tarball; packaged-artifact check verifies launcher dispatch targets - #48

Closed
thegeorgepu wants to merge 25 commits into
mainfrom
fix/package-ships-worker
Closed

release: ship vinci/worker in the tarball; packaged-artifact check verifies launcher dispatch targets#48
thegeorgepu wants to merge 25 commits into
mainfrom
fix/package-ships-worker

Conversation

@thegeorgepu

@thegeorgepu thegeorgepu commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

What

vinci worker is dispatched by the shipped launcher, but vinci/package.sh did not include vinci/worker. Every tarball through 0.0.51 therefore shipped a dead worker command even though checkout-based production workers remained healthy.

This PR now:

  • ships vinci/worker while excluding its README;
  • ships an explicit vinci/dispatch-manifest.json binding every launcher dispatch;
  • derives each allowed launcher dispatch from the manifest's exact canonical shell form and structurally tokenizes shell commands, so quote-concatenated e""xec, direct node "$VINCI/...", alternate/absolute Node spellings, and partial conversions cannot escape discovery;
  • parses executable module syntax with the TypeScript parser rather than source regexes, covering compact ESM syntax, side-effect imports, exports, static CommonJS require, and recursive .cjs dependencies;
  • rejects malformed syntax and dynamic/computed/aliased runtime loaders because they cannot be proven by the static artifact check;
  • validates literal path spelling, containment, identity, and symlink safety against the unpacked tarball;
  • never executes packaged target code during structural verification.

The existing CI and release workflows already build and unpack the real tarball before invoking this check. The bounded packaged --version probe remains a separate positive product control.

Exact head

5145860e1f974b7151a0edd18bbfe24fb3d0c5ba on base 0f7404d77fde08a2d8fa2511f9bfd2a213f1af60 (refreshed 2026-09-02 by lane ws-c-worker under ruling msg_72d4330e section C / assignment msg_09be7393; the previous body named 0a6d9c9b1651fa9bcf078dc8695e8c5bb1e613fe, which is fifteen heads behind).

Head history (base 0f7404d7; each head and the bus envelope that answered it)

  1. 9baa4fa4 — initial PR head (lane claude-vcc-rc-review): ship vinci/worker, checker sees launcher dispatch targets. No envelope; superseded.
  2. efc9ebde — builder repair (status msg_e9d3c8ba). No envelope; superseded.
  3. d97beb3b — builder repair (status msg_9ae3efa5). No envelope; superseded.
  4. 0a6d9c9b — the head this body previously named. No envelope; superseded.
  5. ea3858fd — BLOCK msg_0ce248c0 (hidden dispatches still certified).
  6. bb9d344c — BLOCK msg_ea333bfc (six reachable hidden dispatches; supersedes msg_0ce248c0).
  7. 45412cec — BLOCK msg_dc7a8210 (round 1 under the proof budget: eight reachable hidden execution paths — indirect shell executables/PATH, Reflect/vm/worker_threads/child_process).
  8. a2d7d9bc — BLOCK msg_544ebcd7 (round 2: missing bare dependency escapes to parent node_modules).
  9. 9c55ff22 — BLOCK msg_b53d2f94 (round 3: tsx ships while its required esbuild is excluded; malicious parent esbuild loads after certification) and BLOCK(decision-gated) msg_a1d3d766.
  10. cd4d97b6 — coordinator hold msg_d5b6821f (excluded-but-present authority-identical packages certify; declared conditional-export bare imports not traversed; authority not bound to repo/commit/tree).
  11. ff027c6b2ad897fc71ccb43ebf2b2ccff2061f7b1a4bde23153245f6b6f9995d362fbbd4d86755d4d6089f0fe821d8245145860e (current) — unreviewed descendants repairing the msg_d5b6821f findings. Round 4 (the ONE remaining round under ruling msg_72d4330e) requested by ws-c-worker (msg_da11676f) → BLOCK msg_6322c5f8 (ws-e-review, 2026-09-02T22:05Z), classified CLOSED-CLASS by the reviewer — not a split trigger; a defect in the repair, to be fixed on a normal descendant of 5145860e with one confirming exact-head review (within budget). A fifth round on the SAME head is out of budget.

Per the ruling: a BLOCK on a NEW mechanism class at this head triggers the SPLIT (worker-inclusion fix + existing checker as a small PR; the package-authority binding becomes its own work order). A BLOCK on an already-closed class is a defect in the repair, fixed on a normal descendant.

Round 4 finding (msg_6322c5f8) — descendant-fix spec

  • Mechanism (P1): vinci/test/packaged-artifact-check.mjs:1161-1163 isRuntimeWorkspace = /^packages\/(agent|ai|coding-agent|orchestrator|tui)\/package\.json$/ gates every packageEntryFiles.add (imports 1197-1201, wildcard 1222-1228, main/bin/exports 1257-1263); packageEntryFiles is the only set passed to checkGraph({bindBare:true}) (1268). Every non-workspace shipped package therefore has its main/bin/imports/exports existence-checked but never bare-import-bound. runtime-package-closure.mjs does not save it: it ships declared deps/optional/peer only and skips unresolvable optionals; peerDependenciesMeta is never iterated.
  • Runtime-positive reproduction (real tarball at this head): hostile parent node_modules/{supports-color,bufferutil,utf-8-validate} are loaded by shipped debug/src/node.js:32 (via http(s)-proxy-agent, declared by packages/ai) and shipped ws (via @google/genai, @mistralai/mistralai); the checker still exits 0 printing "13720 required entries close parent-directory dependency resolution". 59 shipped package names have bare imports that do not resolve inside the payload; live-runtime ones: supports-color, bufferutil, utf-8-validate, @modelcontextprotocol/sdk.
  • Old/new discrimination: setting the gate to true flips the clean artifact from exit 0 to exit 1 with "8119 unverifiable dependency edge(s)" naming the three instances — the binding machinery works; the gate suppresses it. The gate arrived in 71ccb43e ("scope entry traversal to shipped workspaces"), which ALSO retargeted the test "every declared package export binds bare imports to the artifact" from a non-workspace fixture to a workspace one (79/79 green over the gap).
  • Required repair (descendant of 5145860): bind bare imports for EVERY shipped package's entry surface (main/bin/imports/exports, plain/conditional/wildcard), with a principled treatment of optional edges (peerDependenciesMeta.optional, optionalDependencies, try/catch-guarded requires) rather than deleting the gate outright; add the three hostile-parent runtime regressions (supports-color via debug, bufferutil + utf-8-validate via ws) as checker-must-refuse cases; restore the retargeted test to a NON-workspace fixture; keep the positive control (clean artifact certifies, vinci --version, vinci worker usage path).
  • Sequencing: per section C, the package-authority binding must not hold the worker; promotion of draft PR worker: include vinci/worker in the packaged release (split from #48) #50 (worker inclusion only, cf196e63, CI 6/6) is a coordinator/Governor decision recorded on the bus.

Installed-artifact evidence at 5145860e (lane ws-c-worker, 2026-09-02)

  • bash vinci/package.sh at this head → vinci-code-0.0.51.tgz sha256 ef11dcb1bb6b1280d5d610ef6cb8509e09dfe95769493bd1d59de5225034c328; tar -tzf lists 21 entries under vinci/worker/ (worker.mjs, run.mjs, bus.mjs, outbox.mjs, contracts/…).
  • packaged-artifact-check.mjs and packaged-runtime-probe.mjs on the unpacked artifact: both pass ("certified package, CLI help, direct CLI, worker, metadata, symlinks, and parent refusal passed").
  • Install through vinci/install.sh + updater into an isolated VINCI_HOME (probe keypair; the only payload byte changed is vinci/updater/public-key.pem, exactly as update-integration.mjs does): vinci --version0.0.51; vinci worker --help → rc=1 vinci worker: Usage: vinci worker start --id <id> --server <url> … (the worker's own usage path).
  • Old-defect control at tag vinci-v0.0.51 (c2078cd), same probe: tarball sha256 8193e44f… has 0 vinci/worker entries; installed copy has no vinci/worker; vinci worker --help → rc=1 node:internal/modules/cjs/loader … throw err (module not found). The repair discriminates.

loop_edge

loop_edge: package | live_producer: release workflow (vinci-release.yml; NOTE: its repository guard names getsimpledirect/vinci-code, so it is SKIPPED in this repository — see the canary plan) | live_consumer: updater channel beta | side_effect_adapter: none | authority_class: ordinary (package-authority binding portion: supply-chain boundary, may go deep) | observation_receipt: required (install probe) | human_required: true (release)

Intended-mechanism discrimination

  • Positive real-tarball control: 3 manifest-bound dispatches, 20 dispatch files / 33 imports, and 63 extension files / 100 imports resolve inside the unpacked artifact.
  • Old behavior restoration: removing vinci/worker fails at the worker target assertion.
  • Transitive fault restoration: removing a dependency reached only through imported .cjs fails at the named CommonJS edge.
  • Discovery mutants: zero entries, partial conversion, $VINCI, absolute Node, env node, quote-split e""xec, direct Node without exec, alternate shell runners, nested command substitutions/backticks, variable runtime, and extra unmarked execs all fail at the dispatch-manifest mechanism.
  • Parser mutants: import{missing}from"./absent.mjs", computed require, aliased require, module["require"], eval, malformed syntax, and missing recursively imported .cjs are covered.
  • Input boundaries: missing, null, empty, malformed, wrong-type, unsafe, wrong-case, symlinked, wrong-repository, and unrelated-CWD cases are covered.
  • Safety control: a packaged module with a top-level write passes structural analysis without executing; the marker remains absent.

Verification

  • node --test vinci/test/packaged-artifact-check.test.mjs — 20/20 passed.
  • Fresh network-free build + unsigned package + unpack + artifact check + exact version probe — passed.
  • npm run check after a fresh network-free build — all checks passed.
  • VINCI_SKIP_SMOKE=1 bash vinci/test/run.sh — every offline/unit/UI/visual group passed; no provider call.
  • ./test.sh — agent and AI suites passed; coding-agent had one unrelated reftable debounce timeout, then test/footer-data-provider.test.ts passed 8/8 on focused rerun; TUI completed. This is recorded as a flaky full-workspace run, not claimed green.

Hosted exact-head checks are running and are not claimed here until terminal.

Scope

No worker deployment, box/fleet mutation, release publication, provider/model call, or GPU work.

Round 4 repair — descendant of 5145860e (lane ws-c2-pr48-fix, 2026-09-02)

New exact head acaaae667a847d8e9c2fb2767ea522eec78b8dcc on base 0f7404d77fde08a2d8fa2511f9bfd2a213f1af60 (merge-base with the base is exactly 0f7404d7). Two normal descendant commits, no force-push:

  1. aaa3f97fthe round-4 repair: bind bare imports for every shipped package entry surface.
  2. acaaae66separate, unrelated finding routed by the coordinator (projects-9f via projects-5a): first-party test paths are not excluded from the release archive.

Files changed: vinci/test/packaged-artifact-check.mjs, vinci/test/packaged-artifact-check.test.mjs, vinci/scripts/package-entries.mjs, vinci/scripts/first-party-test-paths.mjs (new).

Item 1 — the closed-class round-4 finding (msg_6322c5f8)

The isRuntimeWorkspace gate at packaged-artifact-check.mjs:1161 is removed, so every shipped package's main/bin/imports/exports (plain, conditional, wildcard) is traversed with checkGraph({bindBare:true}). The gate is not merely deleted — the reviewer's G8 note that "delete the gate is NOT a safe repair" is honoured by a principled optional-edge policy:

  • An edge that resolves above the artifact root is refused no matter how the package describes it. That is the hostile-parent case.
  • An edge that resolves nowhere is tolerated only when the importing package declares it optional, by manifest (optionalDependencies, peerDependenciesMeta.optional) or by guarding the load in try/catch (the napi-rs platform loaders name 17 platform packages they never declare). Undeclared, unguarded and absent is a missing edge and refuses.
  • Export targets under conditions a plain node process never evaluates (source, @zod/source, types, browser) stay existence-checked but are not traversed; Node-default conditions are. The launcher passes no --conditions.
  • Files reached through a bare edge are now traversed too, so deep subpaths of legacy packages bind.
  • Bare resolutions are batched one child per file and cached. Binding every package (not 5 workspaces) made the per-edge resolver spawn dominant: the fixture suite went 41s → 138s and timed out unrelated tests; batching brings it to 48s, and the real-artifact check to 31s (faster than the 35s pre-fix baseline despite 12.6× more work).

Triage of the 8119 edges the reviewer's mutation surfaced, on the real tarball: 8067 relative (7859 of them @mistralai/mistralai TypeScript source reachable only under the non-Node source condition, 180 zod, 27 @mariozechner/clipboard platform binaries) and 52 bare — 16 optionalDependencies, 3 peerDependenciesMeta.optional, 33 undeclared napi-rs platform packages loaded inside try/catch. Only the three live-runtime ones are undeclared and unguarded.

Runtime hazard first (the artifact is real): bash vinci/package.sh at 5145860evinci-code-0.0.51.tgz sha256 1284d875e7fae56c61513af430abfb3d70684549e7c603b7c40b664f0e972d80, 21 vinci/worker/ entries. Unpacked under a hostile parent holding node_modules/{supports-color,bufferutil,utf-8-validate} that write a marker on load:

--- runtime hazard proof
node exit=0
HOSTILE_PARENT_supports-color_LOADED
HOSTILE_PARENT_bufferutil_LOADED
--- clean payload control
node exit=0
(no marker: clean parent loads nothing)

(2) Old-behaviour control — the checker at the pre-fix head 5145860e, on that same hijacked payload, certifies:

  ✓ packaged artifact: 2 manifest-driven launcher dispatches; 22 dispatch files/33 imports and 63 extension files/100 imports resolve inside the tarball; 376 package entry files/1476 imports bind inside the tarball; 13715 files/5 links match the closed executable authority; 13720 required entries close parent-directory dependency resolution
exit=0

(1) Negative control — the same probe through the same entry point at acaaae66 refuses, in the checker's own message, naming each file and specifier:

✗ packaged artifact: package entry graph has 3 unverifiable dependency edge(s)
    node_modules/debug/src/node.js -> supports-color resolves outside the artifact root
    node_modules/ws/lib/validation.js -> utf-8-validate resolves outside the artifact root
    node_modules/ws/lib/buffer-util.js -> bufferutil resolves outside the artifact root
hostile exit=1

This is the package-entry-graph mechanism itself, not an earlier guard: no authority, layout or provenance failure appears, and the assert.doesNotMatch(hijacked.stderr, /closed executable authority|required by the trusted package layout/) control in the new test pins that.

(3) Positive control — the unmodified artifact from bash vinci/package.sh at acaaae66 still certifies:

  ✓ packaged artifact: 2 manifest-driven launcher dispatches; 22 dispatch files/33 imports and 63 extension files/100 imports resolve inside the tarball; 4744 package entry files/16562 imports bind inside the tarball (79 declared-optional edge(s) absent everywhere); 13715 files/5 links match the closed executable authority; 13720 required entries close parent-directory dependency resolution
clean exit=0

376 → 4744 package entry files and 1476 → 16562 bound imports is the gate's suppression, measured. packaged-runtime-probe.mjs on that artifact: packaged-runtime-probe: certified package, CLI help, direct CLI, worker, metadata, symlinks, and parent refusal passed. Installed through vinci/install.sh + updater into an isolated VINCI_HOME (probe keypair; only vinci/updater/public-key.pem differs, exactly as update-integration.mjs does):

vinci --version -> rc=0 stdout="0.0.51"
vinci worker --help -> rc=1 "vinci worker: Usage: vinci worker start --id <id> --server <url> [--once] [--poll-seconds 60] ..."
installed-artifact probe passed for 0.0.51 (worker dir present: 81838 bytes)

(4) The retargeted test is restored. every declared package export binds bare imports to the artifact goes back to a NON-workspace fixture (node_modules/runtime, through a conditional export), which is what 71ccb43e changed away from while keeping the name. Its docstring names the mutation:

re-gating every packageEntryFiles.add in validatePackageManifest on const isRuntimeWorkspace = /^packages\/(?:agent|ai|coding-agent|orchestrator|tui)\/package\.json$/.test(relativeManifest) (commit 71ccb43e)

Added alongside it: the three hostile-parent regressions as checker-must-refuse cases (with the runtime load proven by marker first, and the clean state certifying with 3 declared-optional edges absent); undeclared/unguarded absent refuses while manifest-declared and try-guarded certify; a load in a catch clause is not guarded by that try; non-workspace wildcard exports, require, dynamic import(), bare-edge traversal, non-Node conditions, and directory imports.

(5) G7 alternate paths. require, dynamic import() and exports subpath patterns (./*) in non-workspace packages are covered, each with its own test. Explicitly out of scope, with reasons, in the commit message: (a) an optional edge that resolves nowhere at verification time but is supplied by a parent node_modules at install time cannot be observed by a static check that runs before installation — closing it needs sealing stubs generated at package time, which is the package-authority binding work order the ruling keeps separate; (b) the dispatch and extension graphs still run bindBare:false, their bare imports being declared workspace dependencies whose presence the trusted layout closure already requires.

Item 2 — first-party test paths in the release archive (separate commit acaaae66)

vinci/package.sh:30 promises "tests, docs, infrastructure state, and release tooling must never enter the public archive", but the only test-shaped exclusion was a hardcoded node_modules/ssh2/test, and package-entries.mjs tars first-party release roots wholesale.

Latent, not live. No first-party test path ships today: tar -tzf of the artifact at this head matches no test-shaped first-party path, and no such file exists on disk under any release root (vinci/{bin,extensions,themes,assets,updater,worker}, packages/*/dist). PR #49's vinci/worker/test/** would have been the first.

Negative control, with vinci/worker/test/ws-c2-probe.test.mjs and vinci/updater/__tests__/ws-c2-probe.test.mjs planted and bash vinci/package.sh run — before:

=== BEFORE FIX: probe paths in the tarball listing
vinci/updater/__tests__/
vinci/updater/__tests__/ws-c2-probe.test.mjs
vinci/worker/test/
vinci/worker/test/ws-c2-probe.test.mjs
count:        4

after (same probe files, same command):

=== AFTER FIX: probe paths in the tarball listing
count:        0
=== positive: shipped runtime files under the same dirs still present
21
vinci/updater/public-key.pem
vinci/updater/update.mjs
vinci/worker/worker.mjs

Fixed as a class, not per path: vinci/scripts/first-party-test-paths.mjs keys off path shape (test/tests/__tests__/__mocks__/spec/specs segments, *.test.*/*.spec.* basenames), first-party only — node_modules stays governed by the production dependency closure, since pruning a dependency by name would change which bytes of a third-party package the artifact carries. The rule is applied on both sides from that one predicate, which is not optional: the checker requires every authority entry under vinci/worker, so a producer-only exclusion would make every artifact fail as "required by the trusted package layout is missing". A test path that does reach the archive is refused in the checker's own words.

Mutation lines, one per guard (they are different code paths and one mutation does not fail both):

no first-party test path enters the packaged entry list — in package-entries.mjs excluded(), replace if (isFirstPartyTestPath(relativePath)) return true; with if (relativePath === "vinci/worker/test") return true;
a first-party test path that reaches the artifact refuses by name — in packaged-artifact-check.mjs compareAuthorityDirectory(), replace if (isFirstPartyTestPath(relativePath)) { with if (relativePath === "vinci/worker/test") {

Both verified: each test passes at this head and fails under its own mutation. Mutating only excludedFromReleaseAuthority does not fail the second test, which is why both guards are pinned separately rather than assumed to share one.

(6) Tests, verbatim

node --test vinci/test/packaged-artifact-check.test.mjs at acaaae66 (87 tests, up from 79 — 8 added):

ℹ tests 87
ℹ suites 0
ℹ pass 87
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 47574.435625
unit exit=0

bash vinci/package.sh at this head: package exit=0. Pre-commit npm run check (biome, pinned-deps, secrets, ts-imports, shrinkwrap, install-lock, tsgo --noEmit, extensions, browser-smoke) passed on both commits. Hosted checks at this head are not claimed here until terminal.

Scope and honest limits

No merge, release, deploy, tag or branch-protection change; PR #50 untouched. Not re-verified by me and taken as recorded: ws-c-worker's ef11dcb1… tarball digest and the vinci-v0.0.51 zero-worker control. macOS only; no Linux or Windows run. The tarball is not byte-reproducible across builds (three builds gave three digests), so each control above names the digest it ran against. The review's fourth unresolved name, @modelcontextprotocol/sdk, is a peerDependenciesMeta.optional of @google/genai and is therefore tolerated-while-absent by the policy above, not bound — it is refused the moment it resolves in a parent.

…fact check see launcher dispatch targets

vinci/bin/vinci hands `vinci worker` to `node "${VINCI}/worker/worker.mjs"`, but package.sh
assembles the tarball from an explicit path list that never named vinci/worker. Every 0.0.x
tarball up to 0.0.51 therefore shipped `vinci worker` as a dead subcommand: the launcher execs
a file that is not there and node dies with ERR_MODULE_NOT_FOUND. Nothing caught it because
packaged-artifact-check.mjs followed imports only under vinci/extensions, and the production
workers run from a git checkout, where the path resolves.

Two changes:
- package.sh adds vinci/worker (README excluded, like every other doc).
- packaged-artifact-check.mjs reads the shipped launcher for `exec node "${VINCI}/<path>"`
  dispatch targets, requires each to exist, follows their relative imports with the same
  resolver the extension layer gets, and then DRIVES `vinci worker` from the unpacked tree,
  requiring its own usage refusal (exit 1, "vinci worker: Usage:") so the whole module graph
  is proven to load from the artifact rather than the repo. Zero matched dispatch targets is a
  refusal, not a pass: a check that finds nothing has gone blind.

Discrimination, run against tarballs built locally at 0f7404d and at this head:
  new check vs tarball built with OLD package.sh -> FAIL: vinci/worker/worker.mjs not in tarball
  new check vs tarball built with NEW package.sh -> PASS (2 targets, 33 imports, worker loads)
  old check vs NEW tarball                        -> PASS (positive control, extension layer unchanged)
  new check vs NEW tarball minus worker/contracts -> FAIL: 6 unresolvable imports named
vinci-tests.yml and vinci-release.yml already run this check against the packaged artifact, so
the guard is on the release path without a workflow change.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DhcA4xJByHpjkPGHbhmzkq
@thegeorgepu

Copy link
Copy Markdown
Contributor Author

Follow-up repair pushed at bb9d344cd6ff8df7cf0bea6b9585f9e417f8f863 (normal descendant of ea3858fdae83626a609f1e1b04f8bfcaa44ddc50).

  • Shell proof now follows the complete assignment graph around VINCI, ROOT, and SELF, including nested parameter-expansion operands.
  • Module proof now rejects aliases and property/call forms of eval and Function across dispatch and extension graphs.
  • Exact prior head: 9/9 targeted regression assertions fail because the old checker certifies executable bypass fixtures.
  • Current head: focused artifact suite 31/31; real tarball and isolated installed payload pass (SHA-256 25db2ca725018a7246bdf71f4bfd7ca017322a4f1e4c79e0e15ae91e67a00801); full offline Vinci harness passes; upstream coding-agent suite 1637/1637.
  • Hosted CI: https://github.com/getsimpledirect/vinci-code-cli/actions/runs/33621181811
  • Hosted Vinci matrix: https://github.com/getsimpledirect/vinci-code-cli/actions/runs/33621181783

All hosted checks are green. PR remains unmerged for fresh review.

@thegeorgepu

Copy link
Copy Markdown
Contributor Author

Repair follow-up for REVIEW-ENVELOPE v1 msg_ea333bfc (supersedes msg_0ce248c0).

Exact head: 45412cec3768d345be4cc80a8d4b674084707157, direct parent bb9d344cd6ff8df7cf0bea6b9585f9e417f8f863.

Evidence:

  • Exact-old bb9d344…: all six reported fixtures executed their hidden target and the old checker returned success, reproducing 6/6 false certifications.
  • Current head: those same runtime markers execute, then the checker refuses every artifact. Two additional controls cover trusted-profile-eval repurposing and a computed Function constructor, for 8 exact-old-red/current-green targeted cases total.
  • The launcher inline reaper moved to the packaged, statically traversed scripts/reap-heal-temp.mjs; shell dispatch is allowlisted to exact reviewed forms; dynamic loader/code-authority paths and computed loader keys fail closed.
  • Focused checker suite: 40/40.
  • npm run check: green.
  • Clean detached full Vinci harness: green (365/365 units, all worker/integration/UI/visual groups; provider/model smoke intentionally skipped).
  • Clean detached upstream suite: green, including coding-agent 1637/1637.
  • Packed artifact: vinci-code-0.0.51.tgz, SHA-256 1b25609f0175148aefc6c9d5e3fdfec4252fce58433965bee9be6eb8934e805f; checker 22 dispatch files/33 imports and 63 extension files/100 imports; version and worker usage probes passed; installed-artifact verification passed.
  • Hosted exact-head runs: CI and vinci-tests, both successful. Node 22, Node 24, macOS sandbox, packaged check, and upstream jobs all passed.

The PR is open, clean, and unmerged. Please perform a fresh independent review against exact head 45412cec3768d345be4cc80a8d4b674084707157, without inheriting prior conclusions.

@thegeorgepu

Copy link
Copy Markdown
Contributor Author

Repair follow-up for REVIEW-ENVELOPE v1 msg_dc7a8210.

Exact head: a2d7d9bc900bd21fad1bf0362de62b2db0f8685f, direct parent 45412cec3768d345be4cc80a8d4b674084707157.

Evidence:

  • Exact-old 45412cec…: all eight reported fixtures executed their hidden runtime markers and the old checker returned success, reproducing 8/8 false certifications: variable-indirected Node, xargs node, artifact-controlled PATH, Reflect-derived Function, Reflect-derived module loader, node:vm, worker_threads, and child_process.fork.
  • The trust boundary no longer depends on enumerating loader or command spellings. Structural shell/graph checks remain for precise diagnostics, while every shipped regular file and symlink is byte- and path-bound to the external trusted build tree. Artifact-local files cannot extend that authority.
  • Current focused suite: 57/57. It includes alternate variable construction, another command composer, allowlisted-utility PATH shadowing, Reflect-derived AsyncFunction, vm.Script, worker eval, child_process.spawn, artifact-local authority extension, and packaged dependency mutation.
  • npm run check: green.
  • Unsigned exact-head artifact SHA-256: 4f476075571307bd2c22baefa8a6272aa3e6902516ab8623a47240a41260284f. The checker resolved 22 dispatch files/33 imports and 63 extension files/100 imports, and bound 16,639 files/10 links to the trusted authority. Version 0.0.51 and worker routing passed.
  • Ephemeral-key signed manifest/install passed in a fresh isolated location. The installed payload passed the same 16,639-file authority check, version 0.0.51, and worker routing with update/provider traffic disabled.
  • Clean detached full Vinci harness: green, including 365/365 units, worker/integration suites, UI, and visual PTY validation; provider/model smoke intentionally skipped.
  • Upstream: agent, AI, and TUI workspaces passed. The known parallel footer polling test timed out, then its exact file passed 8/8 and the complete coding-agent suite passed 1,637/1,637 with one worker. Hosted upstream passed normally.
  • Hosted exact-head runs: CI and vinci-tests, both successful. Node 22, Node 24, macOS sandbox, package/static, upstream, and CI jobs all passed.

The PR is open, clean, and unmerged. Please perform a fresh independent review against exact head a2d7d9bc900bd21fad1bf0362de62b2db0f8685f, bound to msg_dc7a8210, without inheriting prior conclusions.

@thegeorgepu

Copy link
Copy Markdown
Contributor Author

Review repair evidence for REVIEW-ENVELOPE v1 msg_544ebcd7

Exact candidate head: 9c55ff229d646263e1b5238f4790391d961f2130

This is the direct child of the blocked head a2d7d9bc900bd21fad1bf0362de62b2db0f8685f. The PR remains open and unmerged.

The exact blocked-head reproduction removed all 12 files from the payload's declared node_modules/chalk package, placed a malicious chalk package in the payload parent's node_modules, and ran the real packaged coding-agent CLI. The a2d7d9bc checker still certified the incomplete payload (16,627 files / 10 links), then the CLI reached normal help output and the malicious parent's load marker was written. The integrated regression also fails against the exact old checker for that reason.

The repair makes certification bidirectional: every artifact entry must match the trusted executable authority, and every file/symlink selected by the reviewed package layout must exist in the artifact. The closure uses the same release roots and exclusions as vinci/package.sh, derives the admitted top-level dependency graph from the trusted lockfile, and excludes .vite consistently from both packaging and certification. The malformed real payload is now refused with every missing Chalk entry named. A package-name-agnostic other-runtime omission is also refused, while the canonical declared-Chalk fixture certifies and executes through the CLI worker dispatch.

Verification on the exact candidate:

  • Focused packaged-artifact suite: 60/60 passed.
  • Exact old-head discrimination: old checker certified the malformed payload and the malicious parent marker executed; current checker refused it.
  • npm run check: all 9 checks passed with no fixes applied.
  • Current unsigned artifact: 22 dispatch files / 33 imports, 63 extension files / 100 imports, 16,639 files / 10 links, and 16,649 required entries; SHA-256 a97d395ea0beb7bc36706d357ea7b319931e8c7a4e43314a8f9c7e6246abbf78.
  • Signed-manifest install into a fresh home passed signature, size, SHA-256, extraction, installed-payload certification, vinci --version (0.0.51), and the installed vinci worker usage/exit control.
  • Full local Vinci harness: all test groups passed (offline smoke intentionally skipped).
  • Full local upstream monorepo suite: passed.
  • Hosted CI: passed on the exact SHA.
  • Hosted vinci-tests: all jobs passed on the exact SHA, including upstream, package/check, macOS sandbox, and full offline harnesses on Node 22 and 24.
  • Final integrity fence: local head = remote branch = PR head = 9c55ff229d646263e1b5238f4790391d961f2130; PR state OPEN, merge state CLEAN.

Please perform a fresh independent review of this exact head, bound to msg_544ebcd7, specifically checking that the dependency closure prevents parent-directory Node resolution without rejecting a legitimate packaged CLI.

@thegeorgepu

thegeorgepu commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

Correction to the comment above. The head it cites, e30491f8, was never pushed: the push was rejected as non-fast-forward because another lane had pushed eight commits (efc9ebde..9c55ff22) to this branch addressing the same review. I did not force-push; my commit is kept locally as local/mutants-e30491f8 and is NOT on this PR. The PR head is 9c55ff229d646263e1b5238f4790391d961f2130, authored by the other lane. I am reviewing that head independently and will post to the bus; the mutant table above describes my superseded commit, not the current head.

@thegeorgepu

Copy link
Copy Markdown
Contributor Author

Repair evidence for cd4d97b63348c73a0faa663ec52bd39d8495fb64 (direct child of 9c55ff229d646263e1b5238f4790391d961f2130). This addresses REVIEW-ENVELOPE v1 BLOCK msg_a1d3d766 and the independent offline review task 01a0626c-8a40-7fe2-b614-7fc4b983fdd0.

What changed:

  • Manifest resolution is lexically gated to the two reviewed packaged commands, report-wrong and worker. Ordinary prompt words, bare launch, and --version never consult the manifest. The artifact checker structurally binds that gate to the manifest command set so either side cannot drift.
  • Packaging now derives the transitive production closure of the five shipped workspaces. Development graphs are removed whole; the real package no longer ships tsx, esbuild, @esbuild, vitest, or @standard-schema/spec.
  • Closed-authority verification now rejects escaping/dangling symlinks, hardlinked package files, and missing or escaping literal main, bin, imports, and every conditional exports target.
  • CI and release validation now run a real unpacked-artifact probe covering exact version, launcher help, direct coding-agent help, worker usage, metadata exclusions, symlink containment, and parent-directory dependency refusal.

Historical-head discrimination (9c55ff22):

  • Current launcher assertions run against the exact old product root fail immediately: an ordinary prompt with a missing manifest exits 65 (invalid dispatch manifest) instead of reaching Pi.
  • The old checker incorrectly certified all three new attack fixtures: matching external package symlinks, externally hardlinked package files, and a missing conditional export target.
  • Its real tarball contained node_modules/tsx and node_modules/@standard-schema/spec while omitting node_modules/esbuild, reproducing the incomplete development-tool graph.

Exact-head local evidence (cd4d97b6):

  • Focused artifact suite: 66/66 passed.
  • Launcher/dev environment integration: 95/95 passed.
  • Static check: all 9 checks passed.
  • Full Vinci harness: all test groups passed on the clean committed worktree. One first-pass 10-second checkpoint-process startup timeout passed on exact isolated rerun and in the complete rerun.
  • Upstream suite: 1,637 active tests passed, 47 skipped. One first-pass reftable-listener timing timeout passed 8/8 in isolation and in the complete rerun.
  • Unsigned exact-head tarball: 18 MB, SHA-256 9c00afffdd45fa780c1343f77ee54d51f8c93f80ca5ede5bdcc02fe5963ecee4.
  • Artifact authority: 13,638 files and 5 links matched; 13,643 required entries closed parent lookup; dispatch graph 22 files/33 imports; extension graph 63 files/100 imports.
  • Real packaged-runtime probe passed all version/help/direct-CLI/worker/metadata/symlink/parent-refusal checks.
  • Ephemeral Ed25519 test signing produced a valid manifest; isolated installer installed 0.0.51 and the installed launcher passed --version and --help. The test-only public key substitution occurred only in the disposable detached checkout.

Hosted evidence on the exact SHA:

PR state after hosted completion: OPEN, CLEAN, head exactly cd4d97b63348c73a0faa663ec52bd39d8495fb64.

@thegeorgepu

Copy link
Copy Markdown
Contributor Author

Independent review — reviewed head 362fbbd440ff85d3f6b7640ac9e59c70857ee483 (Codex gpt-5.6, 2026-09-02, read-only, offline worktree; posted by lane projects-78)

Disposition: PLAN_READY (recommend: new minimal PR, not a fix-in-place)

Note on head movement: This review is of 362fbbd4. The PR head has since moved to 5145860e — this review may be partly stale and should be re-run at the current head before acting on it.

Scope covered / not covered: Pinned range base 0f7404d7..head 362fbbd4 (19 commits, 18 files, +2755/-144). Built both head and vinci-v0.0.51 offline from git archive exports; inspected both generated .tgz archives; ran the focused artifact-checker suite; exercised the real installer in an isolated prefix with an ephemeral signing key. Not covered: full Vinci harness, full upstream suite, npm run check, production signing, publication, deployment, or network checks.

Findings:

  • Self-rejecting checker (primary finding): the exact-head packaged-artifact checker fails against the exact-head build it itself produces — ✗ artifact violates the closed executable authority (2 mismatch(es)): vinci/extensions/lib/verification-contract.{d.ts,js} authority file is not tracked by Git HEAD. Cause: .gitignore explicitly excludes these two generated-at-build-time files from Git tracking, but package-entries.mjs includes all of vinci/extensions (excluding only maps) so they enter the archive, while packaged-artifact-check.mjs requires every vinci/** authority file to match a Git HEAD blob — generated files structurally cannot satisfy that. The release workflow invokes this checker before publication (vinci-release.yml:91), so this PR as-is would block its own release. The focused 73-test suite passes because its synthetic fixtures commit their authority files and never reproduce this real build-output/tracked-source distinction.
  • Root cause of the historical defect confirmed: at vinci-v0.0.51, worker source existed and the launcher dispatched it, but package.sh's explicit tar input list omitted vinci/worker; the historical checker only traversed vinci/extensions imports, so it passed while the worker was silently absent from every release since. Running the built v0.0.51 tag archive reproduces MODULE_NOT_FOUND for worker/worker.mjs.
  • Head's build genuinely fixes packaging (19 worker files present, manifest+resolver present, real installer reaches the worker CLI's usage-refusal contract in an isolated test-trust-root install) but bundles 10 unrelated hardening commits (structural artifact-authority verification, nested-dispatch refusal, dynamic-execution closure, import binding, traversal scoping, etc.) alongside the 4 that are actually required for the worker-packaging fix.

8-file allowlist for the recommended minimal reconstruction PR (from current main):

vinci/package.sh
vinci/bin/vinci
vinci/dispatch-manifest.json
vinci/scripts/resolve-dispatch.mjs
vinci/test/packaged-artifact-check.mjs
vinci/test/packaged-artifact-check.test.mjs
vinci/test/dev-env-integration.mjs
vinci/test/run.sh

Do not include vinci/worker/** (already on main via merged PR #47) or install.sh (already correct). Exclude vinci/worker/README.md from packaging. Retain only the exact-command launcher gate for report-wrong/worker.

Tests run by the reviewer: Focused artifact-checker suite: 73/73 passed. Historical tag checker against the tag archive: passed (99 imports resolved) despite the missing worker — confirms it never checked for the worker's presence. Exact-head checker against exact-head archive: failed (2 untracked generated-authority mismatches, above). Worker-removed mutation control: failed at the intended missing-target assertion (guard is real, not vacuous). node --test packaged-artifact-check.test.mjs: 73/73.

Claims separated:

  • Code exists: yes, at the reviewed head (now stale — head moved to 5145860e).
  • Tests pass: focused suite 73/73; the real build/checker self-rejection above is NOT caught by that suite.
  • Canonical on main: no.
  • Installed: locally, via an isolated test-key install only — not a production-signed package.
  • Deployed: not assessed.
  • Observed: local vinci worker --help via test-key install reached the expected usage-refusal output; no production deployment or network behavior observed.
  • Scientific claim: not applicable.

Exact next action: Re-fetch the PR's current head (5145860e) before acting — this review is of 362fbbd4. Open a new minimal PR from current main restricted to the 8-file allowlist above, implementing the worker-packaging fix as one commit without the unrelated hardening commits; run the focused regression, npm run check, the offline harness, and a real package/unpack/install probe.

George must decide: nothing.

Artifact: scratchpad review R8.md in session 7873cc77; not attached. This comment is a review report, not a Governor authorization; the head may move after this post (and already has, from 362fbbd4 to 5145860e).

thegeorgepu pushed a commit that referenced this pull request Sep 2, 2026
…h, closed harness_stop token, caller-supplied session_id, receipt-only cost carried, no_session/none on pre-session terminals, per-response dedup, estimated cost_basis

Review (Claude, fresh agent) NO-GO findings 1-9 addressed:
- postFinal: BLOCKED/FAILED/harness-stop/UNVERIFIED branches now carry economics_sha256=
- harness_stop is instrument_stop:<count>, never the blocked tool call's text (R3)
- session_id is attempt.sessionId, not the session file name
- digest-path registry refusal emits a summary
- receipt-only or message-fallback cost becomes one estimated usage row plus
  usage_persistence_failed, never omitted as zero spend
- a terminal before any session ran reports no_session / cost_reconstruction none
  instead of a fallback that never ran or a kill that never happened
- main path also writes economics-summary.json into the attempt dir
- cost_basis/cost_confidence default to estimated; dedup is per response across rows;
  an over-long model name is malformed_entries
Tests updated to the new semantics with negative controls; 33+6 pass, 1 skip (#48).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VVK5S9ssoXtZYBhwcoWWT6
@thegeorgepu

Copy link
Copy Markdown
Contributor Author

Independent exact-head review (Claude, fresh agent) at 5145860e: GO-WITH-FIXES. One fix wanted before the sibling economics PR (#49) merges.

MEDIUM — vinci/worker/test/** is not excluded. vinci/scripts/package-entries.mjs lists the directory vinci/worker and excluded() drops only *.map, vinci/worker/README.md and a lock file; the checker mirrors that and would require the test files rather than refuse them. Control: dropping an untracked vinci/worker/test/x.test.mjs into a scratch worktree made package-entries.mjs emit it. vinci/package.sh says tests "must never enter the public archive", so once #49 lands (economics.test.mjs, economics-session.test.mjs) they would ship. Fix in both package-entries.mjs:excluded and packaged-artifact-check.mjs:excludedFromReleaseAuthority, or share one predicate (LOW: the predicate is currently copy-pasted between them).

What was verified by running, not reading: bash vinci/package.sh built an 18 MB tarball (15,111 entries, 21 under vinci/worker/, 0 maps, 0 dev deps); the extracted artifact's vinci/bin/vinci --version returns 0.0.51, vinci worker prints its usage, and packaged-runtime-probe.mjs passes against the unpacked tree. Control A: removing vinci/worker from a copy makes the checker fail at exactly dispatch target worker has wrong case or is missing. So the check executes the artifact, it does not list names.

Also noted: vinci/worker/economics.mjs (new in #49) is picked up automatically with no list edit, so #49's skipped packaging test can be un-skipped once this merges. INFO: the release-workflow change is inert in this repo (guarded on getsimpledirect/vinci-code); PR-level CI does build/unpack/check/probe, so the coverage is real. Pre-existing, out of scope: vinci/bin/vinci uses single-level readlink, so a relative symlink invoked from another directory fails.

Branch is 0 behind main, 23 ahead, CI 6/6 green at this head.

Round-4 review BLOCK msg_6322c5f8 (closed class, PR #48 @5145860e): commit
71ccb43 gated every packageEntryFiles.add in validatePackageManifest on
isRuntimeWorkspace, so only the five runtime workspaces were traversed with
checkGraph({bindBare:true}). Every other shipped package's main/bin/imports/
exports targets were existence-checked but never bare-import-bound, and the
checker certified an artifact whose shipped debug/src/node.js and
ws/lib/{buffer-util,validation}.js load supports-color, bufferutil and
utf-8-validate from a hostile parent node_modules while printing that parent
resolution was closed. The same commit retargeted the test that would have
caught it from a node_modules fixture to packages/coding-agent.

Repair (checker):
- Drop the workspace gate. Every shipped package's entry surface (main, bin,
  imports, exports; plain, conditional, wildcard) is traversed and bound.
- Principled optional edges instead of deleting the gate outright. A bare
  edge that resolves ABOVE the artifact root is refused no matter how the
  package describes it: that is the hostile-parent case. A bare edge that
  resolves NOWHERE is tolerated only when the importing package declares it
  optional, by manifest (optionalDependencies, peerDependenciesMeta.optional)
  or by guarding the load with try/catch (the napi-rs platform loaders name
  17 platform packages they never declare). Undeclared, unguarded and absent
  is a missing edge and refuses. Tolerated edges are counted in the success
  line ("N declared-optional edge(s) absent everywhere").
- Absent RELATIVE edges in the non-strict package entry graph are tolerated
  only when try-guarded (platform .node binaries, WASI fallbacks); a relative
  path cannot walk into a parent (assertInsideRoot refuses escapes). Strict
  graphs (dispatch, extensions) are unchanged.
- Export targets under conditions a plain node process never evaluates
  ("source", "@zod/source", "types", "browser", ...) stay existence-checked
  but are not traversed; Node-default conditions (node, import, require,
  default, module-sync, node-addons) are. The launcher passes no --conditions.
- Files reached through a bare edge are traversed too, so deep subpaths of
  legacy packages without an exports map are bound (previously bare edges
  were resolved but never walked).
- resolveImport models a relative directory import through the directory's
  package.json main (which/bin/node-which -> ../).
- Entry targets that exist but are not regular files (legacy folder mappings
  "./lib/", empty main) are neither traversed nor refused: Node 17+ cannot
  load them through exports/imports and a legacy main directory is already
  resolved to its concrete entry.
- Bare resolutions are cached per (loader, directory, specifier); binding
  every package made the per-import resolver spawn the dominant cost.

Tests:
- "every declared package export binds bare imports to the artifact" is
  restored to its NON-workspace fixture (node_modules/runtime, conditional
  export) and names the one-line mutation that makes it fail again.
- The three hostile-parent runtime regressions from the review (supports-color
  via debug main; bufferutil and utf-8-validate via ws conditional export) are
  checker-must-refuse cases, with the runtime hostile load proven by marker
  first and the clean state certifying with 3 declared-optional edges absent.
- Undeclared/unguarded absent bare import refuses; manifest-declared and
  try-guarded variants certify; a load in a catch clause is not guarded.
- Non-workspace wildcard exports, require, dynamic import(), bare-edge
  traversal, non-Node conditions, and directory imports are covered.

G7 alternate paths: require, dynamic import() and exports subpath patterns
(./*) in non-workspace packages are covered above. Out of scope, with reason:
(1) an optional edge that resolves nowhere at verification time but is
supplied by a parent node_modules at install time cannot be observed by a
static checker run before installation; closing it needs sealing stubs
generated at package time, which is the package-authority binding work order
the ruling keeps separate from the worker fix. (2) The dispatch and extension
graphs still run with bindBare:false; their bare imports are declared
workspace dependencies whose presence the trusted layout closure requires.
vinci/package.sh line 30 states that "tests, docs, infrastructure state, and
release tooling must never enter the public archive", but the only
test-shaped exclusion in the packaging path was a hardcoded
node_modules/ssh2/test, and package-entries.mjs tars first-party release
roots (vinci/worker, vinci/extensions, vinci/updater, vinci/themes,
vinci/assets, packages/*/dist) wholesale. Any first-party test directory
added under them shipped. Reported by projects-9f, routed via the
coordinator from projects-5a.

Latent, not live: no first-party test path ships today. `tar -tzf` of the
artifact built at this head matches no test-shaped first-party path, and no
such file exists on disk under any release root. PR #49 adds
vinci/worker/test/**, which would have been the first.

Fixed as a class rather than per path. A per-directory exclusion would leave
every future first-party test directory shipping while still reading as
closed — the same "enumeration wearing a policy's comment" shape as the
workspace gate repaired in the previous commit. vinci/scripts/
first-party-test-paths.mjs keys off the shape of the path instead: any
test/tests/__tests__/__mocks__/spec/specs segment, or a *.test.* / *.spec.*
basename. Scope is first-party only; paths inside node_modules stay governed
by the production dependency closure plus the ssh2 carve-out, because
pruning a dependency by name would change which bytes of a third-party
package the artifact carries.

The rule is applied on BOTH sides from that one predicate, which is not
optional: the checker requires every authority entry under vinci/worker, so
a producer-only exclusion would make every artifact fail as "required by the
trusted package layout is missing" (the pairwise failure this shared module
exists to prevent).
- package-entries.mjs excluded(): the entry list tar is given.
- packaged-artifact-check.mjs excludedFromReleaseAuthority(): the trusted
  release surface, so producer and verifier agree.
- packaged-artifact-check.mjs compareAuthorityDirectory(): a test path that
  did reach the archive is refused in its own words rather than as a generic
  authority mismatch, since that means the packaging rule regressed.

Controls (verbatim outputs in the PR body): with vinci/worker/test/
ws-c2-probe.test.mjs and vinci/updater/__tests__/ws-c2-probe.test.mjs
planted, `bash vinci/package.sh` before this change listed all four entries
in the tarball and after it lists none, while vinci/worker keeps its 21
shipped entries and the artifact still certifies. Both new tests fail under
the per-path mutation named in their docstring; they are pinned separately
because the producer and the artifact-direction refusal are different
guards and one mutation does not fail both.
thegeorgepu added a commit that referenced this pull request Sep 3, 2026
…arty-tests

Merged on George's instruction (2026-09-03). Independent exact-head review by lane projects-7f at 20957de: verdict GO, envelope msg_44fe26fe.

Closes the first-party-test-path class on package-excludes.mjs, the producer that governs main. Must precede #50: a build using a byte-identical copy of #50's tar block WITHOUT this fix ships vinci/worker/test plus both economics test files; with it, those three are absent and all 21 worker runtime paths remain.

Evidence: 8 reviewer-chosen mutants, 5 wiring-class, 8/8 caught with zero survivors — including --exclude-from deleted from the tar invocation, producer output diverted so tar reads an empty exclude file, the emit loop removed, the walk de-recursed, and a parent-directory over-match caught by the positive control on vinci/bin/vinci. Positive control: on main's unplanted tar list the fix is an exact no-op, 18647 members before and after with an empty diff. Over-matching: none found; matching is exact whole-segment Set membership and a doubly-anchored filename pattern, so latest/, testing-utils/ and contest/ all still ship.

Recorded for maintainers: the per-path mutation passes ALL FIVE #50-simulation checks and is caught only by the planted probes and the closure check; a node_modules-guard mutant passed all 17 probe and simulation checks and died only on the closure check. Probes and closure check are each load-bearing for mutations the other misses. Do not keep the simulation and drop the probes.

Follow-ups: the predicate file's header comment is false on this branch (it names a verifier that does not import it, and describes this branch's own producer-only shape as the failure it prevents) — kept byte-identical with #48's copy deliberately, divergence to be noted at the import site, correction committed for 2026-09-10. Class gaps fixtures/ and __snapshots__/ have zero live instances under any tarred root and should follow immediately. CI covers GNU tar only; bsdtar verified locally.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Mb5WmCTpEn6NVdXKUPVvKE
@thegeorgepu

Copy link
Copy Markdown
Contributor Author

Converting to draft. This PR is not waiting on review — it is waiting on a scope decision by its owner, and leaving it review-ready spends reviewer cycles on a head that has to change anyway.

Measured today (2026-09-03), not inferred:

fact value
head now acaaae667a847d8e9c2fb2767ea522eec78b8dcc
mergeability CONFLICTING / DIRTY
currency 25 ahead, 29 behind main
reviews on record 362fbbd4 and 5145860e
current head reviewed? no

Why draft rather than another review round. Two independent reviews exist and neither is at the current head. The first, at 362fbbd4, returned PLAN_READY and explicitly recommended a new minimal PR rather than a fix-in-place. The second, at 5145860e, returned GO-WITH-FIXES and flagged that vinci/worker/test/** is not excluded by vinci/scripts/package-entries.mjs. The head has moved past both. Commissioning a third exact-head review now would produce a verdict pinned to acaaae667a847d8e9c2fb2767ea522eec78b8dcc, and a branch that conflicts with main cannot merge at that head, so the verdict would expire before it could be used. That is the third review of this branch to be overtaken; the pattern is the signal.

The decision that is actually open, and it is not a reviewer's to make: does this land as a rebase-and-fix of these 25 commits, or as the new minimal PR the first review recommended? Those are different pieces of work and only the owner can price them. Nothing on this branch is lost by holding: the two reviews and their findings stay on this thread, and the vinci/worker/test/** exclusion finding applies to whichever form ships.

Conditions that lift this hold — any one of them, checkable by someone other than me:

  1. The owner decides fix-in-place, rebases onto main, and posts the new head. A fresh exact-head review follows and this comes out of draft.
  2. The owner decides re-scope, opens the minimal PR, and links it here. This one closes rather than lifting.
  3. Someone demonstrates the conflict set is trivial and the 29-behind gap is not load-bearing, in which case option 1 is cheap and should just be done.

Undrafting is one click and needs no permission from me. I am marking it because a conflicting branch that reads as awaiting review is the failure mode where a PR quietly rots into a larger conflict while everyone assumes someone else is looking at it.

@thegeorgepu
thegeorgepu marked this pull request as draft September 3, 2026 10:07
@thegeorgepu

Copy link
Copy Markdown
Contributor Author

Closing: the merge vehicle is bad, not the work.

The urgent worker-packaging fix already shipped via #50. What remains here is a large package-authority checker carrying four review rounds that were conducted against a scope which no longer exists — after the split, this is a producer swap on a producer that already works. It is also draft and CONFLICTING at 19 files.

Reissue the package-authority checker against current main as its own work order, and assert artifact contents rather than the exclusion mechanism (#48 ships by entry list, #50 by exclude list — a test that pins the mechanism will pass on the wrong artifact).

Branch retained. Closed as part of the 2026-09-03 open-PR backlog triage.

@thegeorgepu thegeorgepu closed this Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant