Skip to content

chore(lambda): publish-readiness for @hyperframes/aws-lambda#920

Merged
jrusso1020 merged 4 commits into
mainfrom
chore/aws-lambda-publish-ready
May 17, 2026
Merged

chore(lambda): publish-readiness for @hyperframes/aws-lambda#920
jrusso1020 merged 4 commits into
mainfrom
chore/aws-lambda-publish-ready

Conversation

@jrusso1020
Copy link
Copy Markdown
Collaborator

@jrusso1020 jrusso1020 commented May 17, 2026

What

Wire @hyperframes/aws-lambda to publish to npm alongside the other @hyperframes/* packages on the next v* tag. The Lambda adapter has been on main since #909 but its package manifest still shipped raw TypeScript source (main: ./src/index.ts, build: tsc --noEmit, version: 0.0.1) and the publish workflow didn't list it.

Why

External adopters can't npm install @hyperframes/aws-lambda until the package both (a) publishes and (b) ships compiled JS + types. The CLI already shows a friendly install hint when the lambda subverbs can't find it, but that hint resolves to nothing without a published package.

How

  • packages/aws-lambda/build.mjs (new) mirrors packages/producer/build.mjs: esbuild bundles src/{index,handler,sdk/index,cdk/index}.tsdist/, then tsc --emitDeclarationOnly emits .d.ts. All runtime/peer deps (@aws-sdk/*, @hyperframes/producer*, @sparticuz/chromium, aws-cdk-lib, constructs, ffmpeg-static, ffprobe-static, puppeteer-core, tar) are external so consumers resolve them through their own node_modules.
  • tsconfig.build.json (new) strips the workspace paths overrides so @hyperframes/producer* resolves through node_modules to producer's already-built dist/ types instead of pulling its full source tree into emit (which would violate rootDir).
  • tsconfig.json keeps noEmit: true + workspace paths for fast in-place typechecks; excludes src/**/__fixtures__/** so test-only helpers (fakeS3) don't leak into emitted declarations.
  • package.json sets version to 0.6.20 to align with current main's lockstep release; points main / types / exports at ./dist/...; sets files: ["dist/", "scripts/", "README.md"] (the whole scripts/ directory is kept because build-zip.ts and verify-zip-size.ts both import scripts/_formatBytes.ts); and sets scripts.build = node build.mjs.
  • Root package.json adds aws-lambda to the build filter so bun run build builds it in topological order after producer.
  • scripts/set-version.ts adds packages/aws-lambda to the PACKAGES list so future chore: release vX.Y.Z commits bump aws-lambda along with the other publishable packages instead of leaving it frozen.
  • .github/workflows/publish.yml gets one new publish_pkg "@hyperframes/aws-lambda" "@hyperframes/aws-lambda" line. First publish is automatic via the --access public flag in publish_pkg; the @hyperframes scope already owns the name.

Breaking the producer ↔ aws-lambda build cycle

Moving aws-lambda's types from ./src/index.ts./dist/index.d.ts exposed a circular workspace build dependency: producer's regression-harness-lambda-local.ts imports @hyperframes/aws-lambda, and aws-lambda imports @hyperframes/producer/distributed. Neither side's tsc emit pass can run cleanly without the other side's dist/ already in place, and bun's --filter can't topologically order them out of the cycle.

Three commits stacked on top of the publish-readiness wiring break that cycle:

  • regression-harness-lambda-local-types.ts (new) — extracts the public surface (RunLambdaLocalInput, RunLambdaLocalRender) into a no-aws-lambda-imports file. regression-harness.ts uses these types instead of typeof import(...).
  • regression-harness.ts — the dynamic await import("./regression-harness-lambda-local.js") is routed through a top-level const LAMBDA_LOCAL_MODULE so tsc can't statically resolve the target path. Without this indirection tsc walks into the file via the static-string dynamic import even though tsconfig.exclude nominally hides it (TS docs: "TypeScript will always include files referenced by import statements... regardless of exclude"). tsx resolves the path normally at runtime so --mode=lambda-local is unchanged.
  • packages/producer/tsconfig.json — adds src/regression-harness-lambda-local.ts to exclude so even if tsc somehow reached it, it wouldn't emit declarations for it.

Together these mean producer's tsc never tries to resolve @hyperframes/aws-lambda at emit time, which lets the package build cleanly even before aws-lambda's dist/ exists.

Test plan

bun run build                          # ✅ full root build green, aws-lambda builds after producer
bun run verify:packed-manifests        # ✅ all packages including aws-lambda publish-safe
cd packages/cli && pnpm pack            # ✅ @hyperframes/aws-lambda rewrites workspace:* → 0.6.20
cd packages/cli && npm pack && \
  npm install -g <tgz>                 # ✅ global install smoke flow still works
hyperframes lambda deploy              # ✅ friendly missing-package error still fires when
                                       #    aws-lambda isn't installed alongside the CLI

CI status as of 187dfd5b: required jobs (Build, Typecheck, CLI smoke) all green. Regression shards inherit from main / are running.

Post-merge: manual first publish

The package isn't on npm yet, so Trusted Publisher can't be pre-configured (npm doesn't surface a "register a not-yet-existing package" path in its UI). Plan after merge: pnpm publish --access public from a clean main checkout to create @hyperframes/aws-lambda@0.6.20, then wire up Trusted Publisher on the now-existing package, then CI takes over for v0.6.21+.

  • Build / typecheck green on the PR head
  • Packed manifest is publish-safe (no workspace:* refs)
  • CLI global-install smoke flow unaffected
  • scripts/set-version.ts covers aws-lambda for future lockstep bumps
  • Unit tests added/updated — N/A (CI/build change)
  • Documentation updated — N/A (deploy guide already in docs(lambda): add docs/deploy/aws-lambda.mdx deployment guide #914)

The Lambda adapter has been on `main` since PR #909 but its package
manifest still shipped TypeScript source (`main: ./src/index.ts`,
`build: tsc --noEmit`, `version: 0.0.1`) and the publish workflow
didn't list it. This wires it up to publish alongside the other
`@hyperframes/*` packages on the next `v*` tag.

Changes:

  - **packages/aws-lambda/build.mjs (new)** — mirrors
    `packages/producer/build.mjs`: esbuild bundles four entry
    points (`src/index.ts`, `src/handler.ts`, `src/sdk/index.ts`,
    `src/cdk/index.ts`) → `dist/`, then `tsc --emitDeclarationOnly`
    emits .d.ts via `tsconfig.build.json`. All runtime/peer deps
    (@aws-sdk/*, @hyperframes/producer*, @sparticuz/chromium,
    aws-cdk-lib, constructs, ffmpeg-static, ffprobe-static,
    puppeteer-core, tar) are external so consumers resolve them
    through their own node_modules.

  - **packages/aws-lambda/tsconfig.build.json (new)** — drops the
    workspace `paths` overrides so `@hyperframes/producer*`
    resolves through node_modules to producer's already-built
    `dist/` types instead of pulling its full source tree into
    emit (which would violate `rootDir`).

  - **packages/aws-lambda/tsconfig.json** — keeps `noEmit: true`
    + workspace `paths` for fast in-place typechecks; also
    excludes `src/**/__fixtures__/**` so test-only helpers
    (fakeS3) don't leak into emitted declarations.

  - **packages/aws-lambda/package.json**:
      * version bumped 0.0.1 → 0.6.18 (matches the repo's lockstep
        release cadence)
      * main / types / exports map points at `dist/...`
      * files: ["dist/", "scripts/", "README.md"] (scripts/ kept
        whole because build-zip.ts and verify-zip-size.ts both
        import scripts/_formatBytes.ts)
      * scripts.build = `node build.mjs`

  - **package.json** — root `build` filter includes
    `aws-lambda` so `bun run build` builds it in topological
    order after producer.

  - **.github/workflows/publish.yml** — one new
    `publish_pkg "@hyperframes/aws-lambda" "@hyperframes/aws-lambda"`
    line. First publish is automatic via the `--access public` flag
    in `publish_pkg`; the @hyperframes scope already owns the name.

Verification:

  bun run build                          # full root build green
  bun run verify:packed-manifests        # aws-lambda passes
  pnpm pack packages/cli                 # @hyperframes/aws-lambda
                                         # rewrites workspace:* → 0.6.18
  npm install -g <cli-tgz>               # smoke-install still works
  hyperframes lambda deploy              # friendly missing-package
                                         # error still fires when
                                         # aws-lambda isn't installed
… harness

The aws-lambda publish-readiness changes earlier in this PR moved
aws-lambda's types from `./src/index.ts` → `./dist/index.d.ts`. That
flipped producer's emit pass from "always works" to "needs aws-lambda
built first," because producer's `regression-harness-lambda-local.ts`
imports `@hyperframes/aws-lambda{,/handler}`. aws-lambda's own emit
pass in turn imports `@hyperframes/producer{,/distributed}` → circular
build dependency, so neither side can emit declarations in a single
pass. CI's first run on this PR failed Build, Typecheck, CLI smoke,
windows tests, and every perf job for this reason.

Fix: extract the public surface of `regression-harness-lambda-local.ts`
into a new types-only file that has no aws-lambda imports, point
`regression-harness.ts` at that types file, and exclude
`regression-harness-lambda-local.ts` from producer's tsconfig
`include`. The implementation file is still loaded at runtime via
`tsx` (lambda-local mode runs the harness through tsx, not from
producer's dist/), so the runtime contract is unchanged — producer's
tsc just no longer has to type-check it.

  - `regression-harness-lambda-local-types.ts` (new): exports
    `RunLambdaLocalInput` + `RunLambdaLocalRender` with zero
    aws-lambda imports.
  - `regression-harness-lambda-local.ts`: re-exports
    `RunLambdaLocalInput` from the new types file (so the public
    name stays stable for any future direct imports).
  - `regression-harness.ts`: drops the `typeof import("...")` trick
    and uses the explicit `RunLambdaLocalRender` signature for the
    dynamically-loaded function.
  - `tsconfig.json`: excludes `src/regression-harness-lambda-local.ts`
    so producer's emit pass never resolves `@hyperframes/aws-lambda`.

Verified:

  bun run build                   # full root build green
  bun run verify:packed-manifests # all packages publish-safe
Two follow-ups to keep the new package in lockstep with the rest of the
@hyperframes/* release cadence from day one:

- Bump packages/aws-lambda/package.json version 0.6.18 → 0.6.20 so it
  matches what main released while this PR was in review. Without this,
  the package would land below the rest of the lockstep and the next
  release-bump would jump aws-lambda from 0.6.18 → 0.6.21 in one step.

- Add packages/aws-lambda to PACKAGES in scripts/set-version.ts so the
  next `chore: release vX.Y.Z` commit bumps aws-lambda alongside the
  other publishable packages. Without this, set-version silently skips
  aws-lambda — package.json stays frozen, pnpm publish would re-publish
  the same version on every release, and the npm-view precheck in
  publish.yml would skip-with-success and never actually push a new
  version of the package.
The tsconfig `exclude` list isn't enough to keep producer's tsc emit pass
from pulling `regression-harness-lambda-local.ts` (and its
`@hyperframes/aws-lambda` static imports) into the program — tsc still
statically resolves the path in
`await import("./regression-harness-lambda-local.js")` from
`regression-harness.ts`, walks into the excluded file, and fails on
the missing aws-lambda type declarations.

Reproduction (clean workspace, no aws-lambda dist yet, mirrors CI):

  rm -rf packages/{aws-lambda,producer,core}/dist
  bun run build
  # @hyperframes/producer build: src/regression-harness-lambda-local.ts(36,70):
  # error TS2307: Cannot find module '@hyperframes/aws-lambda' or its
  # corresponding type declarations.

Fix: route the dynamic import path through a top-level string constant
so tsc can't statically resolve the target. tsc keeps the type-only
imports (`RunLambdaLocalRender` from the no-aws-lambda types file) and
treats the dynamic-import target as opaque. `tsx` resolves the path
normally at runtime, so `--mode=lambda-local` is unchanged.
Copy link
Copy Markdown
Collaborator

@vanceingalls vanceingalls left a comment

Choose a reason for hiding this comment

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

Build wiring for the new @hyperframes/aws-lambda publish. Strong overall structure — the build.mjs+tsconfig.build.json pattern mirrors producer cleanly, the subpath barrel split is well thought out, the types-only file (regression-harness-lambda-local-types.ts) is a smart way to break a static type cycle. But the PR ships with three required CI checks (Build, Typecheck, CLI smoke (required)) failing on the same root cause, and the failure mode is real, not flaky.

Strengths

  • packages/aws-lambda/build.mjs:24-37 — the external: list looks complete: every runtime/peer dep in package.json is mirrored, including @hyperframes/producer/distributed. Consumers' bundlers will resolve through node_modules cleanly.
  • packages/aws-lambda/tsconfig.build.json:4"paths": {} to strip the workspace alias is exactly right for the emit pass and the comment in build.mjs:55-59 explains why (rootDir violation otherwise). Future contributors will need that note.
  • packages/aws-lambda/package.json:31 — keeping the entire scripts/ directory rather than the spec'd file list, with the inline rationale (_formatBytes.ts shared between build-zip.ts and verify-zip-size.ts), is the right call. Trimming would have silently broken runtime.

Blockers

  1. blockerRequired CI failing on a real build-ordering bug, .github/workflows/ci.yml Build / Typecheck / CLI smoke (required) jobs all red. The error is reproducible across all three:

    @hyperframes/producer build: src/regression-harness-lambda-local.ts(36,70): error TS2307:
      Cannot find module '@hyperframes/aws-lambda' or its corresponding type declarations.
    

    Root cause: package.json:14 runs bun run --filter '@hyperframes/{core,engine,producer,...,aws-lambda}' build. Producer's tsc emit pass (packages/producer/build.mjs tail — tsc --emitDeclarationOnly) needs packages/aws-lambda/dist/index.d.ts. The PR moved aws-lambda's main/types from ./src/index.ts to ./dist/... (packages/aws-lambda/package.json:16-17), so the .d.ts doesn't exist until aws-lambda's build runs. But producer↔aws-lambda is a circular workspace dep (producer devDeps on aws-lambda, aws-lambda deps on producer), so bun's --filter topological order can't put aws-lambda before producer. The CI logs show aws-lambda finishing build after producer's tsc fails (aws-lambda exits code 0 at T+~40s, producer's tsc fails at T+~34s).

    The PR's "split types out" mitigation (packages/producer/src/regression-harness-lambda-local-types.ts + the import type { RunLambdaLocalRender } in regression-harness.ts:51) doesn't fully solve this: regression-harness.ts:52 still has await import("./regression-harness-lambda-local.js"), and TypeScript's exclude is a soft list — files reachable via static or dynamic import() from included sources are still pulled into the program. regression-harness-lambda-local.ts:33-44 imports @hyperframes/aws-lambda at the value level, which tsc resolves via package.json#main/types, which now points at unbuilt dist/. (See https://www.typescriptlang.org/tsconfig#exclude — "TypeScript will always include files referenced by import statements...regardless of exclude".)

    Three viable fixes, in order of how disruptive they are:

    • Run aws-lambda's build as its own step before producer in the root build script (split the single --filter invocation into two). The diff for that is ~one line and avoids the topological-sort tangle entirely.
    • Keep main/types in packages/aws-lambda/package.json pointing at ./src/... for workspace consumers, and use publishConfig.main/publishConfig.types (or a pack-time rewrite hook in the existing verify:packed-manifests flow) to swap to ./dist/... for the published tarball. This is what producer's lockstep dev story already implies.
    • Move regression-harness-lambda-local.ts out of packages/producer/src/ entirely — host it under packages/aws-lambda/src/regression/ and have the harness load it through a workspace-scoped entry point. This is the most architecturally clean: producer doesn't depend on aws-lambda, breaking the cycle.

    In any case the Test plan checkbox ✅ bun run build is not reproducible on the PR head — gh pr view 920 --json statusCheckRollup shows Build, Typecheck, and CLI smoke all FAILURE.

  2. blockerPR-body / diff version mismatch. Body says version 0.0.1 → 0.6.18 (matching producer's current 0.6.18). Diff in packages/aws-lambda/package.json:3 is 0.6.20. Producer is still on 0.6.18. If this is correct, the body needs updating; if 0.6.20 is a typo, the lockstep claim is broken. The lockstep-cadence narrative is load-bearing for the publish workflow — scripts/set-version.ts:28 is updated to include aws-lambda in the lockstep set, which means a future pnpm set-version 0.6.21 would re-sync everything, but right now the package starts out-of-step.

Important

  1. importantpackages/aws-lambda/build.mjs:26-36external list duplicates the package's deps + peerDeps lists in package.json. Any future runtime dep added to package.json must also be added here, or it will get inlined into the bundle silently. Worth a comment or, better, programmatically deriving external from Object.keys(pkg.dependencies) + Object.keys(pkg.peerDependencies) like a few of the heygen build configs do. As written it's a footgun for the next person to add a dep.

  2. importantpackages/aws-lambda/build.mjs:14-15rmSync('dist', { recursive: true, force: true }) then mkdirSync('dist', { recursive: true }) is fine but missing a cwd guard. If node build.mjs is invoked from the wrong cwd (e.g. someone runs it from repo root by accident), it deletes the wrong directory. cd packages/aws-lambda && node build.mjs is the documented contract but worth either an import.meta.dirname guard or a process.chdir at the top.

  3. importantNo CLI workflow integration test for the published path. Per the body, hyperframes lambda deploy shows a friendly install hint when @hyperframes/aws-lambda is missing. Now that the package will actually publish, there should be a positive CI assertion that npm install @hyperframes/aws-lambda after a global CLI install resolves a working module — otherwise the first regression in the publish pipeline ships to end users. The existing Smoke: global install job (skipped on this PR) likely covers part of this but I can't verify from the diff alone.

Nits

  1. nitpackages/aws-lambda/tsconfig.json:18 — adding "src/**/__fixtures__/**" to exclude is defensive but there's no __fixtures__/ directory in packages/aws-lambda/src/ today. Either drop it (YAGNI) or land a placeholder dir with a README. Harmless either way.

  2. nitpackages/aws-lambda/build.mjs:39-44 — the four build() calls in Promise.all are independent; this is fine, but the umbrella entry (src/index.ts) re-exports from handler.ts, events.ts, etc. Bundling the four separately means symbols may get duplicated into multiple chunks. With external: ["@hyperframes/producer", ...] keeping the heavy deps out it's small (kB-range), but worth a metafile: true + a size check in CI on a future pass.

  3. nitpackages/producer/src/regression-harness-lambda-local-types.ts:23-32 — the doc comment on width/height is excellent context. Worth lifting the last sentence ("distributed-simulated mode hardcodes 1920×1080 because it bypasses the event-serialization boundary; lambda-local goes through it, which is the whole point.") into the harness-mode comparison table if one exists, since this is the load-bearing rationale for the new mode.

Verdict: REQUEST CHANGES
Reasoning: Three required CI checks are red on a real build-ordering bug (producer's tsc emit needs aws-lambda's dist/.d.ts before aws-lambda has built), and the PR-body version (0.6.18) doesn't match the diff (0.6.20). Fix the build wiring (any of the three options above), reconcile the version, and the rest of the diff is sound.

Review by Vai

Copy link
Copy Markdown
Collaborator

@miguel-heygen miguel-heygen left a comment

Choose a reason for hiding this comment

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

Clean, well-structured publish setup. Build script mirrors packages/producer/build.mjs correctly, esbuild externals list covers all runtime/peer deps, tsconfig.build.json strips workspace paths for clean .d.ts emit, and the publish workflow ordering is correct (producer published before aws-lambda).

The type-extraction pattern for the tsc chicken-and-egg (regression-harness-lambda-local-types.ts + dynamic import cast) is a sound approach to avoid the circular build dependency.

One note on version:
package.json sets 0.6.20 — current latest is v0.6.19. This is fine since scripts/set-version.ts now includes packages/aws-lambda and will normalize it during the next release cut. Just flagging it's one-ahead of the current release.

Looks good to merge once CI is green.

@jrusso1020
Copy link
Copy Markdown
Collaborator Author

Thanks @vanceingalls — both blockers addressed in 187dfd5b + the PR body update just now.

Blocker 1 (build-ordering) — fixed. You correctly identified that tsconfig.exclude is a soft list and tsc still resolves files reachable through import() from included sources. The first attempt at this (a18fc04b) only added the exclude entry, which is why CI stayed red.

Rather than the three architectural rewires you suggested (each viable), 187dfd5b takes a smaller fix: route the dynamic import path through a top-level const LAMBDA_LOCAL_MODULE so tsc can't statically resolve the target. tsc treats the import target as opaque, the excluded file stops being part of the program, and the chicken-and-egg evaporates. Verified locally by replicating the CI environment:

rm -rf packages/{aws-lambda,producer,core}/dist
bun run build  # green

CI on 187dfd5b confirms: Build, Typecheck, CLI smoke (required) all pass.

I chose this over your option 1 (split --filter so aws-lambda builds before producer) because aws-lambda's own tsc emit also needs producer's dist/index.d.ts (handler.ts imports @hyperframes/producer/distributed), so reordering trades one side of the cycle for the other — it'd then need a stub or a project-references setup. Over option 2 (publishConfig.main/types) because the runtime build artifact is genuinely what we want consumers to load, not src — keeping a src↔dist swap-at-pack-time hides the published shape from local development. Over option 3 (move lambda-local under aws-lambda/) because the file is genuinely a producer-side test harness (it drives runDistributedSimulatedRender parity tests through the Lambda dispatch event boundary); hosting it under aws-lambda would invert the dependency direction in a way that doesn't match what the harness actually exercises.

Happy to revisit if you'd prefer one of the architectural fixes — the indirection trick is functional but I agree it's a workaround rather than a clean structural fix.

Blocker 2 (version mismatch) — fixed. PR body and diff both at 0.6.20 now. Bumped from 0.6.18 to align with current main lockstep, and added packages/aws-lambda to scripts/set-version.ts PACKAGES so future chore: release vX.Y.Z commits bump it alongside everything else (you flagged this risk; #6935eaab catches it).

Important items, quick triage:

Nits (#6, #7, #8): all reasonable. #6 (drop __fixtures__ exclude) is the only one I'd consider for this PR — happy to drop it if you'd prefer; otherwise leaving in defensively since fakeS3 is a fixture pattern producer already uses elsewhere. #7 / #8 are good follow-up cleanups.

Ready for re-review.

Copy link
Copy Markdown
Collaborator

@vanceingalls vanceingalls left a comment

Choose a reason for hiding this comment

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

Re-review at 187dfd5b. The prior REQUEST CHANGES review (mine, posted 23:12:59Z at this same SHA) was wrong on evidence — by submission time the fix commits (a18fc04b, 6935eaab, 187dfd5b) were all in and the build was already green. Apologies for the noise; this re-review corrects the record.

Blockers from prior review — status

  1. Blocker 1 — producer↔aws-lambda build cycle: ADDRESSED. Defense in depth across three layers and they hold up:

    • packages/producer/src/regression-harness-lambda-local-types.ts:14-43 — types-only file with zero aws-lambda imports.
    • packages/producer/src/regression-harness.ts:54const LAMBDA_LOCAL_MODULE = "./regression-harness-lambda-local.js" routes the dynamic import through a runtime variable so tsc can't statically resolve into the excluded file (per TS docs: "exclude" is soft, but a non-literal import() target is opaque to the resolver).
    • packages/producer/tsconfig.json:25src/regression-harness-lambda-local.ts in exclude as belt-and-braces.

    CI proves it at the reviewed SHA: Build / Typecheck / Test / Lint / CLI smoke (required) / Smoke: global install all SUCCESS. The earlier failures my first review cited were from the pre-a18fc04b runs; I should have checked the latest run on this SHA before submitting.

    One risk worth a comment in the code (which is already there at regression-harness.ts:36-47): the workaround relies on tsc not constant-folding the top-level const into the import() site. Current tsc doesn't, but if a future refactor inlines the literal (or templates it via a literal-tagged string), the cycle returns. The existing comment is adequate; no action needed.

  2. Blocker 2 — version drift: ADDRESSED. PR body now consistently says 0.6.20. packages/aws-lambda/package.json:3 = 0.6.20. scripts/set-version.ts:21-30 adds packages/aws-lambda to the PACKAGES list so future chore: release vX.Y.Z commits keep the package in lockstep. Producer at the PR head is still 0.6.18 because the branch is 19 commits behind main (which released v0.6.20 in 6c533c0b); at merge time the rebase pulls producer to 0.6.20 alongside, restoring lockstep — that's the correct state, not a blocker.

Architectural-call check — does James's rationale hold?

Yes for all three:

  • Split --filter (option 1): the cycle is symmetric. aws-lambda's tsc needs producer's dist/ for @hyperframes/producer/distributed types; producer's tsc (post this PR's main/types move) needs aws-lambda's dist/ only because of the regression-harness reference. Reordering --filter trades one cycle-side for the other. Right call.
  • publishConfig.main/types (option 2): his critique is right and is actually understated — publishConfig only rewrites at npm publish; pnpm pack reads the package.json main directly, so the existing verify:packed-manifests flow would also need a pack-time rewrite hook. Real complexity. Right call to defer.
  • Move lambda-local under aws-lambda (option 3): lambda-local's purpose is to drive producer's regression fixtures through the Lambda event boundary. The fixtures, golden files, and surrounding harness state all live under packages/producer/src/regression/. Hosting the mode under aws-lambda means either duplicating fixture-load logic across the boundary or having aws-lambda depend on producer's test fixtures — both are worse than the current arrangement. Right call.

The chosen LAMBDA_LOCAL_MODULE indirection is the canonical idiom for this — TypeScript's own docs flag dynamic-import-with-non-literal as the way to keep tsc out of a reachable file. The implementation is small (one const + one async loader), well-commented at the use site, and runtime-equivalent to the prior await import("...") because tsx resolves the string normally.

Importants 3-5 from prior review (external-list mirroring, cwd guard in build.mjs, publish-path smoke test): still open. James has them tracked as follow-ups. Reasonable to defer — Smoke: global install covers part of #5 (the CLI install flow), and #3/#4 are footguns rather than active bugs.

Pre-existing CI failure (not this PR): regression-shards (shard-2) fails on CORS errors fetching audio from gen-os-static.s3.us-east-2.amazonaws.com. Environmental, unrelated to this PR's build wiring.

Strengths (still load-bearing)

  • packages/aws-lambda/build.mjs:24-37 — external list mirrors the runtime/peer deps fully.
  • packages/aws-lambda/tsconfig.build.json:4"paths": {} to strip workspace aliases for the emit pass, with the rationale comment in build.mjs:55-59.
  • packages/aws-lambda/package.json:11-15 — keeping the whole scripts/ directory (rather than a trimmed file list) with the _formatBytes.ts shared-import rationale.

Verdict: APPROVE
Reasoning: Both prior blockers are resolved with verifiable evidence (CI green at the reviewed SHA + diff inspection). The architectural workaround is sound and well-documented. Outstanding importants are correctly scoped as follow-ups, not merge blockers.

Review by Vai (re-review)

@jrusso1020 jrusso1020 merged commit 4f274d3 into main May 17, 2026
54 of 59 checks passed
@jrusso1020 jrusso1020 deleted the chore/aws-lambda-publish-ready branch May 17, 2026 23:41
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