fix(ts): exclude test artifacts from package by maxmilian · Pull Request #3055 · strands-agents/harness-sdk

maxmilian · 2026-07-01T04:34:47Z

Description

This PR keeps generated test artifacts out of the published TypeScript npm package.

The TypeScript build still uses the existing src/tsconfig.json, but prepack now prunes generated __tests__, __fixtures__, and compiled .test.* artifacts from dist before npm assembles the tarball. postpack rebuilds dist afterward so local project-reference type checking is not left with missing declaration outputs.

I also wired a package contents assertion into test:package so future changes fail if test artifacts re-enter the dry-run tarball.

Related Issues

Resolves #3004

Documentation PR

No documentation changes needed.

Type of Change

Bug fix

Testing

npm run test:package -w strands-ts
npm run type-check -w strands-ts
npm run lint -w strands-ts
npm run format:check -w strands-ts
I ran hatch run prepare

Checklist

I have read the CONTRIBUTING document
I have reviewed and understand every line of code in this PR, including any generated by AI tools, and I can explain why it works
My change is focused and reasonably small; I have split unrelated work into separate PRs
I have added any necessary tests that prove my fix is effective or my feature works
I have updated the documentation accordingly
I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
My changes generate no new warnings
Any dependent changes have been merged and published

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

github-actions · 2026-07-02T02:36:10Z

+}
+
+const packOutput = JSON.parse(chunks.join(''))
+const files = packOutput[0]?.files?.map((file) => file.path) ?? []


Issue: The ?? [] fallback lets the guard pass vacuously. If npm pack --json ever emits an unexpected shape, or the piped stdout gets corrupted (note npm pack --dry-run runs prepack/postpack, and any stdout they emit is prepended to this JSON), files becomes [], forbiddenFiles is empty, and the script prints OK (0 files) — a green check that verified nothing. That's the failure mode a regression guard most needs to avoid.

Suggestion: Assert the payload is non-empty and contains the expected entrypoint before checking for forbidden files, e.g.:

if (files.length === 0 || !files.includes('dist/src/index.js')) { throw new Error(`Unexpected npm pack output; got ${files.length} files`) }

so a malformed/empty payload fails loudly instead of passing silently.

github-actions · 2026-07-02T02:36:12Z

Assessment: Request Changes

The fix works and is runtime-safe (verified: no production code imports from __tests__/__fixtures__, all test/fixture files are properly co-located, and the prune regexes are well-anchored with no false positives). My main concern is that it solves a declarative packaging problem with imperative machinery — a build that emits test artifacts, a script that deletes them, and a postpack full rebuild to undo the prune — when npm's files allowlist can express the exclusion directly.

Review themes

Simplicity / approach: The build → prune → rebuild cycle plus custom script can likely be replaced by two files negation patterns (verified on npm 10). See the package.json comment.
Regression-guard robustness: The contents assertion can pass vacuously on an empty/malformed pack payload — it should fail loudly instead.
Maintainability: Forbidden-pattern logic is duplicated across the prune and assert scripts and must be kept in sync (moot if the files approach is adopted).
Conventions: The new regression guard should link [BUG] __tests__ and __fixtures__ are included in published NPM package #3004 per AGENTS.md.

Nice touch wiring the contents check into test:package so this can't silently regress again.

yonib05

The fix works on the happy path, but the prune/rebuild mechanism is more fragile than the problem needs. npm's files field supports negation patterns, so this can be:

"files": ["dist", "!dist/**/__tests__", "!dist/**/__fixtures__", "!dist/**/*.test.*", "README.md", "LICENSE"]

Same tarball, no prepack prune, no postpack rebuild, no dist mutation. Keep the tarball assertion in test:package as the regression check. Alternatively, stop compiling tests into dist with a build tsconfig that excludes them; nothing consumes dist/tests (vitest runs from src).

Either of those makes most of the inline comments below moot.

One more small thing: lint and format only cover src test/integ, so the two new scripts are not checked by either.

yonib05 · 2026-07-02T03:17:23Z

+}
+
+const packOutput = JSON.parse(chunks.join(''))
+const files = packOutput[0]?.files?.map((file) => file.path) ?? []


This fails open. If prepack fails, npm pack --dry-run --json prints {"error":{...}} on stdout and exits 1, but the pipe's exit code comes from this script, and ?.files ?? [] turns the error object into an empty list. Verified locally: a failing prepack prints [package-contents] OK (0 files) and exits 0, so a broken build passes the check.

Fail if the parsed output has an error key, and assert files.length > 0.

yonib05 · 2026-07-02T03:17:23Z

    "build": "tsc --project src/tsconfig.json",
-    "prepack": "npm run build",
+    "prepack": "npm run build && node scripts/prune-package-artifacts.js",
+    "postpack": "rm -rf dist && npm run build",


rm -rf doesn't exist on Windows (npm runs lifecycle scripts under cmd.exe), and CI runs test:package on windows-latest. postpack fails on every pack there, and the fail-open pipe in test:package hides it, so the package check never actually runs on the Windows matrix jobs.

