feat(core): build adapters declare their watch paths; fix symlink handling in node()'s directory form - #158
Merged
Merged
Conversation
…ocal providers Records the local-dev architecture: prisma-composer dev re-runs the deploy pipeline (Load, assemble, lower, Alchemy converge) against local implementations of the same resource types, declared on an optional dev field of ExtensionDescriptor. The field carries no nodes/provisions, so dev structurally cannot diverge the lowering. Adds the local-dev domain deep dive (process table, supervisor, substitution table, disk-backed S3 server, value sourcing, error surface), indexes both docs, points deploy-cli.md at the design, and marks the deferred.md dev-time sketch resolved. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pins every name, type, behavior, and file placement for ADR-0041: DevDescriptor (no state — dev uses alchemy localState via LowerOptions), the framework-owned DevProcessRecord table, the eight local providers and their exact reconcile semantics, the @internal/s3-protocol extraction (layering forces the protocol pieces down to the lowering layer), the serviceAddress threading through ComputeSerialized/DeploymentProps, the dir() build adapter, the dev command pipeline/supervisor/watch loops, the value-sourcing policy (secret placeholders vs hard-error env params), and the full error surface. Aligns ADR-0041 and local-dev.md with the refinements the deep code read forced (standIns hook, per-Database prisma dev instances, Ctrl-C semantics, stage `dev`, port allocation). Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Converge-scoped (the alchemy child), session-scoped (service children and the bucket server, owned by the dev command via the process table and standIns), and machine-scoped daemons (prisma dev Postgres instances). Names why the bucket server is session-scoped rather than a daemon, and that standIns permits a detached daemon without a seam change. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s; drop the standIns hook Operator decision: treat every local backing service the way prisma dev Postgres instances already work — long-lived daemons that survive dev sessions, the firebase/supabase emulator model. Providers ensure their emulator at reconcile and provision instances by communicating with it (the ORM CLI for Postgres; a loopback /_pcdev/ admin API on the new daemonized bucket emulator). This deletes the standIns hook entirely: DevDescriptor is providers/container/preflight/teardown. Pins the emulator daemon manager (machine-global registry, stable ports from 4300, version-skew restart, ready-file protocol), the emulator-main entry in @internal/s3-protocol, Ctrl-C leaving all emulators up, and --fresh as the only removal path. Vocabulary swept from stand-in to emulator across ADR-0041, local-dev.md, and the project spec/plan. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ervice processes Operator decision refining the emulator model: the target spins up one machine-scoped, multi-tenant emulator per node kind the topology uses — a Compute emulator (a small local counterpart of the platform compute service: deployment PUTs, child processes, crash backoff, logs), the ORM prisma dev Postgres emulator, and one bucket emulator — and providers provision isolated instances by communicating with them, wired together exactly as deployed. This deletes the framework-owned process table and the CLI supervisor: the dev command is a view through a new dev.attach hook (endpoints, merged logs, stop control), plus a topology-aware dev.emulators ensure hook. Ctrl-C stops the app services; emulators and data persist; --fresh removes app-scoped instances but never the shared daemons. Bucket namespacing via <app>--<name> physical names on the binding; per-bucket in-project data roots keep objects browsable. Daemon machinery consolidates into a new @internal/dev-emulators package; the plan grows to six slices. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Verification pass over the spec after the emulator-model rewrites, fixing six genuine gaps an implementer would otherwise have had to improvise: deployment PUT start rules (stopped/held services always start — an explicit converge clears held and undoes stop); per-daemon health paths (the bucket daemon root is the S3 wire); the prisma dev stop/start port-stability assumption turned into a pinned verification item with a pinned fallback (--db-port from a machine-global allocation file); the watch contract made adapter-declared (Bundle.watch, populated by node/ nextjs/dir, consumed by the CLI); bucket credential upsert keyed by accessKeyId plus physical-name length validation and lazy dir recreation; and DevProvidersInput threading — dev.providers(input) receives the resolved container and devDir, since local providers are emulator clients that must know which app they provision for (lower() already holds the deserialized container map). Plus API hygiene pins (path-segment validation, serialized state writes, no log rotation), a warm-start acceptance criterion, and plan updates (S2 carries Bundle.watch, S4 runs the port-stability check first). Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ADR-0041 (local dev): `prisma-composer dev` rebuilds on a change to the app's own build output, never the framework's. Bundle.watch names the absolute paths to the USER-BUILT inputs a bundle was assembled from, so the dev watch loop knows exactly what to watch without guessing. Optional so a build adapter that predates this field still compiles. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Per spec: nextjs() watches the whole `.next/standalone` tree the app's own `next build` produced, so a rebuild is picked up regardless of which file inside it changed. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Extracts directory resolution/validation, the symlink hard error (ADR-0005), the deploy-working-dir overlap guard, and the boot-wrapper esbuild build into `control/assemble-shared.ts`, so the upcoming dir() adapter reuses the same, well-tested logic instead of duplicating it — the two must never drift on error wording. Also populates Bundle.watch: [resolved entry file] for both the single-file and directory forms of node(), per spec. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…/dir
Per spec section 7: a directory-only sibling of node() for a service
whose runnable is a whole built tree (the open-chat proof's shape).
`dir({ module, dir, entry })` resolves both paths relative to
dirname(module) (ADR-0004); assemble validates dir exists, validates
entry exists inside it, copies the tree verbatim (symlink = hard error,
ADR-0005), and applies the same wrapper bundling as node() — reusing
control/assemble-shared.ts so the two adapters cannot drift.
Ships inside the existing @internal/node package as a sibling entry
(packages/0-framework/2-authoring/node/src/dir.ts), mapped through
9-public exactly how node() is: @prisma/composer/dir (authoring) and
@prisma/composer/dir/control (the extension descriptor for
prisma-composer.config.ts).
Signed-off-by: willbot <w.a.madden+machine@gmail.com>
Signed-off-by: Will Madden <madden@prisma.io>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Mirrors node()'s directory-form test idioms: missing dir/entry errors verbatim, the symlink hard error (both a symlinked file and a symlinked directory, unreported-descent), the deploy-working-dir overlap guards, and a multi-file fixture (nested dirs + a binary-ish asset) proving tree fidelity byte-for-byte. Also asserts Bundle.watch names the resolved input dir, and the authoring surface's plain-data contract. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Mirrors the register and length of node()'s and nextjs()'s entries in both docs/guides/building-an-app.md (canonical for humans) and skills/prisma-composer/SKILL.md (the agent-condensed mirror), per .agents/rules/user-facing-surface-changes.mdc. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`resolveDir as resolveDirShared` tripped the no-bare-cast Biome plugin (a plain `` `$x as $t` `` GritQL match, not scoped to expression context) — a real diagnostic, but not a real cast, and it would have counted against the lint:casts CI ratchet. Renaming the LOCAL wrapper to resolveDirRunnable instead and importing the shared resolveDir unaliased sidesteps the false positive without touching its public name. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…dir, entry points into the copied tree
Review finding on S2: the spec said { dir: <workDir>/bundle, entry },
which taken literally drops the wrapper main.mjs from the artifact and
breaks boot. The implementation correctly followed the stronger
"identical to node()" pin; align the prose with the real Bundle contract.
Signed-off-by: willbot <w.a.madden+machine@gmail.com>
Signed-off-by: Will Madden <madden@prisma.io>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Summary by CodeRabbit
WalkthroughThe changes establish the 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
commit: |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.drive/projects/local-dev/spec.md:
- Around line 397-401: Sanitize captured prisma dev output before including it
in the “could not read the database URL” error, masking connection URLs and
other sensitive values while preserving the command’s last non-empty-line
parsing behavior. Update .drive/projects/local-dev/spec.md lines 397-401 and
document in docs/design/10-domains/local-dev.md line 218 that command output is
sanitized rather than shown verbatim.
- Around line 208-215: Update the credential persistence and S3 SigV4
authentication flow described around the PUT credentials and “ANY accepted
credential” behavior to retain each credential’s owning app. During request
authentication, derive the physical bucket’s app namespace from its
<app>--<name> form and accept a credential only when that namespace matches the
credential owner; reject cross-app credentials while preserving valid same-app
access and credential replacement behavior.
In `@docs/design/90-decisions/README.md`:
- Line 65: Update docs/design/90-decisions/README.md:65 to list the actual
DevDescriptor hooks without dev.state, and state that service supervision
belongs to the Compute emulator. Also update
docs/design/90-decisions/ADR-0041-local-dev-runs-the-deploy-pipeline-against-local-providers.md:266-267
to remove dev.state references and use LowerOptions.state/localState() instead.
In `@packages/0-framework/2-authoring/node/src/control/assemble-shared.ts`:
- Around line 80-89: Update assertNoSymlinks to lstat dirPath before walking and
reject immediately when the root path itself is symbolic, while preserving the
existing recursive entry scan. Add a test covering a symlink supplied directly
as dir and verify it is rejected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: db76c837-2ddd-4cd4-9325-f17a6d9946c8
📒 Files selected for processing (31)
.drive/deferred.md.drive/projects/local-dev/plan.md.drive/projects/local-dev/spec.mdarchitecture.config.jsondocs/design/10-domains/README.mddocs/design/10-domains/deploy-cli.mddocs/design/10-domains/local-dev.mddocs/design/90-decisions/ADR-0041-local-dev-runs-the-deploy-pipeline-against-local-providers.mddocs/design/90-decisions/README.mddocs/guides/building-an-app.mdpackages/0-framework/1-core/core/src/control/deploy.tspackages/0-framework/2-authoring/nextjs/src/__tests__/assemble.test.tspackages/0-framework/2-authoring/nextjs/src/control/build.tspackages/0-framework/2-authoring/node/package.jsonpackages/0-framework/2-authoring/node/src/__tests__/assemble-dir.test.tspackages/0-framework/2-authoring/node/src/__tests__/assemble.test.tspackages/0-framework/2-authoring/node/src/__tests__/dir.test.tspackages/0-framework/2-authoring/node/src/__tests__/no-control-import.test.tspackages/0-framework/2-authoring/node/src/control/assemble-shared.tspackages/0-framework/2-authoring/node/src/control/build.tspackages/0-framework/2-authoring/node/src/control/dir.tspackages/0-framework/2-authoring/node/src/dir.tspackages/0-framework/2-authoring/node/src/exports/dir-control.tspackages/0-framework/2-authoring/node/src/exports/dir.tspackages/0-framework/2-authoring/node/tsdown.config.tspackages/9-public/composer/package.jsonpackages/9-public/composer/src/exports/dir-control.tspackages/9-public/composer/src/exports/dir.tspackages/9-public/composer/tsdown.config.tsskills/prisma-composer/SKILL.mdtsconfig.depcruise.json
wmadden-electric
added a commit
that referenced
this pull request
Jul 22, 2026
…stics, stale-reference fixes Three of the four CodeRabbit findings on #158, all valid. Bucket-emulator SigV4 now authenticates against only the credentials owned by the target bucket's app (derived from the <app>-- prefix) — cross-app access was possible with a known bucket name, violating the stated multi-tenant isolation; credential registration records the owning app and a foreign re-registration 409s. The pinned prisma dev error masks connection-URL credentials before embedding captured output (the no-value-logging contract applies to diagnostics). The ADR index one-liner and ADR-0041 Related section still described the pre-pivot model (dev.state, process table, CLI supervision) — rewritten to the emulator model. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…stics, stale-reference fixes Three of the four CodeRabbit findings on #158, all valid. Bucket-emulator SigV4 now authenticates against only the credentials owned by the target bucket's app (derived from the <app>-- prefix) — cross-app access was possible with a known bucket name, violating the stated multi-tenant isolation; credential registration records the owning app and a foreign re-registration 409s. The pinned prisma dev error masks connection-URL credentials before embedding captured output (the no-value-logging contract applies to diagnostics). The ADR index one-liner and ADR-0041 Related section still described the pre-pivot model (dev.state, process table, CLI supervision) — rewritten to the emulator model. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The b1d7324 cherry-pick alone left spec.md behind four other docs(drive) commits already on the design branch (fs-store unknown-bucket/invalid-key behavior, the registryRoot pins, the dir() return-shape fix) that this PR's docs also carry. Take the design branch's spec.md wholesale so the four reviewed doc files (spec.md, local-dev.md, the ADR index, ADR-0041) match it exactly — no local drift on files this PR does not own the content of. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ents CodeRabbit finding on #158: resolveDir() confirmed dir is a directory via statSync, which follows symlinks, and assertNoSymlinks() only walked dirs children via readdir — so a symlink passed AS dir itself was silently dereferenced and its target copied, violating ADR-0005 (a symlink is a hard error, never dereferenced). lstat dir before the walk and treat it like any other found symlink, reusing the same error shape. Covers both call sites that share this code: dir()s assemble and node()s directory form each get a test asserting a symlinked dir hard-errors without following the link. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
All four review findings addressed:
Gates re-run green: typecheck 64/64, node tests 47/47 (+2), lint/lint:deps clean, cast ratchet delta 0. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/0-framework/2-authoring/node/src/control/assemble-shared.ts (1)
48-49: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse
path.relative()for containment checks.Appending
path.sepbreaks for filesystem roots:/ + /becomes//, so an entry below/is rejected, and the work-dir overlap guard does not detect that a root source containsworkDir. The latter can permit assembly to copy a source into its own artifact subtree.
packages/0-framework/2-authoring/node/src/control/assemble-shared.ts#L48-L49: validate the entry using a relative-path containment check.packages/0-framework/2-authoring/node/src/control/assemble-shared.ts#L117-L124: use the same helper for both overlap directions, including equality.Proposed fix
+function isWithinOrEqual(parent: string, child: string): boolean { + const relative = path.relative(parent, child); + return ( + relative === '' || + (!relative.startsWith(`..${path.sep}`) && relative !== '..' && !path.isAbsolute(relative)) + ); +} + - if (!entryPath.startsWith(dirPath + path.sep)) { + if (!isWithinOrEqual(dirPath, entryPath) || entryPath === dirPath) { throw new Error( // ... ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/0-framework/2-authoring/node/src/control/assemble-shared.ts` around lines 48 - 49, Replace the string-prefix containment check around entryPath with a path.relative()-based helper that treats the path itself and descendants as contained while rejecting paths outside the base. In packages/0-framework/2-authoring/node/src/control/assemble-shared.ts lines 48-49, use this helper for entry validation; in lines 117-124, reuse it for both source/workDir overlap directions and include equality so root sources cannot overlap their artifact subtree.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.drive/projects/local-dev/spec.md:
- Around line 128-129: Update ensureDaemon’s registry coordination to use an
OS-level lock or atomic claim protocol covering the health check, port
persistence, and spawn decision. After acquiring the claim, re-read the registry
and reuse any healthy existing daemon; only persist a new port and spawn when no
healthy entry remains, then release the lock reliably.
In `@docs/design/90-decisions/README.md`:
- Line 65: Update the ADR-0041 summary to distinguish emulator scope: describe
Compute and S3/Buckets as machine-global daemons, and clarify that Postgres uses
one detached local prisma dev instance per Database. Replace the broader “one
machine-scoped emulator per node kind” wording while preserving the surrounding
deployment-pipeline and credential behavior details.
---
Outside diff comments:
In `@packages/0-framework/2-authoring/node/src/control/assemble-shared.ts`:
- Around line 48-49: Replace the string-prefix containment check around
entryPath with a path.relative()-based helper that treats the path itself and
descendants as contained while rejecting paths outside the base. In
packages/0-framework/2-authoring/node/src/control/assemble-shared.ts lines
48-49, use this helper for entry validation; in lines 117-124, reuse it for both
source/workDir overlap directions and include equality so root sources cannot
overlap their artifact subtree.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8ac7f95d-6158-4a2d-b6fe-c98be0718375
📒 Files selected for processing (7)
.drive/projects/local-dev/spec.mddocs/design/10-domains/local-dev.mddocs/design/90-decisions/ADR-0041-local-dev-runs-the-deploy-pipeline-against-local-providers.mddocs/design/90-decisions/README.mdpackages/0-framework/2-authoring/node/src/__tests__/assemble-dir.test.tspackages/0-framework/2-authoring/node/src/__tests__/assemble.test.tspackages/0-framework/2-authoring/node/src/control/assemble-shared.ts
wmadden-electric
added a commit
that referenced
this pull request
Jul 22, 2026
…stics, stale-reference fixes Three of the four CodeRabbit findings on #158, all valid. Bucket-emulator SigV4 now authenticates against only the credentials owned by the target bucket's app (derived from the <app>-- prefix) — cross-app access was possible with a known bucket name, violating the stated multi-tenant isolation; credential registration records the owning app and a foreign re-registration 409s. The pinned prisma dev error masks connection-URL credentials before embedding captured output (the no-value-logging contract applies to diagnostics). The ADR index one-liner and ADR-0041 Related section still described the pre-pivot model (dev.state, process table, CLI supervision) — rewritten to the emulator model. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
wmadden-electric
added a commit
that referenced
this pull request
Jul 23, 2026
…logy wording Second-round CodeRabbit findings on #158, both valid. Two concurrent prisma-composer dev processes could both observe no healthy daemon, double-spawn, and collide on a port allocation — the ensure critical section is now serialized across processes with an atomic mkdir lock (stale-holder recovery, bounded wait, re-read after acquire, allocation inside the lock). The ADR index over-claimed one emulator per node kind: Compute/buckets are machine-global daemons, Postgres is one prisma dev instance per Database under the ORM CLI's own manager. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Carries the second-round CodeRabbit resolutions: the inter-process daemon-ensure lock protocol (implemented on #160) and the precise emulator-topology wording, plus the S4-findings pins. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
Second-round findings addressed:
This branch's spec/plan/ADR-index copies are synced byte-identical to the design tip (8ea6f27). |
…composer/dir"
Operator catch: nodes directory form (node({ module, dir, entry })) —
which S2s dir() duplicated 1:1 — already shipped on main before this
projects design pass, as friction #3s fix. The spec pinned dir() against
a stale friction-log premise (design tip 147eef8, spec.md sect. 7
revised).
Strikes the dir() authoring surface, its control assemble, both
packages exports (@internal/node and the published
@prisma/composer/dir), the package.json/tsdown/tsconfig.depcruise/
architecture.config entries, the dir.ts firewall-test addition, and the
guide/skill entries. assemble-shared.ts, build.ts, and assemble.test.ts
keep the pieces that were actually S2s real content (the symlink-as-dir
fix and the watch-field work) — those are reworked in the following
commit rather than reverted here.
Signed-off-by: willbot <w.a.madden+machine@gmail.com>
Signed-off-by: Will Madden <madden@prisma.io>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…emble-shared.ts back in Spec sect. 7 (revised, design tip 147eef8): with dir() struck, assemble-shared.ts had exactly one consumer again, so its extraction stopped paying for itself — folded back into build.ts, byte-identical error strings preserved. The symlink-as-dir fix now runs before ANY dereferencing stat decides directory-ness: lstat(dir) first, so a symlink hard-errors identically whether it points at a real directory (which the old statSync-based check silently accepted) or a file — neither is ever dereferenced (ADR-0005). Reuses the existing "contains symlinks" error shape via a shared symlinksFoundError() rather than a new message. Bundle.watch for the directory form now names the whole resolved dir, not just the entry file — a rebuild may touch only a sibling entry doesnt import. The single-file forms watch (the entry file) is unchanged. Syncs spec.md/plan.md wholesale from the design branch tip (147eef8). Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…lization pin) Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
Scope correction (d3df738): the |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.drive/projects/local-dev/spec.md (2)
396-397: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRestrict persisted Postgres URLs like secrets.
postgres.jsonstores connection URLs that contain database credentials, but the specification only requiressecrets.jsonto use mode0600. Requirepostgres.jsonand the containing directory to be owner-only, preserving those permissions across temp-file replacement.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.drive/projects/local-dev/spec.md around lines 396 - 397, Update the specification for postgres.json to require owner-only permissions on both the file and its containing directory, treating persisted database URLs like secrets. Ensure these 0600/0700 permissions are preserved when atomically replacing the file via a temporary file.
471-473: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMake Postgres instance names collision-resistant.
Lowercasing, replacing characters, collapsing runs, and truncating to 63 characters can map distinct
(app, database)pairs to the samepcdev-*instance—for example, names that differ only after the truncation point. That can cause one database to reuse another’s instance and make teardown remove the wrong instance. Add a stable hash/suffix or detect and reject collisions, and use the same mapping everywhere.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.drive/projects/local-dev/spec.md around lines 471 - 473, Update the Postgres instance name derivation specification to make `(app, database-id)` mappings collision-resistant, preferably by adding a deterministic hash suffix within the 63-character limit; alternatively require collision detection and rejection. Ensure the resulting mapping is used consistently for instance creation, lookup, reuse, and teardown.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.drive/projects/local-dev/spec.md:
- Line 729: Reword the line beginning with “#3” in the referenced design note so
it no longer starts with a hash character, while preserving its meaning and
avoiding interpretation as a Markdown heading.
---
Outside diff comments:
In @.drive/projects/local-dev/spec.md:
- Around line 396-397: Update the specification for postgres.json to require
owner-only permissions on both the file and its containing directory, treating
persisted database URLs like secrets. Ensure these 0600/0700 permissions are
preserved when atomically replacing the file via a temporary file.
- Around line 471-473: Update the Postgres instance name derivation
specification to make `(app, database-id)` mappings collision-resistant,
preferably by adding a deterministic hash suffix within the 63-character limit;
alternatively require collision detection and rejection. Ensure the resulting
mapping is used consistently for instance creation, lookup, reuse, and teardown.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2baba3bc-03c2-4aaa-809a-9ab5d3302a0b
📒 Files selected for processing (6)
.drive/projects/local-dev/plan.md.drive/projects/local-dev/spec.mddocs/design/10-domains/local-dev.mddocs/design/90-decisions/README.mdpackages/0-framework/2-authoring/node/src/__tests__/assemble.test.tspackages/0-framework/2-authoring/node/src/control/build.ts
…rmed heading Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
wmadden
approved these changes
Jul 23, 2026
| | Cluster | Resources | Local implementation | | ||
| | --- | --- | --- | | ||
| | Compute | `ComputeService`, `Deployment`, `EnvironmentVariable` | a Compute **emulator** owning one child process per service, plus a local env-var store | | ||
| | Postgres | `Project`, `Database`, `Connection` | the ORM CLI's local Postgres **emulator** (`prisma dev`) | |
Collaborator
There was a problem hiding this comment.
Why is Connection listed as a Postgres-backed resource? Don't RPC dependencies have Connections too?
Contributor
Author
|
@coderabbitai resolve |
✅ Action performedComments resolved and changes approved. |
wmadden-electric
enabled auto-merge
July 23, 2026 12:36
wmadden-electric
added a commit
that referenced
this pull request
Jul 23, 2026
…ents CodeRabbit finding on #158: resolveDir() confirmed dir is a directory via statSync, which follows symlinks, and assertNoSymlinks() only walked dirs children via readdir — so a symlink passed AS dir itself was silently dereferenced and its target copied, violating ADR-0005 (a symlink is a hard error, never dereferenced). lstat dir before the walk and treat it like any other found symlink, reusing the same error shape. Covers both call sites that share this code: dir()s assemble and node()s directory form each get a test asserting a symlinked dir hard-errors without following the link. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The decision: build adapters declare which files to watch, and
node()'s directory form gets two fixes.Context in one breath: we're building
prisma-composer dev(ADR-0041, whose docs ride on this branch), a command that runs a whole Composer app locally and redeploys a service when you rebuild it. This PR is standalone groundwork for that command. It has no dependency on the other local-dev PRs.Watch paths
A build adapter (
node(),nextjs()) is the piece that turns your build's output into a deployable bundle. When you rebuild, the dev command needs to notice — but only the adapter knows which files on disk are "your build's output." So the adapter now says so, in the new optionalwatchfield above:node({ module, entry })declares its entry file.node({ module, dir, entry })declares the whole directory. A rebuild may touch only a sibling asset, so watching just the entry file would miss it.nextjs()declares its standalone output directory.A bundle without
watchsimply isn't watched. Core stays adapter-agnostic: it watches whatever the adapter declared, and knows nothing else.Two fixes to
node()'s directory formReview of this PR found a real bug that exists on
maintoday: a symlink passed asdiritself is silently followed and its target gets copied into the artifact. The path check usedstatSync, which follows links, and the no-symlink scan only inspected the directory's children. That violates the build rules in ADR-0005: a symlink is a hard error, never dereferenced. Fixed with anlstatbefore anything follows the link. A related sharp edge went with it: a symlink pointing at a file used to surface as a confusing "not a directory" error; both target types now produce the same symlink error.History worth knowing
This PR originally added a new
dir()adapter for directory-shaped builds. Will caught thatnode({ module, dir, entry })already ships that exact contract — the design had pinned a duplicate against a stale premise. Thedir()commits are reverted in-branch (the revert pair is kept in history deliberately), and the guide text that still describednode()as single-file-only is corrected. What remains is the content that was always independently needed.Tested
Symlink-as-
dirfor both file and directory targets, through the directory form. Deep-nested tree fidelity (files arrive byte-identical). Thewatchvalue per adapter and per form. Nothing from the removeddir()tests was dropped — cases were relocated into the directory-form suite. Workspace: typecheck 64/64, tests 53/53, lint and dependency-layering clean, no new type casts.Alternatives considered
dir()beside the directory form — a permanently confusing duplicate public API.watchwith the dev command itself — that PR is already the largest; this merging independently lets the dev command build on real watch support.🤖 Generated with Claude Code