Skip to content

fix: add a files allowlist so packages stop shipping their tests - #434

Open
ianwremmel-ai-agent wants to merge 4 commits into
mainfrom
clc-1132-add-a-files-allowlist-so-packages-stop-shipping-their-tests
Open

ianwremmel-ai-agent wants to merge 4 commits into
mainfrom
clc-1132-add-a-files-allowlist-so-packages-stop-shipping-their-tests

Conversation

@ianwremmel-ai-agent

Copy link
Copy Markdown
Collaborator

Ticket: https://linear.app/code-like-a-carpenter/issue/CLC-1132/add-a-files-allowlist-so-packages-stop-shipping-their-tests

No package declares files, and the root .npmignore excludes only node_modules, so every tarball carries whatever happens to be on disk — test files, __snapshots__, tsconfig.json, and dist/tsconfig.tsbuildinfo.

This makes the @clc/nx:package-json executor write a files allowlist, and regenerates the 35 package.json files it owns.

@linear-code

linear-code Bot commented Sep 7, 2026

Copy link
Copy Markdown

CLC-1132

@ianwremmel-ai-agent

ianwremmel-ai-agent commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator Author

Plan

  • Confirm {"message":"Forbidden"} is the unrouted-stage response and that an unmatched path on a live stage answers differently (Missing Authentication Token), so a probe can tell the two apart. All three examples that emit an ApiUrl output (basic-wiring, aws-otel, aws-authorizer) leave the probe path undefined and override no MISSING_AUTHENTICATION_TOKEN gateway response.
  • Poll API_URL in jest.d/environments/example.ts loadEnv when TEST_ENV=aws, using @code-like-a-carpenter/wait-for, up to 60s.
  • CI green on the probe alone. The probe did not hold: on run 34372987406 the aws-authorizer shard got 200 for example 1 and 403 {"message":"Forbidden"} for example 2, 72ms later, with examples 3-7 passing. The stage was routable either side of the failing request, so a single probe on one connection cannot speak for the connections the tests open.
  • Retry the gateway's own Forbidden per request, in an Examples-only setup file that wraps fetch. Budget 3s, attempts a second apart.
  • Second adversarial review pass (spec-aware + spec-blind, different model families).
  • Lint, prettier, typecheck; predicates and the retry loop exercised against a local server.
  • CI green on 20ad69c: all five test-integration shards, plus build, lint, TSC, unit tests and both Codecov gates. Run 34376872049.
  • Copilot review — requested.
  • Operator review.

Review triage, round 2

Acted on:

  • The 3s retry budget was 5s and the deadline was checked only after sleeping, so a retried request that then cold-started could finish at ~10.3s and blow the 10s per-example timeout contract-tests sets — turning the Forbidden failure into a timeout. Budget cut to 3s and the deadline is now checked before sleeping: at most three attempts, the last starting 2s in.
  • basic-wiring's returns valid html test is a plain it(), so it ran on Jest's 5s default rather than the 10s buildContractTests passes. A 5.3s cold start alone can exceed that, before any retry. The Examples project now sets testTimeout: 30_000.
  • The probe treated any 403 with a non-JSON body as routable, so the HTML error page CloudFront serves for a stage it cannot reach would have released the tests immediately. Both the probe and the retry now match API Gateway's error envelope by shape — a lone message key — which also keeps the examples' own 403s (aws-authorizer adds type/name, basic-wiring adds name/requestIds) out of the retry.
  • The setup file read TEST_ENV from the sandbox copy, which configureEnvironment defaults to aws while the constructor defaults to localstack. A default local run would have retried 403s against localstack for 3s. The environment now publishes the value it acts on.
  • clone() tees the body, and the copy on the retry path was discarded unread. It is cancelled before sleeping.
  • The retry slept on the sandbox setTimeout, which fake timers would freeze into a hang. It captures the timer alongside fetch.
  • Comments claimed guarantees the code did not provide ("cannot outlast the retry budget", "stays well inside the 10s timeout"). Rewritten to state the actual bounds.