yonib05 · 2026-07-02T03:17:23Z

  "scripts": {
    "build": "tsc --project src/tsconfig.json",
-    "prepack": "npm run build",
+    "prepack": "npm run build && node scripts/prune-package-artifacts.js",


If pack fails or is interrupted after prepack, postpack never runs and dist stays pruned. Since the build is composite/incremental and the tsbuildinfo still lists the deleted files as emitted, the next npm run build is a no-op and doesn't restore them. Only a manual clean recovers.

yonib05 · 2026-07-02T03:17:23Z

    "test:all": "vitest run --project unit-node --project unit-browser",
    "test:all:coverage": "vitest run --coverage --project unit-node --project unit-browser",
-    "test:package": "cd test/packages/esm-module && npm install && node esm.js && cd ../cjs-module && npm install && node cjs.js",
+    "test:package": "npm run build && cd test/packages/esm-module && npm install && node esm.js && cd ../cjs-module && npm install && node cjs.js && cd ../../.. && npm pack --dry-run --json | node test/packages/assert-package-contents.js",


This now runs three full tsc builds per invocation: the leading npm run build, prepack's build, and postpack's rebuild (which is a cold build because rm -rf dist deletes the tsbuildinfo). CI already builds right before test:package, so every matrix job pays for three redundant compiles.

yonib05 · 2026-07-02T03:17:23Z

+import { join } from 'node:path'
+
+const distDir = new URL('../dist/', import.meta.url)
+const testFilePattern = /\.test(?:\.[^/.]+)*\.(?:js|d\.ts)(?:\.map)?$/


This regex and the __tests__/__fixtures__ names are duplicated in assert-package-contents.js. If one side is updated and the other isn't, either test files ship with a green check or packing breaks. Put the patterns in one shared place.

yonib05 · 2026-07-02T03:17:23Z

+  chunks.push(chunk)
+}
+
+const packOutput = JSON.parse(chunks.join(''))


Lifecycle script stdout gets interleaved before the JSON on npm's stdout (only the > pkg script banners go to stderr). This works today only because tsc and the prune script print nothing on success; any future console.log in prepack breaks JSON.parse here with a confusing error.

Use npm files-field negation patterns to exclude test artifacts from the tarball declaratively, dropping the prepack prune script and the postpack rebuild. Removes the Windows rm -rf failure, the interrupted-pack dist corruption, and the triple tsc build. The strands-agents#3004 regression guard now fails loudly on an empty/errored/unparseable pack payload instead of passing vacuously. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

maxmilian · 2026-07-02T09:55:10Z

Thanks for the review @yonib05 — adopted the files-negation approach in 6e6de70. Much simpler, and it moots most of the inline comments:

files negation — dist now carries !dist/**/__tests__, !dist/**/__fixtures__, !dist/**/*.test.*. Verified against a built dist (which does contain those artifacts): npm pack --dry-run --json yields 0 forbidden paths and 741 legit files.
Dropped postpack entirely and reduced prepack to just npm run build. That removes the Windows rm -rf failure, the interrupted-pack dist corruption, and the cold triple-build — all gone with the machinery.
Deleted prune-package-artifacts.js, so the duplicated forbidden-pattern regex now lives in exactly one place (the assert guard).
Fixed the fail-open guard — assert-package-contents.js now throws on an empty stdin, an unparseable payload, a {"error":...} result, or an empty file list, instead of printing OK (0 files). It also links [BUG] __tests__ and __fixtures__ are included in published NPM package #3004 per AGENTS.md.

npm run test:package passes end-to-end (esm + cjs module checks, pack, assert).

On the last point — lint/format only covering src test/integ: with the prune script deleted, the only remaining new file is the assert guard under test/packages/. I left the lint globs alone rather than pull the esm-module/cjs-module fixtures into scope, but ran prettier --check on the guard manually (clean). Happy to widen the globs to test/packages/*.js if you'd prefer it enforced.

fix(ts): exclude test artifacts from package

3023db8

github-actions Bot added size/s typescript Pull requests that update typescript code area-community Related to community and contributor health bug Something isn't working strands-running labels Jul 1, 2026

yonib05 self-requested a review July 2, 2026 02:31

github-actions Bot reviewed Jul 2, 2026

View reviewed changes

github-actions Bot removed the strands-running label Jul 2, 2026

yonib05 reviewed Jul 2, 2026

View reviewed changes

yonib05 requested changes Jul 2, 2026

View reviewed changes

maxmilian force-pushed the agent-tasks/3004 branch from 47cd960 to 6e6de70 Compare July 2, 2026 09:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(ts): exclude test artifacts from package#3055

fix(ts): exclude test artifacts from package#3055
maxmilian wants to merge 2 commits into
strands-agents:mainfrom
maxmilian:agent-tasks/3004

maxmilian commented Jul 1, 2026

Uh oh!

github-actions Bot Jul 2, 2026

Uh oh!

github-actions Bot commented Jul 2, 2026

Uh oh!

yonib05 left a comment

Uh oh!

yonib05 Jul 2, 2026

Uh oh!

yonib05 Jul 2, 2026

Uh oh!

yonib05 Jul 2, 2026

Uh oh!

yonib05 Jul 2, 2026

Uh oh!

yonib05 Jul 2, 2026

Uh oh!

yonib05 Jul 2, 2026

Uh oh!

maxmilian commented Jul 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

maxmilian commented Jul 1, 2026

Description

Related Issues

Documentation PR

Type of Change

Testing

Checklist

Uh oh!

github-actions Bot Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jul 2, 2026

Uh oh!

yonib05 left a comment

Choose a reason for hiding this comment

Uh oh!

yonib05 Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

yonib05 Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

yonib05 Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

yonib05 Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

yonib05 Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

yonib05 Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

maxmilian commented Jul 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants