Skip to content

packaging: exclude first-party test paths from the release archive (exclude-list producer) - #51

Merged
thegeorgepu merged 1 commit into
mainfrom
packaging/exclude-first-party-tests
Sep 3, 2026
Merged

packaging: exclude first-party test paths from the release archive (exclude-list producer)#51
thegeorgepu merged 1 commit into
mainfrom
packaging/exclude-first-party-tests

Conversation

@thegeorgepu

@thegeorgepu thegeorgepu commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

vinci/package.sh:30 promises that "tests, docs, infrastructure state, and release tooling must never enter the public archive", and then tars first-party directories wholesale. The exclude-list producer never implemented the test half of that promise. Its only test-shaped exclusion is the hardcoded --exclude='node_modules/ssh2/test', which governs a dependency's own contents and says nothing about first-party trees. package-excludes.mjs walked only the npm dependency tree and had no test/spec rule at all. So the comment asserted a rule the code did not enforce.

Why now

That was latent until #49 (merged 2026-09-02T22:56:55Z), which put vinci/worker/test/economics.test.mjs and vinci/worker/test/economics-session.test.mjs on main.

  • On main today, package.sh:49's tar path list does not include vinci/worker, so nothing under it ships. This change is therefore a no-op on main's artifact (proved below: the unplanted listings are identical, 18529 members both).
  • PR worker: include vinci/worker in the packaged release (split from #48) #50 (cf196e63) package.sh:50 does add vinci/worker, excluding only vinci/worker/README.md. From that moment both test files ship.
  • worker: include vinci/worker in the packaged release (split from #48) #50's checker cannot see it: it verifies dispatch targets are present (lines 100-105) and drives vinci worker to its usage refusal (135-141). It has no unexpected-file check and no closure check. So the two files would ship silently, behind a green checker.

This PR should merge before #50.


What changed

File Change
vinci/scripts/first-party-test-paths.mjs new, byte-identical to PR #48
vinci/scripts/package-excludes.mjs emits an exclusion for every first-party test path it discovers
vinci/test/package-first-party-tests.mjs new, 19 checks, asserts real tarball contents
vinci/test/run.sh registers the new lane

vinci/package.sh is not modified.

PREDICATE_BYTE_IDENTICAL: yes

vinci/scripts/first-party-test-paths.mjs is taken verbatim from PR #48's head acaaae667a847d8e9c2fb2767ea522eec78b8dcc:

sha256(git show acaaae66:vinci/scripts/first-party-test-paths.mjs) = c97ca5c09b873b4566985b58e2d643c695cb0081ef02891dc5e580e963b6f53e
sha256(this branch's vinci/scripts/first-party-test-paths.mjs)     = c97ca5c09b873b4566985b58e2d643c695cb0081ef02891dc5e580e963b6f53e

Zero bytes changed, so #48 merges as a clean no-op on this file rather than a conflict. The identity survives npm run check (biome runs --write; the sha is unchanged after it).

One thing a reviewer should know rather than discover: because the file is byte-identical, its header comment describes #48's world — it names package-entries.mjs (the entry-list producer) and runtime-package-closure.mjs, neither of which exists on this branch. I judged byte identity with #48 worth more than a locally-accurate comment, since the comment becomes accurate the moment #48 lands. The comment is not load-bearing; the exported isFirstPartyTestPath is.

How it joins the exclude-list producer

package-excludes.mjs writes lines into the file package.sh already passes as --exclude-from. It now walks the repository and emits an exclusion for every path matching the shared predicate. Two properties are deliberate:

  1. Keyed off path SHAPE, never a per-path list. vinci/worker/test is not enumerated anywhere — that per-path form is exactly what was rejected, and the mutation control below pins it.
  2. It walks the repository, not package.sh's tar path list. So a first-party root is covered the moment it enters that list. worker: include vinci/worker in the packaged release (split from #48) #50 needs no change here and no change to package.sh. Exclusion patterns that match nothing are inert in both bsdtar and GNU tar, which is what makes over-emitting safe.

Scope stays first-party: isFirstPartyTestPath returns false under node_modules and the walk skips node_modules outright, so a dependency still ships exactly the bytes its own package contains, and the ssh2/test carve-out remains the thing that governs it.

Producer output on this tree (9 lines; only vinci/worker/test is under a root #50 would tar):

packages/agent/test
packages/ai/test
packages/coding-agent/examples/extensions/custom-provider-gitlab-duo/test.ts
packages/coding-agent/test
packages/tui/test
scripts/check-all.test.mjs
vinci/containment-broker/test
vinci/test
vinci/worker/test

One judgement call worth flagging: --exclude-from lines are read as globs, and a literal path containing a glob metacharacter could match nothing — i.e. fail open and ship the file. There is no escaping form both bsdtar and GNU tar accept, so the producer throws rather than shipping a hole.


Evidence

Every item below is a real tarball built by the real producer (bash vinci/package.sh), listed with tar -tzf. Nothing here is a source-tree approximation. Local tar is bsdtar 3.5.3; the new lane is registered in vinci/test/run.sh, so CI's offline (22) / offline (24) run it on ubuntu under GNU tar — the only lane that exercises package.sh under GNU tar.

The "before" builds use a disposable copy of package.sh differing only in the producer path, pointed at git show origin/main:vinci/scripts/package-excludes.mjs. Verified: diff <(sed 's#ws-c3-package-excludes-BEFORE#package-excludes#' BEFORE.sh) vinci/package.sh is empty.

NEGATIVE_CONTROL — testable on main today

Three probes planted under roots the tar list already carries, in three shapes (a __tests__ directory segment; a *.test.* basename with no test directory above it; a specs directory holding a non-JS file, which only the directory-segment half of the rule catches):

vinci/updater/__tests__/ws-c3-probe.test.mjs
vinci/extensions/ws-c3-probe.test.mjs
vinci/themes/specs/ws-c3-probe.json

BEFORE (origin/main's producer) — 18534 members; grep 'ws-c3' on the listing, verbatim:

18520:vinci/extensions/ws-c3-probe.test.mjs
18526:vinci/themes/specs/ws-c3-probe.json
18531:vinci/updater/__tests__/ws-c3-probe.test.mjs

AFTER (this branch) — 18529 members; grep 'ws-c3' on the listing, verbatim:

(none)

Full delta between the two listings, verbatim — comm -23 before after:

vinci/extensions/ws-c3-probe.test.mjs
vinci/themes/specs/
vinci/themes/specs/ws-c3-probe.json
vinci/updater/__tests__/
vinci/updater/__tests__/ws-c3-probe.test.mjs

comm -13 before after (present after, absent before) is empty. Exactly the 5 probe members were removed and nothing else.

PR50_SIMULATION — the case main cannot exercise

🔴 Caveat, stated plainly: this is NOT an assertion about main's current behaviour. On main vinci/worker is not in the tar path list at all, so nothing under it ships and there is nothing to exclude. The test asserts that separately (✓ on main, vinci/worker is not tarred at all). The block below simulates #50's condition so this branch can prove the archive is closed before #50 merges.

The simulation applies #50's two edits (--exclude='vinci/worker/README.md', and vinci/worker appended to the roots line) to a disposable copy of package.sh. vinci/package.sh on this branch is untouched. The simulated tar block was verified byte-identical to the real thing:

diff <(sed -n '28,53p' SIM.sh) <(git show cf196e63:vinci/package.sh | sed -n '28,53p')
→ empty ("SIM tar block is IDENTICAL to PR #50 cf196e63")

grep '^vinci/worker' on the listing, #50-sim WITHOUT the fix (18554 members), verbatim:

vinci/worker/
vinci/worker/branch-lease.mjs
vinci/worker/build.mjs
vinci/worker/bus.mjs
vinci/worker/cleanroom.mjs
vinci/worker/contracts/
vinci/worker/contracts/canonical.mjs
vinci/worker/contracts/digest.mjs
vinci/worker/contracts/path-grant.mjs
vinci/worker/contracts/within-order.mjs
vinci/worker/debris-authority.mjs
vinci/worker/economics.mjs
vinci/worker/evidence.mjs
vinci/worker/exec.mjs
vinci/worker/governor.mjs
vinci/worker/lease.mjs
vinci/worker/outbox.mjs
vinci/worker/publisher.mjs
vinci/worker/run.mjs
vinci/worker/session-read.mjs
vinci/worker/task.mjs
vinci/worker/test/
vinci/worker/test/economics-session.test.mjs
vinci/worker/test/economics.test.mjs
vinci/worker/worker.mjs

#50-sim WITH the fix (18551 members), verbatim:

vinci/worker/
vinci/worker/branch-lease.mjs
vinci/worker/build.mjs
vinci/worker/bus.mjs
vinci/worker/cleanroom.mjs
vinci/worker/contracts/
vinci/worker/contracts/canonical.mjs
vinci/worker/contracts/digest.mjs
vinci/worker/contracts/path-grant.mjs
vinci/worker/contracts/within-order.mjs
vinci/worker/debris-authority.mjs
vinci/worker/economics.mjs
vinci/worker/evidence.mjs
vinci/worker/exec.mjs
vinci/worker/governor.mjs
vinci/worker/lease.mjs
vinci/worker/outbox.mjs
vinci/worker/publisher.mjs
vinci/worker/run.mjs
vinci/worker/session-read.mjs
vinci/worker/task.mjs
vinci/worker/worker.mjs

Delta, verbatim — the three members #50 would otherwise have shipped:

vinci/worker/test/
vinci/worker/test/economics-session.test.mjs
vinci/worker/test/economics.test.mjs

All 21 worker runtime paths survive. That is the positive reachability control for the simulation: the absences above are the exclusion working, not the root failing to be tarred.

POSITIVE_CONTROL — the unmodified artifact

With no probes planted, before vs after:

unplanted BEFORE members: 18529
unplanted AFTER  members: 18529
diff before after → IDENTICAL — no runtime member lost on main today

Per-root member counts, before → after (identical throughout):

vinci/bin                        before=2   after=2
vinci/extensions                 before=65  after=65
vinci/themes                     before=3   after=3
vinci/assets                     before=15  after=15
vinci/updater                    before=4   after=4
packages/agent/dist              before=58  after=58
packages/ai/dist                 before=305 after=305
packages/coding-agent/dist       before=363 after=363
packages/orchestrator/dist       before=28  after=28
packages/tui/dist                before=58  after=58

The artifact still certifies with the repo's existing checker and the packaged CLI drives:

$ node vinci/test/packaged-artifact-check.mjs <extracted>
  ✓ packaged artifact: 100 relative imports in vinci/extensions all resolve inside the tarball
  exit=0

$ <extracted>/vinci/bin/vinci --version
0.0.51        (identity.json says 0.0.51 — MATCH)

MUTATION_LINE

Named in the test's docstring. Replacing the predicate call in vinci/scripts/package-excludes.mjs

if (isFirstPartyTestPath(relativePath)) {

with the per-path enumeration

if (relativePath === "vinci/worker/test") {

fails the test by name, verbatim:

AssertionError [ERR_ASSERTION]: planted first-party test path is absent from the archive: vinci/updater/__tests__/ws-c3-probe.test.mjs

false !== true
    at check (.../vinci/test/package-first-party-tests.mjs:41:9)

This is the discriminating part: the per-path mutant still passes every #50 SIMULATION: check, because it does exclude vinci/worker/test. Only the probes planted under the other first-party roots catch it. That asymmetry is the whole argument for keying off path shape, and it is why the probes live under roots the tar list already carries. The mutant was reverted from an out-of-repo copy, not with git checkout --.

The tests assert artifact CONTENTS, not the exclusion mechanism

Deliberately. A test asserting the mechanism — "the excludes file contains line X", "tar was invoked with flag Y" — passes on the exclude-list producer and inverts on the entry-list producer in #48, because the two build the member set from opposite directions. Only the artifact is common to both. This is projects-a9's rule, and it is why its proof survived a base change.


Why the checker is NOT touched here

This is a one-sided fix on purpose, not an oversight.

PR #48 carries the two-sided version, because on the entry-list producer it had to be: that checker requires every authority entry under vinci/worker to be present, so a producer-only exclusion would make every artifact fail verification as "required by the trusted package layout is missing". Producer and verifier there must import the same predicate or they contradict each other — the pairwise failure first-party-test-paths.mjs's own header comment describes.

The exclude-list producer that governs main has no such coupling. #50's checker has no closure check and no unexpected-file check; it only asserts dispatch targets are present and drives vinci worker to its usage refusal. So there is nothing on the verifier side that a producer-only exclusion can break, and nothing on the verifier side that would have caught this. Adding a checker-side rule here would collide with #48 on the very file whose byte identity this PR is trying to preserve.

Scope of this branch is the producer that governs main. The verifier side arrives with #48.


Tests

$ node vinci/test/package-first-party-tests.mjs
package-first-party-tests: 19/19 checks passed (no first-party test path enters the release archive)
  (37s: two real tarballs)

$ node vinci/test/acceptance-packaged-integration.mjs
ok (a) packaged launcher passes arguments, stdio, and exit status through to vac
ok (b) packaged launcher gives guidance without vac and does not start Pi
ok (c) packaged extension set includes acceptance, verification state, and receipt exports
ok (d) packaged receipt maps current verdicts and shows staled verdict context
✓ acceptance-packaged-integration.mjs: all tests passed

$ node vinci/test/packaged-artifact-check.mjs <extracted real build>
  ✓ packaged artifact: 100 relative imports in vinci/extensions all resolve inside the tarball

$ npm run check
PASS biome check --write --error-on-warnings . (exit 0)
PASS npm run check:pinned-deps (exit 0)
PASS npm run check:secrets (exit 0)
PASS npm run check:ts-imports (exit 0)
PASS npm run check:shrinkwrap (exit 0)
PASS npm run check:install-lock:coding-agent (exit 0)
PASS tsgo --noEmit (exit 0)
PASS npm run check:extensions (exit 0)
PASS npm run check:browser-smoke (exit 0)

Full harness against this exact head, verbatim tail:

$ VINCI_SKIP_SMOKE=1 bash vinci/test/run.sh          (exit 0)

── VISUAL (rendered TUI header via PTY) ──────────────
  ✓ native PTY renders working + completed Vinci frames cleanly

── SMOKE (real CLI, headless) ────────────────────────
  ⚠ skipped — VINCI_SKIP_SMOKE=1 (offline validation).

✅ all test groups passed

2143 lines of output, 0 markers, and the new lane inside it at line 805:

805:package-first-party-tests: 19/19 checks passed (no first-party test path enters the release archive)

CI at this exact head — all 6 checks pass

build-check-test  pass  2m12s
check             pass  48s
offline (22)      pass  7m44s
offline (24)      pass  7m18s
sandbox-macos     pass  24s
upstream-suite    pass  2m17s

The claim above about GNU tar is not an inference — both ubuntu legs really ran the new lane. From the job logs:

offline (22)  Full Vinci test harness (Node 22)  2026-09-03T01:55:12Z  package-first-party-tests: 19/19 checks passed (no first-party test path enters the release archive)
offline (24)  Full Vinci test harness (Node 24)  2026-09-03T01:54:47Z  package-first-party-tests: 19/19 checks passed (no first-party test path enters the release archive)

So every listing in this body was reproduced on ubuntu/GNU tar as well as locally on bsdtar 3.5.3. check additionally builds and verifies a real artifact there.

The new lane plants files into the working tree and removes them in a finally; the run above left git status clean.


Typed record

loop_edge: package
live_producer: vinci/package.sh + vinci/scripts/package-excludes.mjs
live_consumer: the published tarball
side_effect_adapter: none
authority_class: ordinary
observation_receipt: required (build listing)
human_required: false

head_sha: 20957def4c86e82af43fdfec9f759306be9821ed
base_sha: 4af5f2b438d599abea8fe44cd353bd5cb0d0cb02

Requesting exact-head review at 20957def.

vinci/package.sh:30 promises that "tests, docs, infrastructure state, and
release tooling must never enter the public archive", and then tars
first-party directories wholesale. The exclude-list producer never
implemented the test half of that promise: its only test-shaped exclusion
is the hardcoded node_modules/ssh2/test carve-out, which governs a
dependency's own contents. The comment asserted a rule the code did not
enforce.

That was latent until #49, which put vinci/worker/test/economics.test.mjs
and economics-session.test.mjs on main. main's tar path list does not
carry vinci/worker, so nothing ships today; PR #50 adds vinci/worker to
that list, and from that moment both files ship. #50's checker verifies
dispatch targets are PRESENT and drives `vinci worker` to its usage
refusal, but has no unexpected-file or closure check, so they would ship
silently behind a green checker.

vinci/scripts/first-party-test-paths.mjs is taken byte-identical from PR
#48 (acaaae6), which carries the same predicate for the entry-list
producer, so #48 merges as a clean no-op rather than a conflict.

package-excludes.mjs now walks the repository and emits an exclusion for
every first-party test path it finds, keyed off path SHAPE rather than a
per-path enumeration. Because it walks the repository rather than
package.sh's tar list, it covers a root the moment that root is added:
#50 needs no change here and no change to package.sh.

The checker is deliberately untouched on this branch; see the PR body.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@thegeorgepu
thegeorgepu merged commit e85c5d0 into main Sep 3, 2026
6 checks passed
thegeorgepu pushed a commit that referenced this pull request Sep 3, 2026
…ker-inclusion

Brings the first-party test-path exclusion producer and the worker economics
modules under the tar path list added by this branch, so the interaction
between the two can be measured on a real artifact rather than simulated.
thegeorgepu pushed a commit that referenced this pull request Sep 3, 2026
…simulating it

package-first-party-tests.mjs could only ever simulate PR #50 before the base
was integrated: vinci/worker was not in package.sh's tar path list, so nothing
under it shipped and there was nothing to exclude. The file therefore copied
#50's tar block onto a disposable package.sh and asserted against that copy,
and it pinned the pre-#50 state with 'on main, vinci/worker is not tarred at
all'.

Merging origin/main (through PR #51, e85c5d0) puts #50's tar-list edit and
#51's shape-predicate exclusion in one tree for the first time, which makes
that scope assertion false and the simulation redundant. Both are replaced by
the same claims stated against the archive the real vinci/package.sh builds:
worker runtime ships (21 members under vinci/worker/), the two real test files
and their directory do not, and the whole-archive closure check still finds
nothing.

The simulation's prediction is reproduced exactly by the real artifact, so this
is a conversion of a prediction into a measurement, not a change of claim.
economics.mjs joins the runtime reachability control on purpose: it is the
runtime sibling of economics.test.mjs, so an exclusion that over-matched on the
name would drop it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
thegeorgepu added a commit that referenced this pull request Sep 3, 2026
…sion

Merged on George's standing instruction (2026-09-03). Independent exact-head review by lane projects-7f at cf196e6 (GO-WITH-NITS); merge held afterwards until the REAL merged tree was built, because the #51 interaction had only ever been verified by simulation.

The hold was justified by what the real build found. #51's test asserted against the real package.sh while its simulation asserted against a disposable copy, so on the merged tree the test went red -- it had pinned the pre-#50 state ("on main, vinci/worker is not tarred at all"). The simulation's PREDICTION reproduced exactly, so the artifact claim was correct and only the test around it was stale. Prediction converted to measurement.

Measured on the first-ever build of this merged state (47M, 18,669 members): no vinci/worker/test, no economics test files, no first-party .test./.spec. outside node_modules. All 21 members under vinci/worker present -- 20 files plus the contracts/ directory entry. Source truth is 23 tracked; the difference is exact and closed: README (this PR's explicit exclude) plus the two economics tests (#51's predicate). Nothing untracked shipped. Checker exit 0; 'vinci worker --help' reaches the worker usage line from the unpacked copy.

Discriminating controls: removing --exclude-from makes the worker test directory and both economics files ship; restoring it removes them while economics.mjs -- the runtime sibling of economics.test.mjs -- still ships, so no over-match. Dropping 'test' from the segment set fails on the directory assertion with all three planted probes still green, so nothing answered first.

Known and unchanged from the original review: releases do not ship from this repo. The workflow is guarded on getsimpledirect/vinci-code, whose vinci branch has no vinci/worker at all. So this is MERGED, not DEPLOYED, and it corrects tarballs built from this repo rather than anything a user installs.

Follow-up #52 adds fixtures/ and __snapshots__/ as confirmed no-op hardening; it also touches package-first-party-tests.mjs and will need a small merge behind this.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Mb5WmCTpEn6NVdXKUPVvKE
thegeorgepu added a commit that referenced this pull request Sep 3, 2026
Base integration after #50 (worker inclusion) and #51 (exclude-list predicate) landed.

One conflict, in vinci/test/package-first-party-tests.mjs, resolved as follows:

  KEPT from this branch: the plant-scoped cleanup (plantedFiles / createdDirectories).
  That is the branch's purpose -- the previous cleanup removed enclosing directories by
  NAME and recursively, which is correct only while those directories exist solely
  because the test created them. The day a real vinci/themes/specs holds real files, one
  run of this test deleted them, from a finally block that runs even on failure.

  DROPPED: every reference to the PR #50 simulation (simulationScript,
  package.pr50-simulation.tmp.sh). #50's lane retired the simulation when it merged,
  replacing it with assertions against the real archive, so main no longer defines or
  uses it. Keeping the cleanup line would have referenced an undefined binding.

Check count reconciles: 27 on this branch alone, minus the 2 retired simulation checks,
equals 25 on the merged tree. All 25 pass, including the real worker test files being
absent and all 21 worker runtime paths surviving.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AQrQYCTVyjgEdBCdbwEnWk
thegeorgepu added a commit that referenced this pull request Sep 3, 2026
…oes not have

The header claimed 'both the producer (package-entries.mjs) and the verifier
(packaged-artifact-check.mjs) import this one predicate'. On this branch neither does:
the only importer is package-excludes.mjs. git blame attributes those lines to #51; #52
extended the block and inherited the overclaim.

It is not merely stale. It asserts a two-sided arrangement as a safety property, and a
reader of main would conclude the verifier enforces this too. A header that asserts a
pairwise property the branch does not have is the defect this file exists to prevent,
one level up -- describes-is-not-binds, in the file whose whole purpose is that the
comment and the code agree.

Corrected to say what is true here (producer-only is complete, because this checker has
no closure requirement over first-party roots) and to name the entry-list arrangement as
the other configuration rather than as this one.

Raised by projects-7f in its #52 review. 28/28 checks unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AQrQYCTVyjgEdBCdbwEnWk
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