Dismissed:

  • "The diff reverts @clc/nx cliMain work and leaves cli-core/cli-plugin-example pointing at absent bin files." An artifact of the diff I handed the reviewers: main gained fix: stop setting bin on packages that have no cli.mjs #435/fix: repair CI on main: root tsconfig.json cache poisoning and the dead test-split step #436 after this branch forked, so a two-dot diff showed them reversed. git diff main...HEAD touches four files, all under jest.d/ and jest.config.ts.
  • "jest.d/ is outside the root tsconfig.json include, and nothing there can carry a unit test, so the predicate has neither." True on both counts and pre-existing for everything in jest.d/, including the environment itself. The alternative — moving an AWS test-harness predicate into a published package to buy coverage — puts the quirk somewhere worse. Typechecked here directly under nodenext, and the predicates and retry loop were exercised against a local server: CloudFront HTML, Missing Authentication Token, 5xx, both examples' real 403s, and the bare gateway body.
  • "{"message":"Forbidden"} is also API Gateway's answer for a missing API key, a resource-policy denial, and WAF blocks." None applies: no example declares an API key, a resource policy, or a WAF. A signature that broad would still only cost 3s per request and then fail on the real body.
  • "waitFor can start a probe just before the deadline, so the startup wait reaches ~70s, not 60s." Real, but it only happens when the stage genuinely is not up, and 70s per test file still sits well inside the 15 minute job timeout. The message now calls the 60s a budget.
  • Workflow annotation "Node.js 20 is deprecated … actions/setup-node@v4, nrwl/nx-set-shas@v4". Fires on every run on main, unrelated to this change.

Conflict resolution, 2026-09-09

main moved again when #435 (CLC-1131) merged, which conflicted in
packages/@clc/nx/src/create-nodes.mjs. Merged main into the branch
(f5f7ffa) and resolved by unioning both sides: main's {projectRoot}/cli.mjs
input and its comment, plus this branch's extraFiles option.

Regenerating produced exactly one delta — cli.mjs dropped out of files for
cli-core and cli-plugin-example. That is correct: CLC-1131 types a package
cli only when its package.json sets code-like-a-carpenter.cliMain: true, and
only @code-like-a-carpenter/cli does. It also closes the "Out of scope" note
above — those two packages no longer allowlist a cli.mjs that never existed.
A diff of every files array before and after the merge shows no other change.

Green on the merged tree: nx run-many --target build (451/451), tsc --build,
eslint, prettier, publint, and npm pack --dry-run per package.

@ianwremmel-ai-agent

Copy link
Copy Markdown
Collaborator Author

Pre-push review triage

Two adversarial passes on Codex (GPT-5.x), one spec-aware and one spec-blind. Two findings, both dismissed:

Yarn Classic ignores the ! negations (spec-blind, major as filed). Verified true — Codex ran yarn 1.22.22's own filter code and it keeps the test files. Dismissed because nothing packs these packages with Yarn Classic: @semrel-extra/npm delegates to @semantic-release/npm, whose lib/prepare.js:26 shells out to npm pack, so npm-packlist semantics are the ones that apply. The alternative — an explicit positive list — would have to enumerate every dist subdirectory and every non-test source file, and would rot the first time a package grows a directory.

extraFiles is inferred from disk, so an untracked schema.graphqls would change committed output (spec-aware, minor). Dismissed: create-nodes.mjs already infers mjs, mts and the .foundationrc extension with existsSync (lines 25, 26, 218-222), and an untracked src/index.mjs would rewrite the whole exports map by the same mechanism. Adding a per-project config just for these two paths would introduce a mechanism the repo doesn't otherwise have. CI's scripts/diff-lint catches any drift anyway.

Verification

