Skip to content

test(cli): derive the packaged closure and install it offline - #1314

Merged
davidfarah2003 merged 3 commits into
mainfrom
fix/packaged-closure-completeness
Sep 6, 2026
Merged

test(cli): derive the packaged closure and install it offline#1314
davidfarah2003 merged 3 commits into
mainfrom
fix/packaged-closure-completeness

Conversation

@davidfarah2003

Copy link
Copy Markdown
Contributor

Refs #1305. Unblocks the 0.46.0 release PR and closes the mechanism behind it.

The defect

legacy-packaged-manager.smoke.ts packed a hand-written list of eight directories and installed them as a closure. bin/package.json declares "@cotal-ai/runtime": "workspace:*", and implementations/runtime was not in that list, so npm install had to resolve it from the registry.

  • On main the current version is published, npm fetches it, the install succeeds.
  • On a release PR the version being packed is by definition not yet published. chore(release): version packages #1305 fails here: AssertionError: installed current packaged closure 1 !== 0, taking shard 3/4 and 10 unstarted suites with it.

So every release PR would fail this suite, and the deeper problem is on main: the cell is named "installed current packaged closure" and has been passing while one member came from npm rather than from this build. A green there was partly an artifact of the previous release existing.

Two changes; the second is the one that generalises

1. The packed set is now DERIVED — the transitive workspace closure of cotal-ai, walked through workspace: dependency ranges.

This is not a tidier way to write the same list. It finds ten members where the hand-list had eight:

MISSED by the old hand-list:  implementations/runtime, packages/lang
in the old list but not in the closure:  (none)

I found implementations/runtime by inspection and missed packages/lang, which the derivation caught. A corrected hand-list would have shipped with the same class of defect it was correcting — which is the whole argument for deriving rather than editing.

2. The install now runs --offline.

Without it, a member missing from the closure is fetched from the registry and the install succeeds by luck. That luck is exactly what hid this. With it, any attempt to reach npm fails loudly and names the package.

Derivation fixes today's membership. --offline makes tomorrow's omission impossible to pass unnoticed — and there is already a tomorrow: #1312 (#783 M2) adds packages/seat and has implementations/manager declare "@cotal-ai/seat": "workspace:*". npm view @cotal-ai/seat version returns E404 — never published at any version — so unlike runtime and lang it has no registry copy to mask it, and it would red this suite on main rather than only on a release PR.

There is also a tarball-count assertion, so a pack that silently produces fewer archives than the closure has members fails with both numbers and the member list.

Verification

  • Derivation logic extracted and run standalone against the repo: 23 workspace packages discovered, 10-member closure, the two missing directories named above.
  • npm view confirms @cotal-ai/runtime and @cotal-ai/lang are both published at the current release, which is why neither was ever caught; and @cotal-ai/seat is E404.
  • Typecheck: this file contributes zero errors. The worktree reports 11 pre-existing errors from a stale dist, identical in count on untouched origin/main — measured both ways rather than assumed.
  • Not run end to end here: the suite needs an isolated broker and a full pack cycle, so CI is its first real execution. The --offline flag is the change most likely to surface something, since it converts any remaining registry dependency into a failure.

Scope note

Deliberately not addressed: the install uses --ignore-scripts, so it does not exercise any package's native build step. That is fine for this suite's question but means nothing in CI installs these packages the way a consumer will. Being handled separately on #1312.

The packed set was a hand-written list of eight directories. `bin`
declares `@cotal-ai/runtime` as a workspace dep and that package was not
in the list, so `npm install` resolved it from the registry. On main the
current version is published, npm fetches it, and the install succeeds —
so a cell named "installed current packaged closure" has been passing
while one member came from npm rather than from this build. The hole is
invisible until the version being packed is unpublished, which is
exactly a release PR: #1305 fails here at 1 !== 0.

Two changes, and the second is the one that generalises.

The set is now DERIVED as the transitive workspace closure of
`cotal-ai`. That is not a tidier way to write the same list: it finds
ten members where the hand-list had eight. I found `implementations/
runtime` by inspection and missed `packages/lang`, which the derivation
caught. A corrected hand-list would have shipped with the same class of
defect it was correcting.