Per package, against a staged copy with a synthetic version injected (packages carry none):

  • npm pack --dry-run file list diffed against today's. Across all 33 publishable packages nothing is dropped except tests, __snapshots__, tsconfig.json and *.tsbuildinfo, and nothing is added.
  • Every path named by exports, bin and executors.json is still in the tarball — including tools/*.json and the src/__generated__/*.shim.cjs implementations for the five .mjs tools, and dist/cjs/__generated__/foundation-executor.cjs for tool-foundation.
  • publint reports no internal test or config files. What remains is the require.types warning (CLC-1130) and pkg.bin is ./cli.mjs but the file does not exist for cli-core and cli-plugin-example (CLC-1131).

nx run-many --target=build (448 tasks, no cache), tsc --build, eslint and nx format:check all pass, and a second codegen:package run leaves the tree clean, so scripts/diff-lint is satisfied.

npm test fails identically on an unmodified main in this container — jest's TS config loader wants ts-node, which isn't installed. Not caused by this change.

@nx-cloud

nx-cloud Bot commented Sep 7, 2026

Copy link
Copy Markdown

View your CI Pipeline Execution ↗ for commit f5f7ffa

Command Status Duration Result
nx run-many --target build ✅ Succeeded 43s View ↗

💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗


☁️ Nx Cloud last updated this comment at 2026-09-09 17:14:40 UTC

@codecov

codecov Bot commented Sep 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 60.64%. Comparing base (d2a3396) to head (f5f7ffa).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #434   +/-   ##
=======================================
  Coverage   60.64%   60.64%           
=======================================
  Files          28       28           
  Lines        1601     1601           
  Branches      354      363    +9     
=======================================
  Hits          971      971           
  Misses        626      626           
  Partials        4        4           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ianwremmel-ai-agent

Copy link
Copy Markdown
Collaborator Author

Blocked on a red test-integration that this branch did not cause

All five test-integration shards fail, and they fail the same way on main.
Run 34163408910
is main at 5584eeb9 — a commit that does not contain this branch — and its
five shards die with the identical error. I re-ran the failed jobs on this head
(6eda895) once; they failed again at the same line, so it is not transient.

The chain:

  1. The check-run-reporter/action@v2.13.1 split step returns an error instead of
    a test list — '<p>An unexpected error occurred. Please see the console for more details.</p>', Request ID: undefined — so steps.split.outputs.tests
    is empty.
  2. scripts/crr-sam is then invoked with no arguments. Line 35 reads
    $testfile outside the for testfile in "$@" loop that sets it; under
    set -u that aborts with testfile: unbound variable and exit code 1.
  3. Because deploy never ran, no jest run happened, and the reporting step then
    reports Could not find any report files matching glob reports/junit/**/*.xml.

Nothing in this PR touches .github/ or scripts/. Fixing either the CRR
outage or the latent crr-sam bug is separate work — and fixing the bug alone
would not turn the job green, since with an empty test list
stack:list --as-projects returns every project and the shard would try to
deploy all of them.

Question for the operator: how do you want this handled?

  • merge with test-integration red, since it is red on main too;
  • hold this PR until the CI failure is fixed under its own ticket; or
  • fold a crr-sam fix into this PR (I'd rather not — different concern).

Everything else on this head is green: build, diff, lint, nopush,
fixup, test-unit, ESLint, TSC, Unit Tests, codecov/patch,
codecov/project, and both Socket checks. No merge conflicts, no review threads.
All actionable annotations are pre-existing: no-explicit-any warnings in
foundation-runtime/src/actions/unmarshall.ts, logger/src/console-logger.ts
and types/*.d.ts (none of which this diff touches), the Node 20 runner
deprecation, and the noise from the failing shards above.

I also re-verified the change itself locally on this head:
tool-inliner's tarball still carries executors.json, tools/inliner.json and
src/__generated__/inliner-executor.shim.cjs;
foundation-intermediate-representation still carries schema.graphqls and
foundation-plugin-cloudformation still carries json-schemas/; logger no
longer ships src/console-logger.test.ts; and no package ships tsconfig.json
or a .tsbuildinfo.

@ianwremmel-ai-agent

Copy link
Copy Markdown
Collaborator Author

Re-ran the five failed test-integration shards on this same head (6eda895)
at 22:08 UTC. Identical failure, so this is a third data point rather than a
momentary blip — retrying is not an answer:

##[error]Request ID: undefined
##[error]'<p>An unexpected error occurred. Please see the console for more details.</p>'
./scripts/crr-sam: line 35: testfile: unbound variable
##[error]Process completed with exit code 1.
##[error]Error: Could not find any report files matching glob reports/junit/**/*.xml

main has not moved since the red run I cited (still 5584eeb), and nothing
has touched scripts/crr-sam. The check-run-reporter split endpoint is still
returning an error instead of a test list, and that is outside this repo.

The three options in my previous comment stand unchanged. Still waiting on a
decision before this can go anywhere.

@ianwremmel-ai-agent

Copy link
Copy Markdown
Collaborator Author

The escalation has an answer: hold behind CLC-1138

My two previous comments asked whether to merge red, hold, or fold a crr-sam
fix in. That is now decided in the tracker rather than here — CLC-1138
"Replace the check-run-reporter split step in test-integration"

was filed for the CI outage, CLC-1132 was marked blocked by it, and #437 is
open against it. So: hold, and the fix lands in its own PR. I am not folding
anything into this one.

#437 is still draft and red, so nothing here can go green yet. When it merges I
will rebase this branch onto it and re-run.

Current state of this head (6eda895), unchanged:

  • Green: build, diff, lint, nopush, fixup, test-unit, ESLint,
    TSC, Unit Tests, codecov/patch, codecov/project, both Socket checks.
  • Red: the five test-integration shards, for the reason above.
  • No merge conflicts, no review threads.
  • All 15 actionable annotations dismissed with rationale: 10 pre-existing
    no-explicit-any warnings in types/*.d.ts,
    foundation-runtime/src/actions/unmarshall.ts and
    logger/src/console-logger.ts (none touched by this diff), 4 from the
    test-integration outage itself, and the Node 20 runner deprecation notice.

No package declared `files`, and the root `.npmignore` excludes only
`node_modules`, so every tarball carried whatever was on disk: test
files, `__snapshots__`, `tsconfig.json`, and `dist/tsconfig.tsbuildinfo`.

The `@clc/nx:package-json` executor now writes a `files` allowlist, and
the 35 package.json files it owns are regenerated.

`src` is published for every package rather than only the ten that serve
`./src/index.mjs` as their `import` condition. `tsconfig.references.json`
sets `declarationMap` and tsc does not inline sources, so every
`dist/types/*.d.ts.map` points at `../../src/*`; dropping `src` from the
`.ts`-source packages would leave those maps dangling. The tests inside
`src` are excluded by pattern instead.

Two packages publish a root asset that no project `type` implies:
`foundation-intermediate-representation/schema.graphqls`, which
`.graphqlrc.js` loads straight out of `node_modules`, and
`foundation-plugin-cloudformation/json-schemas`. A new `extraFiles`
option carries them; `create-nodes.mjs` detects them the same way it
already detects `src/index.mjs` and `.foundationrc.*`, so the value is
part of the nx cache key.

Verified per package against a staged copy with a synthetic version:
`npm pack --dry-run` drops nothing but tests, `__snapshots__`,
`tsconfig.json` and `*.tsbuildinfo`, and still lists every target named
by `exports`, `bin` and `executors.json`. `publint` no longer reports
internal test or config files for any package.
@ianwremmel-ai-agent
ianwremmel-ai-agent force-pushed the clc-1132-add-a-files-allowlist-so-packages-stop-shipping-their-tests branch from 6eda895 to 92a5333 Compare September 8, 2026 16:29
@ianwremmel-ai-agent

Copy link
Copy Markdown
Collaborator Author

Rebased onto current main, and the Done when criteria now run clean

main had moved ten commits (all dependabot) since this branch forked, and six
of them edit package.json files this executor generates. Rebased
6eda89592a5333; no conflicts, and the authored diff is byte-identical
across the rebase — only base blob hashes moved, so no new adversarial review
pass.

All three acceptance criteria, run against the rebased tree in a clean
npm ci checkout:

nx run-many --target=build --skip-nx-cache — exit 0.

publint — no package emits a "publishes internal tests or config files"
suggestion. Two warnings remain across the set, neither owned here:
exports["."].require.types resolving as ESM under require (CLC-1130 / #433),
and pkg.bin is ./cli.mjs but the file does not exist for cli-core and
cli-plugin-example (CLC-1131).

npm pack --dry-run — for each of the 32 publishable packages, every path
named by exports, bin, main/module/types, and by executors.json
(both its schema and its implementation) is still in the tarball, and no
tarball carries a *.test.*, a __snapshots__/ entry, a tsconfig.json, or a
*.tsbuildinfo. 30 packages pass outright.

The two that do not are cli-core and cli-plugin-example, both missing
cli.mjs. That file does not exist on disk in either package — their bin
points at nothing today, before this change, so no allowlist could have kept
it. @code-like-a-carpenter/cli, which does have a real cli.mjs, ships it.
This is the CLC-1131 bug, unchanged either way.

On the red test-integration

Unchanged and still not from this branch: the check-run-reporter split step
returns a 404, steps.split.outputs.tests comes back empty, and
scripts/crr-sam line 35 reads $testfile outside the loop that sets it —
set -u, exit 1. Run 34182043796 fails the same way on main at 3bff384.
#437 (CLC-1138) owns the fix and is still a draft.

One thing worth stating plainly, since the earlier park framed this as a merge
blocker: it is not one at the platform level. main's ruleset carries
pull_request with required_approving_review_count: 1 and no
required_status_checks rule, so a red test-integration does not itself
prevent a merge. Whether to wait for #437 or review this on its merits is an
operator call, not a platform constraint.

Yielding here rather than clearing draft — CI on this head is still red, and
that is the one gate this branch cannot move on its own.

@ianwremmel-ai-agent

Copy link
Copy Markdown
Collaborator Author

The hold behind CLC-1138 has no automatic end, so I am handing this back

Nothing about this branch has changed since the rebase to 92a5333. What has
become clear is that "wait for #437" is not a wait that resolves on its own.

The chain under this PR:

So the thing this PR waits for waits on a thing that waits on a human. Every
item in that chain is recorded as blocked on an operator in the dispatch graph.
Continuing to poll #434 does not shorten it.

What needs a decision

One of:

  1. Review and merge fix: repair CI on main: root tsconfig.json cache poisoning and the dead test-split step #436, then fix(ci): shard example tests in-repo instead of via check-run-reporter #437; test-integration goes green and
    I rebase this branch and finish it normally. This is the clean path.
  2. Approve fix: add a files allowlist so packages stop shipping their tests #434 on its merits. main's ruleset has
    required_approving_review_count: 1 and no required_status_checks
    rule, so the red test-integration is not a platform-level merge blocker —
    only a policy one. Every check this diff can affect is green.

I am not choosing between these; both are operator calls.

State of this branch, unchanged and complete

  • Implementation done and verified against all three Done when criteria on a
    clean npm ci checkout (see the 16:47 UTC comment).
  • Green: build, diff, lint, nopush, fixup, test-unit, ESLint,
    TSC, Unit Tests, codecov/patch, codecov/project, both Socket checks.
  • Red: the five test-integration shards, for the check-run-reporter 404 →
    crr-sam line 35 unbound variable reason documented above. Identical on
    main at 3bff384 (run 34182043796).
  • No merge conflicts, no open review threads, no unaddressed annotations.
  • Not yet done, and gated on the above: Copilot review, clearing draft,
    operator review.

The branch and its worktree stay in place. A resumed run picks this up from the
PR itself once one of the two paths above is taken.

…allowlist-so-packages-stop-shipping-their-tests
@ianwremmel-ai-agent

Copy link
Copy Markdown
Collaborator Author

Merged origin/main into the branch (279cdbf).

The five test-integration shards were failing for a reason unrelated to this
change. The branch was five commits behind main and still carried the old
scripts/crr-sam, whose line 35 read $testfile outside the loop that defines
it — fatal under set -euo pipefail:

./scripts/crr-sam: line 35: testfile: unbound variable

Main replaced that script in 9cb01ad. The branch now carries the fixed version
verbatim.

Because main had concurrently edited the two files this change touches
(create-nodes.mjs, package-json/executor.mjs) and added a workspace-refs
executor, I re-verified rather than trusting the clean textual merge:

  • nx run-many --target=codegen:package --skip-nx-cache over all 39 projects
    produces no diff, so the generated manifests are still current.
  • nx run-many --target=build: 451/451 tasks pass.
  • @clc/nx is private and unpublished, so the new executor needs no allowlist
    entry.

Two adversarial review passes ran on the merge result. Both findings are
pre-existing and dismissed:

  • create-nodes.mjs:221 resolves existsSync against the process cwd. This is
    the file's existing convention, shared with lines 66, 67, 260, 262 and 264,
    and the createNodes callback does not take a context to resolve against.
    Lines 66-67 pick mjs/mts, which drive every package's exports map, so a
    wrong cwd would already corrupt the output; regeneration producing no diff
    shows it does not. Changing it means refactoring five unrelated call sites.
  • cli-core and cli-plugin-example declare "bin": "./cli.mjs" for a file
    that does not exist. Present on main already, and the subject of CLC-1131.

Verification that the allowlist keeps what it must: npm-packlist run across
all 34 manifests retains every exports target, all seven tool shims and
foundation's compiled executor, and selects no path containing .test.,
__snapshots__, tsconfig or tsbuildinfo. The declarationMap rationale for
publishing src holds against real artifacts — logger/dist/types/index.d.ts.map
and env/dist/types/index.d.mts.map point at ../../src/* with no inlined
sources.

@ianwremmel
ianwremmel requested a lite review from Copilot September 9, 2026 15:54
@ianwremmel-ai-agent
ianwremmel-ai-agent marked this pull request as ready for review September 9, 2026 15:55
@ianwremmel-ai-agent

Copy link
Copy Markdown
Collaborator Author

Green and ready for review

Every check on 279cdbf passes: build, diff, lint, nopush, fixup,
test-unit, all five test-integration shards, ESLint, TSC, Unit Tests,
codecov/patch, codecov/project, both Socket checks.

test-integration (5, 2) (examples/basic-wiring) was red on the first
attempt: all 41 contract tests got API Gateway's own {"message":"Forbidden"}
body — including the 200 health check — 23 seconds after the REST API reached
CREATE_COMPLETE. That is
CLC-1156
(#443), a freshly created stage answering 403 until it propagates, and it hit
main at 1d0ecb2 the same way. Re-running the job passed with no code change.

No Copilot review: copilot-pull-request-reviewer[bot] is not among this
repo's assignable actors, and a REST reviewer request for it is accepted and
then dropped. Going straight to operator review.

What this changes

@clc/nx:package-json now writes a files allowlist, and the 34 publishable
manifests it owns are regenerated. src ships for every package, not just the
.mjs ones, because declarationMap is on and tsc does not inline sources —
dropping it would leave every dist/types/*.d.ts.map dangling. Tests,
__snapshots__ and *.tsbuildinfo are excluded by pattern instead. A new
extraFiles option carries the two package-root assets no type implies
(schema.graphqls, json-schemas/).

Verified against all three Done when criteria on a clean npm ci checkout:
publint emits no "publishes internal tests or config files" suggestion;
npm-packlist across all 34 manifests retains every exports target, all seven
tool shims and foundation's compiled executor, and selects nothing matching
.test., __snapshots__, tsconfig or tsbuildinfo; nx run-many --target=build passes 451/451.

@ianwremmel
ianwremmel self-requested a review September 9, 2026 15:55

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The changes consistently introduce a files allowlist via the generator/executor pathway and include the needed escape hatch for package-root schema assets without introducing behavioral risk beyond publish contents.

Pull request overview

Adds an npm publish allowlist (files) across the repo’s packages by teaching the @clc/nx:package-json executor to emit a consistent files list (and a small “extra files” escape hatch), then regenerating all managed package.json files so published tarballs stop including tests, snapshots, tsconfig files, and *.tsbuildinfo artifacts.

Changes:

  • Extend @clc/nx:package-json to write a files allowlist (plus extraFiles support) and exclude *.test.*, __snapshots__, and *.tsbuildinfo.
  • Update Nx node generation to pass extraFiles for packages that must publish package-root schema inputs.
  • Regenerate package manifests to include the new files allowlist (and required tool/cli ancillary files).
File summaries
File Description
packages/@code-like-a-carpenter/workbench-config/package.json Adds files allowlist to prevent publishing tests/build metadata.
packages/@code-like-a-carpenter/wait-for/package.json Adds files allowlist to prevent publishing tests/build metadata.
packages/@code-like-a-carpenter/tooling-common/package.json Adds files allowlist to prevent publishing tests/build metadata.
packages/@code-like-a-carpenter/tool-tool/package.json Adds files allowlist including Nx tool assets (executors.json, tools/).
packages/@code-like-a-carpenter/tool-stack/package.json Adds files allowlist including Nx tool assets (executors.json, tools/).
packages/@code-like-a-carpenter/tool-json-schema/package.json Adds files allowlist including Nx tool assets (executors.json, tools/).
packages/@code-like-a-carpenter/tool-inliner/package.json Adds files allowlist including Nx tool assets (executors.json, tools/).
packages/@code-like-a-carpenter/tool-foundation/package.json Adds files allowlist including Nx tool assets (executors.json, tools/).
packages/@code-like-a-carpenter/tool-deps/package.json Adds files allowlist including Nx tool assets (executors.json, tools/).
packages/@code-like-a-carpenter/telemetry/package.json Adds files allowlist to prevent publishing tests/build metadata.
packages/@code-like-a-carpenter/sentry/package.json Adds files allowlist to prevent publishing tests/build metadata.
packages/@code-like-a-carpenter/parallel/package.json Adds files allowlist to prevent publishing tests/build metadata.
packages/@code-like-a-carpenter/logger/package.json Adds files allowlist to prevent publishing tests/build metadata.
packages/@code-like-a-carpenter/lambda-handlers/package.json Adds files allowlist to prevent publishing tests/build metadata.
packages/@code-like-a-carpenter/interact/package.json Adds files allowlist to prevent publishing tests/build metadata.
packages/@code-like-a-carpenter/graphql-codegen-helpers/package.json Adds files allowlist to prevent publishing tests/build metadata.
packages/@code-like-a-carpenter/foundation-transform-environment/package.json Adds files allowlist to prevent publishing tests/build metadata.
packages/@code-like-a-carpenter/foundation-runtime/package.json Adds files allowlist to prevent publishing tests/build metadata.
packages/@code-like-a-carpenter/foundation-plugin-typescript/package.json Adds files allowlist to prevent publishing tests/build metadata.
packages/@code-like-a-carpenter/foundation-plugin-cloudformation/package.json Adds files allowlist including json-schemas/ for published schemas.
packages/@code-like-a-carpenter/foundation-parser/package.json Adds files allowlist to prevent publishing tests/build metadata.
packages/@code-like-a-carpenter/foundation-intermediate-representation/package.json Adds files allowlist including schema.graphqls in published output.
packages/@code-like-a-carpenter/exception/package.json Adds files allowlist to prevent publishing tests/build metadata.
packages/@code-like-a-carpenter/errors/package.json Adds files allowlist to prevent publishing tests/build metadata.
packages/@code-like-a-carpenter/env/package.json Adds files allowlist to prevent publishing tests/build metadata.
packages/@code-like-a-carpenter/dotenv/package.json Adds files allowlist to prevent publishing tests/build metadata.
packages/@code-like-a-carpenter/contract-tests/package.json Adds files allowlist to prevent publishing tests/build metadata.
packages/@code-like-a-carpenter/cli/package.json Adds files allowlist including cli.mjs for the published bin entrypoint.
packages/@code-like-a-carpenter/cli-plugin-example/package.json Adds files allowlist including cli.mjs for the published bin entrypoint.
packages/@code-like-a-carpenter/cli-core/package.json Adds files allowlist including cli.mjs for the published bin entrypoint.
packages/@code-like-a-carpenter/aws-env-loader/package.json Adds files allowlist to prevent publishing tests/build metadata.
packages/@code-like-a-carpenter/assert/package.json Adds files allowlist to prevent publishing tests/build metadata.
packages/@clc/test-helpers/package.json Adds files allowlist to prevent publishing tests/build metadata.
packages/@clc/dependencies/package.json Adds files allowlist to prevent publishing tests/build metadata.
packages/@clc/nx/src/create-nodes.mjs Computes/passes extraFiles so package-root schema inputs survive the allowlist.
packages/@clc/nx/executors/package-json/schema.json Adds extraFiles option to executor schema.
packages/@clc/nx/executors/package-json/schema.d.json.mts Adds extraFiles typing/docs to executor options interface.
packages/@clc/nx/executors/package-json/executor.mjs Writes pkg.files allowlist and supports extraFiles.
Review details
  • Files reviewed: 38/38 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…allowlist-so-packages-stop-shipping-their-tests

# Conflicts:
#	packages/@clc/nx/src/create-nodes.mjs
@ianwremmel-ai-agent

Copy link
Copy Markdown
Collaborator Author

Conflict with main resolved (f5f7ffa)

#435 (CLC-1131) landing conflicted in packages/@clc/nx/src/create-nodes.mjs.
Both sides of the hunk were additive and orthogonal, so the resolution is their
union: main's {projectRoot}/cli.mjs input and comment, plus this branch's
extraFiles option.

Regenerating the manifests produced exactly one delta — cli.mjs dropped out of
files for cli-core and cli-plugin-example. That is the correct result now
that CLC-1131 types a package cli only when it sets
code-like-a-carpenter.cliMain: true, which only @code-like-a-carpenter/cli
does. It also settles this PR's "out of scope" note: those two packages no
longer allowlist a cli.mjs that never existed on disk. Comparing every files
array before and after the merge shows no other change.

Green on the merged tree: nx run-many --target build (451/451), tsc --build,
eslint, prettier, publint, and npm pack --dry-run per package.

Pre-push review

Codex was rate-limited, so both adversarial passes ran on Claude instead of a
distinct model family. Flagging that rather than claiming a cross-family review
happened. Findings and triage:

Acted on: none — no finding was introduced by this diff.

Dismissed:

  • extraFiles not declared as a target input. Not a bug, and verified rather
    than argued: extraFiles is a target option, and nx hashes target options,
    so a change to it re-runs the task. cli.mjs needed a fileset input because
    its existsSync lives inside the executor, invisible to the options.
  • .tsbuildinfo might survive the allowlist if npm does not glob dotfiles.
    Moot — tsc writes dist/tsconfig.tsbuildinfo with no leading dot, and a pack
    of tool-deps ships zero tsbuildinfo, test, snapshot, or tsconfig entries.
  • json-schemas/ adds 4.45 MB to foundation-plugin-cloudformation. Deliberate:
    the allowlist preserves what that package publishes today, and trimming its
    published surface is a separate decision.
  • publint and the pack assertions are not enforced anywhere in CI. True, and
    that is what CLC-1135 is for.
  • effect should be affect in executor.mjs, and build:types declares an
    output path (dist/.tsconfig.tsbuildinfo) that never matches a real file.
    Both pre-existing on main and unrelated to this diff.

Worth a follow-up, not folded in here. Both passes independently flagged
that codegen:package does not list {projectRoot}/package.json in its
inputs, even though the executor reads that file and preserves description,
author, license, publishConfig, and dependencies from it. Because
explicit inputs replace nx's default {projectRoot}/**/*, a hand edit to any
preserved field does not change the task hash, so the next run restores the
cached file over it. Reproduced: deleting description from
@code-like-a-carpenter/assert/package.json and re-running codegen:package
hit the cache and put the description back, with the executor's own
assert(pkg.description, ...) never firing.

This is present on main — the same omission predates CLC-1131 — so it is not a
regression from this branch, and I have left it alone rather than expand an
approved diff. The sibling codegen:project-refs target already carries the fix
and a comment explaining exactly this hazard, so the repair is one line. Happy
to file it or fold it in, whichever you prefer.

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.

3 participants