The install now runs `--offline`. Without it, a member missing from the
closure is fetched from the registry and the install succeeds by luck,
which is precisely how this survived. With it, any registry reach fails
loudly and names the package. Derivation fixes today's membership;
offline makes tomorrow's omission impossible to pass unnoticed — and
there is already a tomorrow: #1312 adds `packages/seat`, which npm has
never published at any version.

Refs #1305
packed.txt was a shell redirect from deriving the closure by hand. It is
referenced by nothing, and its contents are the OLD eight-member list
without runtime or lang — the exact list this PR exists to replace. A
reader re-deriving from it would recover the defect being fixed.

It reached the branch through `git add -A` after a `> packed.txt` in a
scratch command. Found by review, not by me.
@davidfarah2003

Copy link
Copy Markdown
Contributor Author

BLOCK 32f98ed2c6c9133d92db9ce39ad58e2b05a5dfb9

The derivation is right. --offline as written will fail a complete closure, not only a missing member. Isolated HOME gives npm an empty $HOME/.npm, and the packed tarballs still have registry production deps. That is the load-bearing half of this PR, and it is inverted.

1. Derivation — verified independently

Walked bin/package.json and followed every workspace: range myself. Did not use the smoke's output.

  • Smoke-style public map: 23 packages (matches the author's count).
  • Transitive dependencies only, from cotal-ai: 10.
  • Old hand-list: 8. Extras: implementations/runtime, packages/lang. Nothing in the old list is outside the closure.
  • Depth-limited: depth ≤ 1 → 9 (runtime in, lang out). Depth ≤ 2 → 10. Depth 2 and depth N agree. The walk is actually transitive, not one hop.
  • devDependencies are not followed. Including them yields 17 (connectors, web, pi). Production-only is the right choice: npm install of published tarballs does not install devDependencies.
  • Cycles: needed.has(name) continues, so a cycle cannot hang. No workspace: cycles in the public graph today.
  • Missing package.json: swallowed by try/catch. 13 scan-depth dirs have none. Fine.

A corrected hand-list that added only runtime would still have missed lang. That claim holds.

Residual, not the block: discovery is two directory levels plus a bin special case. That matches today's layout. peerDependencies are not walked (connector-core peers @cotal-ai/core as >=0.1.0, not workspace:); core is already in via bin.

2. --offline — BLOCK

The suite sets HOME to a fresh temp dir, then:

npm install --offline --ignore-scripts --no-audit --no-fund …tarballs

npm's cache under that env is $HOME/.npm, empty. --offline is only-if-cached.

Probed, not inferred:

case result
empty cache, parent+child tarballs 0
empty cache, parent tarball only (missing workspace sibling) ENOTCACHED (this is the intended guard)
cache warmed by installing the sibling tarball, then parent-only --offline still ENOTCACHED (local tarball in cache is not a registry response)
empty cache, tarball whose production dep is registry ms ENOTCACHED ms
same tarball after an online install into that cache --offline 0, ms installed from cache
empty cache, optionalDependencies missing or uncached 0, optional skipped
real pnpm pack of packages/lang, then --offline under isolated HOME ENOTCACHED https://registry.npmjs.org/acorn

@cotal-ai/lang production deps: acorn, json-canonicalize, ses. The ten-member closure has 25 external production deps (acorn, @nats-io/*, react, zod, …). Optional @eplightning/nats-server-* will be skipped under --offline; that is not the failure.

So --offline does cover the original hole (a missing workspace member is not satisfied by a warmed tarball cache, and is not fetched). It also covers the path the old suite used successfully: third-party registry GETs. Isolated HOME means the runner's pnpm store and any operator ~/.npm are irrelevant. The current install runs before the online cotal-ai@0.42.0 install, so that cannot warm the cache either.

Net: a complete derived closure still fails installed current packaged closure with ENOTCACHED on acorn (or the next uncached production dep). This will red shard 3 on every PR, including the release it is meant to unblock.

The intended guard is "do not fetch @cotal-ai/* from the registry". --offline is "do not fetch anything". Those are not the same. A fix has to keep the first without imposing the second: local verdaccio / npm install of the tarballs without --offline plus an assertion that every @cotal-ai/* resolved from a file: tarball, or copy a warmed npm cache of only third-party packages into the isolated HOME, or drop HOME isolation for npm's cache while still isolating ~/.cotal. Do not ship --offline against an empty cache.

Tarball-count assertion: message includes both numbers and [...needed].join(", "). A short pack fails. Vacuous path: if cotal-ai is missing from the map, needed.size === 0 === tarballs.length and the count passes; then npm install --offline of zero tarballs can still exit 0. Not today's layout. Not the block.

Windows shard 3 listed pnpm smoke:legacy-packaged-manager and then FAILED at: pnpm smoke:mesh-attach-plane — this suite is in that shard's never-ran list. So Windows did not execute the new install. Linux shard 3 was still in_progress when this was posted; treat that as UNMEASURED, not green.

3. Scope and hygiene

  • packed.txt: addressed at this head. Present on 4270c55e with the old eight-member list; deleted by 32f98ed2. Absent in the worktree.
  • Body is Refs #1305. Grep of title, body, and both commit messages: no close(s|d)|fix(es|ed)|resolve(s|d) bound to a #number. "closes the mechanism behind it" has no issue number. Will not close chore(release): version packages #1305.
  • Changeset: none. House CI-only pattern is empty frontmatter (---\n--- as in mutation-reproof-gate.md). Absence does not bump the fixed group; it omits a CHANGELOG line. Justified as CI-only, still missing the empty record.
  • Typecheck: not measured locally (no node_modules; did not pnpm install). CI unit on this SHA is success, and that job runs pnpm typecheck then pnpm build. That is "typecheck is green on a clean CI tree", which is a different gate from "adds no new errors vs a stale local dist". Local 0-vs-11 was not reproduced here.

Conflict with #1312: noted, not graded.

4. CI on 32f98ed2c6c9133d92db9ce39ad58e2b05a5dfb9

Presence by name (separate from conclusion), via GET /repos/Cotal-AI/Cotal/actions/runs?head_sha=32f98ed2c6c9133d92db9ce39ad58e2b05a5dfb9:

workflow present status conclusion
CI yes (33993903126) in_progress UNMEASURED overall. unit success, live success, smoke shards 0–3 in_progress
Docs no path filter on docs.yml; this PR only touches the smoke file. Absent, not skipped-green
Mutation reproof yes (33993903145) completed success
Windows yes (33993903076) completed success (windows-ok). Sharded smoke is non-blocking; shard 3 never reached this suite

queued / in_progress counted as UNMEASURED. This suite's first real Linux execution is still in flight. The BLOCK does not wait on it: the install half already fails locally on a real lang tarball.

Ran vs read

Ran:

  • git rev-parse HEAD and gh pr view 1314 --json headRefOid — both 32f98ed2c6c9133d92db9ce39ad58e2b05a5dfb9
  • Independent Python BFS of workspace: deps (depth-limited, deps vs dev, cycles, missing package.json)
  • pnpm pack of packages/lang + npm install --offline under isolated HOME → ENOTCACHED acorn
  • Synthetic npm --offline matrix (empty cache, missing sibling, tarball-warmed cache, registry-warmed cache, optionalDeps)
  • Closing-keyword grep on PR body and commits
  • Workflow-runs and check-runs APIs on the full 40-char sha; Windows shard 3 log
  • MemAvailable ~8.1 GiB at publish; one probe at a time; no pnpm install

Did not run: the full smoke:legacy-packaged-manager (isolated broker + ten-pack cycle; author already said CI is first execution), pnpm typecheck / pnpm install, anything -live, pnpm check, pnpm cotal, mutation-proof. Did not pnpm install --frozen-lockfile.

Worktree clean. No node_modules.

…stry

`--offline` was the wrong guard. This fixture installs under an isolated HOME
whose npm cache starts empty, so forbidding registry access fails on legitimate
third-party dependencies as readily as on a workspace package that leaked to the
registry: `@cotal-ai/lang` needs `acorn`, and a complete, correct closure still
ENOTCACHEDs. It would have redded every run, including the release PR it exists
to unblock.

Assert the narrower property instead. After install, every workspace package
present in node_modules must resolve to a tarball this run packed; third-party
packages resolve from npm and stay silent. Membership is keyed on the workspace
set rather than an `@cotal-ai/` name prefix, because the entry point `cotal-ai`
carries no scope and a prefix test would exempt the package the closure is
rooted at. A missing `resolved` fails rather than passes.

A count assertion keeps the loop from going vacuous if npm reshapes the hidden
lockfile: zero matches would otherwise pass in silence.

Controls, run against a real pack and install rather than reasoned about:
dropping `@cotal-ai/lang` and dropping `@cotal-ai/runtime` each leave the
install succeeding (status 0, npm serves the omitted package from the registry)
and are caught only by the provenance assertion, which names the package and its
registry URL. The full closure passes with 10 of 10 checked.
@davidfarah2003

Copy link
Copy Markdown
Contributor Author

Block accepted. --offline is removed. Head 641f88773.

Your probe was right and the mechanism is the inverse of the one I guarded against. This fixture installs under an isolated HOME whose npm cache starts empty, so forbidding the registry fails on legitimate third-party dependencies exactly as readily as on a workspace package that leaked to it. Reproduced on the way in: packages/lang depends on acorn, json-canonicalize and ses, all from npm. A complete and correct closure still ENOTCACHEDs, so the guard would have redded every run including #1305.

What replaced it

After install, every workspace package present in node_modules must resolve to a tarball this run packed. Third-party packages resolve from npm and stay silent.

Membership is keyed on the workspace set rather than on an @cotal-ai/ name prefix. The entry point cotal-ai carries no scope, so a prefix test would have exempted the one package the closure is rooted at, which is the same shape as the hand-list defect this PR exists to fix.

Controls, against a real pack and a real install

POSITIVE      closure=10  packed=10  install.status=0   checked=10/10  failures=0
DROP lang     closure=10  packed=9   install.status=0   failures=1
    @cotal-ai/lang    resolved from https://registry.npmjs.org/@cotal-ai/lang/-/lang-0.45.0.tgz
DROP runtime  closure=10  packed=9   install.status=0   failures=1
    @cotal-ai/runtime resolved from https://registry.npmjs.org/@cotal-ai/runtime/-/runtime-0.45.0.tgz

install.status=0 in both drop cases is the point. npm serves the omitted package from the registry and the install succeeds, which is what main has been doing for @cotal-ai/runtime all along. Only the provenance assertion notices.

One addition beyond the remedy you specified

The loop asserts checked === needed.size. Without it the guard is vacuous the day npm moves the hidden lockfile or reshapes its keys: nothing matches, zero packages are checked, and the loop passes in silence. Say the word if you read that as scope creep and I will drop it.

A missing resolved also fails rather than passes. An unrecorded source is an unanswered question.

Verification and its limits

Typecheck: zero errors name the changed file. The repo-wide figure is 172 under the root tsconfig, which is not comparable to the 11 I quoted earlier from a narrower config; different denominators, and reporting them as a trend would be false. The file exports nothing and is referenced only by its package.json script entry, so a leaf's clean typecheck is sufficient.

Not run end to end locally: the suite needs an isolated broker and a full update cycle, so CI is still its first real execution.

The derivation, the tarball-count assertion and the packedTarballs set are unchanged from the version you already graded.

@davidfarah2003

Copy link
Copy Markdown
Contributor Author

APPROVE 641f88773ff18a14101a1b6e499b011e98fd4182

The --offline block on 32f98ed2 stands; this head replaces that guard with provenance, and the replacement holds under the case the author's drop-controls cannot separate.

1. Derivation

Unchanged from 32f98ed2. Earlier independent walk still applies: 10 members, extras implementations/runtime and packages/lang, depth-1 = 9 vs depth-2 = 10. Not re-derived.

2. Provenance guard — probed, not accepted

The loop walks node_modules/.package-lock.json. A workspace name (the public map, not an @cotal-ai/ prefix) must have resolved starting file: whose basename is in packedTarballs. Then checked === needed.size.

needed is the closure. The discriminating case is a member that is not installed at all:

case install.status checked inner failures smoke
POSITIVE 10/10 (live npm install of 10 tarballs) 0 10/10 0 PASS
DROP lang (live, 9 tarballs) 0 10/10 1 (https://registry.npmjs.org/@cotal-ai/lang/-/lang-0.45.0.tgz) FAIL inner
ABSENT lang (lock entry deleted, not installed) n/a 9/10 0 FAIL count
ABSENT runtime n/a 9/10 0 FAIL count
EMPTY / missing packages n/a 0/10 0 FAIL count
lock keys reshaped (drop node_modules/ prefix) n/a 0/10 0 FAIL count
lang resolved = registry URL n/a 10/10 1 FAIL inner
missing resolved n/a 10/10 1 FAIL inner
stale file: same basename, different dir n/a 10/10 0 PASS
stale file: different basename (0.44.0) n/a 10/10 1 FAIL inner

install.status=0 on DROP lang is the original defect: npm served @cotal-ai/lang@0.45.0 from the registry and the install succeeded. Only the inner assertion noticed. Matches the author's controls.

The gap the drop-controls cannot separate: a member missing from node_modules entirely. checked is incremented only for lock entries whose extracted name is in the public workspace map; the equality is against needed.size (the 10 the walk produced), not against how many workspace packages happen to be present. Deleting node_modules/@cotal-ai/lang from a real lock yields checked=9. The guard fires. It is not keyed off presence.

Vacuity: checked === needed.size fails when the hidden lock is empty, absent, or its keys no longer contain node_modules/<name>. Constructed, not reasoned.

This-run vs merely local: packedTarballs is basenames of this run's tarballs. A file: URL whose basename is not in that set fails. A file: URL whose basename is in the set but whose directory is some other tree passes. The suite always passes this run's absolute paths to npm install, so a leftover same-version tarball is not the defect under test. Residual, not a block: the asserted property is "local file named like one we packed", not "the exact path we packed".

cotal-ai (unscoped) is included; a prefix test would have exempted the root. lastIndexOf("node_modules/") would double-count a nested copy and then fail the count (fail-closed). The live lock had 176 entries, no nested @cotal-ai/*.

This worktree has no node_modules, so I could not pnpm pack (bin prepack and workspace: both need the install). Packs were npm pack --ignore-scripts of copies with workspace:* rewritten to 0.45.0 — the same post-pack shape. Live POSITIVE and DROP-lang installs used those tarballs under isolated HOME. Full suite (broker + ten pnpm pack + update) not run; Linux smoke on this sha is still queued.

3. Scope and hygiene

  • One file: implementations/cli/smoke/legacy-packaged-manager.smoke.ts. packed.txt absent.
  • Body still Refs #1305. Closing-keyword grep: none bound to a #number.
  • PR title and body are stale: they still describe --offline as the generalising half. The code no longer does that. Not a functional defect; it will mislead the next reader.
  • No changeset. CI-only, lockstep fixed group: costs a CHANGELOG line, not a bump. House pattern is empty frontmatter. Marginal, as previously noted.

4. CI on 641f88773ff18a14101a1b6e499b011e98fd4182

Workflow runs at the full 40-character sha (GET .../actions/runs?head_sha=641f88773ff18a14101a1b6e499b011e98fd4182):

workflow present status conclusion
CI yes (33997661521) queued UNMEASURED (unit/smoke/live all queued)
Docs no path filter; only the smoke file changed. Four workflows is complete
Mutation reproof yes (33997661524) completed success
Windows yes (33997661519) queued UNMEASURED

queued/pending/in_progress counted as UNMEASURED, never green. This suite's first Linux execution of the provenance guard has not started.

Ran vs read

Ran: checkout of 641f88773ff18a14101a1b6e499b011e98fd4182; live 10-tarball install + exact guard; live DROP-lang 9-tarball install (status 0, registry URL, inner fail); lock mutations for absent member, vacuity, reshape, stale basename. One install at a time. Reclaimed trees after each. SwapFree was ~42 MB at start and 60 kB during the positive install; did not start a second concurrent install.

Did not run: full smoke:legacy-packaged-manager, pnpm install, pnpm pack of bin, typecheck, anything -live, pnpm cotal, mutation-proof.

Worktree clean at 641f88773ff18a14101a1b6e499b011e98fd4182. No node_modules.

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