Repository navigation
Design: a deploy finds its Project by logicalId only, and every node gets a logicalId and a name - #417
Design: a deploy finds its Project by logicalId only, and every node gets a logicalId and a name#417wmadden-electric wants to merge 2 commits into
Conversation
…nsent Two Drive projects that replace #355. deployment-identity: a deploy finds its Project by logicalId only, refuses to create one without --create, Console creates the Project at connect, and a platform script backfills logicalId on old Projects. node-identity-and-consent: written for handoff. Every node gets a required logicalId and an optional name, data-losing deploys use the ORM's consent model, and the platform scopes CI tokens to one Project. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
|
✅ Gizmo reviewed 878f1b5 — posted 0 inline comment(s) this pass. Open findings: 🟠 1 major · 🟡 6 minor Change walkthroughThis pull request is documentation-only design work in Deployment-identity docs (design-notes.md, plan.md, spec.md): the inline ADR-0051 citation is replaced with the spelled-out record name, and the amendment/DoD bullets now name ADR-0024 and ADR-0057 as the records that still carry the display-name fallback (verified against the actual ADR text) and must be amended when it is removed. The reference list adds ADR-0057 and ADR-0059 with correct titles. Node-identity-and-consent docs (design-notes.md, plan.md, spec.md): the compatibility-path citation is re-pointed to ADR-0057, D10 now states that a Project with neither a Assets (logicalid-walkthrough.md, user-scenarios.md): addresses throughout are rewritten as full paths from the app name ( |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
![prisma-gizmo[bot]](https://avatars.githubusercontent.com/in/4769794?s=60&v=4){kind=small}
| - [ ] Releasing a setup ("Remove setup") or closing the setup PR unmerged deletes the Project the connect created, and only that one. | ||
| - [ ] Every linked Project in production has a `logicalId`; the backfill reports zero remaining. | ||
| - [ ] `prisma deploy --create` creates a Project from a laptop; without the flag the same deploy refuses. | ||
| - [ ] The two Composer decision records named above are amended. The deploying guide and the core-concepts skill say that a deploy finds its Project by `logicalId` and needs `--create` to make one. The guide's `prisma deploy module.ts --name demo-42` example still works, with `--create` on its first run. |
There was a problem hiding this comment.
🟠 Major · consistency — DoD amends the superseded ADR-0051 and never names ADR-0057, the live record for Project resolution
"The two Composer decision records named above" are ADR-0024 and ADR-0051 ("a node's address is its logical ID on the platform", referenced at line 26 and in the references at line 61). ADR-0057 supersedes ADR-0051, and it is ADR-0057's Consequences that record the behavior this project deletes: "container resolution falls back to matching a Project by display name … a compatibility path" (and ADR-0057 also rejects "Match on display names"). If slice 3 amends only the records named here, the repository's binding decision record keeps prescribing the fallback the new resolver no longer implements. The same omission repeats in plan.md slice 3 ("the decision records on stage resolution and node addresses amended") and in the node-identity-and-consent spec and plan.
Recommended fix
| - [ ] The two Composer decision records named above are amended. The deploying guide and the core-concepts skill say that a deploy finds its Project by `logicalId` and needs `--create` to make one. The guide's `prisma deploy module.ts --name demo-42` example still works, with `--create` on its first run. | |
| - [ ] The two Composer decision records named above are amended, together with ADR-0057 ("a node's address is its full path from the app name"), which supersedes ADR-0051 and is the live record that carries the display-name fallback. The deploying guide and the core-concepts skill say that a deploy finds its Project by `logicalId` and needs `--create` to make one. The guide's `prisma deploy module.ts --name demo-42` example still works, with `--create` on its first run. |
|
|
||
| ## The model | ||
|
|
||
| The Project's `logicalId` is the app name (ADR-0051). Container resolution matches on it and on nothing else. If no Project matches, the deploy stops unless `--create` was passed. The platform creates the Project when a repo is connected, so a connected repo's CI run always finds its Project and never needs `--create`. |
There was a problem hiding this comment.
🟡 Minor · consistency — Model statement cites ADR-0051, the superseded record, instead of ADR-0057
ADR-0051 was superseded by ADR-0057, which is the live record for "the address is the node's logicalId" (for the root, the address is the app name, so the claim here still holds). Citing only the superseded ADR — here, and in the references lists of both projects' specs — sends the reader to a dead record. The same stale citation appears in .drive/projects/deployment-identity/spec.md (references) and .drive/projects/node-identity-and-consent/spec.md and its design-notes D2.
Recommended fix
| The Project's `logicalId` is the app name (ADR-0051). Container resolution matches on it and on nothing else. If no Project matches, the deploy stops unless `--create` was passed. The platform creates the Project when a repo is connected, so a connected repo's CI run always finds its Project and never needs `--create`. | |
| The Project's `logicalId` is the app name (ADR-0051, superseded by ADR-0057, which keeps this: the root's address is the app name, and the address is the node's `logicalId`). |
| const auth = provision(authModule, { logicalId: 'auth', secrets: { signingKey: envSecret('AUTH_SIGNING_SECRET') } }); | ||
| provision(web, { logicalId: 'web', name: 'Web', deps: { db: catalog, auth: auth.rpc } }); | ||
| provision(api, { logicalId: 'api', deps: { db: catalog } }); |
There was a problem hiding this comment.
🟡 Minor · consistency — Walkthrough's proposed API puts logicalId/name in provision() options, contradicting settled decision D1
The example passes logicalId: 'web', logicalId: 'api' and logicalId: 'auth' inside the provision() options object, but design-notes D1 settles that "logicalId lives on the node definition (postgres({ logicalId }), module({ logicalId, name }, …)), not on the provision() call" — the provision-time override stays id, for provisioning the same node twice or a packaged module. The glossary likewise defines the provision-time override as id. A handoff implementer reading this asset as the worked example could build the wrong constructor surface.
Recommended fix
| const auth = provision(authModule, { logicalId: 'auth', secrets: { signingKey: envSecret('AUTH_SIGNING_SECRET') } }); | |
| provision(web, { logicalId: 'web', name: 'Web', deps: { db: catalog, auth: auth.rpc } }); | |
| provision(api, { logicalId: 'api', deps: { db: catalog } }); | |
| const auth = provision(authModule, { secrets: { signingKey: envSecret('AUTH_SIGNING_SECRET') } }); | |
| provision(web({ logicalId: 'web', name: 'Web' }), { deps: { db: catalog, auth: auth.rpc } }); | |
| provision(api({ logicalId: 'api' }), { deps: { db: catalog } }); |
|
|
||
| ``` | ||
| Project "billing" (proj_…) exists and holds a production deploy from | ||
| another source. Continue? (--yes) |
There was a problem hiding this comment.
🟡 Minor · consistency — Takeover prompt in 5b offers (--yes), which the design's consent model forbids
The scenario shows Continue? (--yes) for deploying into a Project that holds another source's production deploy. D8 and the spec settle that --yes never consents to a data-losing action — the consent flags are per-subject --delete <subject> / --rename <old>:<new>. Setting logicalId: 'billing' over store is exactly a root re-identification, so the consistent answer mechanism is --rename store:billing. The surrounding text hedges ("Whether Composer asks, or just logs, is a separate decision"), but the illustrated mechanism contradicts the consent model the design hands off.
Recommended fix
| another source. Continue? (--yes) | |
| another source. Continue? (--rename store:billing) |
|
|
||
| | # | Scenario | Expected | Covered by | | ||
| | --- | --- | --- | --- | | ||
| | B1 | Author changes the root `name` from "Store" to "Storefront". | Console label changes. Same Project, same stages, same data. | D1, D3 | |
There was a problem hiding this comment.
🟡 Minor · consistency — Scenarios use the retired word "stage" for what ADR-0059 renames to "environment"
B1 says "same stages" and D5 (line 57) "User destroys a stage, or production". ADR-0059 replaces "stage" with "environment" on every Composer surface (guides, errors, agent skill), and these handoff docs seed exactly those surfaces. The same slip appears in the walkthrough's §2 table ("overwrites its production stage") and in node-identity design-notes ("The hash differs per stage"). Quoting ADR-0024's title is fine; describing behavior is not.
Recommended fix
| | B1 | Author changes the root `name` from "Store" to "Storefront". | Console label changes. Same Project, same stages, same data. | D1, D3 | | |
| | B1 | Author changes the root `name` from "Store" to "Storefront". | Console label changes. Same Project, same environments, same data. | D1, D3 | |
| 2. A6: migration rule for existing single-string modules. | ||
| 3. A7: backfill of `logicalId` on old Projects, or a one-time Console edit. | ||
| 4. C3 and B5: plan output for identity changes below the root, and whether it needs consent. | ||
| 5. E3: the fate of `--name`. |
There was a problem hiding this comment.
🟡 Minor · consistency — E3 and open item 5 still list the fate of --name as unsettled, but the design settles it
Row E3 (line 66) marks --name as "Open" and item 5 here repeats "the fate of --name" as an open item, but the settled design removes it: design-notes "Smaller consequences" ("--name on prisma deploy goes away"), the spec's phase 2 ("--name on prisma deploy goes away") and plan phase 2 ("--name is removed from prisma deploy"). A reader reconciling the asset against the spec gets two different answers.
Recommended fix
Delete open item 5 and change the E3 row's "Open" to the settled answer: --name is removed in phase 2 and re-identifying the root is --rename.
| ### D10. Backfill `logicalId` on old Projects once | ||
|
|
||
| A platform script sets `logicalId` from the link's reserved module name on | ||
| every linked Project that has none. A Project with neither is set by hand in |
There was a problem hiding this comment.
🟡 Minor · correctness — D10 claims leftover Projects are "set by hand in Console", which deployment-identity's spec defers as an open question
D2's safe-removal story assumes D10 covers old Projects, but D10 only backfills linked Projects from the link's reserved module name. For the remaining population — Projects old enough to predate logicalId that also have no repo link — D10 says they are "set by hand in Console", yet the deployment-identity spec's open questions (line 55) defer "whether Console should also allow editing a Project's logicalId by hand for Projects with no link" to the follow-on project "if it comes up". Both cannot hold: either the Console capability exists (then the spec's open question is already answered) or it does not (then this population loses display-name resolution with no remediation, and --create would build them a second, empty Project). The cross-cutting requirement "every repo connected through Console today keeps deploying" is unaffected — linked Projects are backfilled — but the docs disagree about the unlinked tail.
Recommended fix
Reconcile the two documents: either state in D10 that Console editing of a Project's logicalId ships with the backfill slice, or scope D10 to linked Projects and record that unlinked Projects without a logicalId have no automatic remediation and their deploys stop with the --create message.
…nd environments Cite the decision record that makes a node's address its full path from the app name, which superseded the one the designs cited and is the record that keeps the display-name fallback. Use full-path addresses and the word environment in examples. Record how the scenarios' open items were settled, and put logicalId on the node definition in the worked example. Add the open question this raises: on a child, the value an author writes is one segment of the logicalId. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
There was a problem hiding this comment.
New findings: none · trace
Still open from previous reviews: 🟠 1 major · 🟡 6 minor
Today, renaming the root module in a connected repository can send the next deploy into a different Project:
Composer selects the Project by matching the root module's name against each Project's
logicalId, and falls back to matching the display name. Nothing ties the deploy to the Project the repository was connected to, so the next push from repo A reconciles into repo B's production stage. A rename to an unused name creates a stray, empty Project instead.This pull request adds two Drive projects that fix this. It replaces #355, which checked the GitHub repository id inside Composer.
The decision
A deploy finds its Project by
logicalIdonly, and never creates one unless asked. The display-name fallback is removed. A deploy whose identifier matches nothing stops with "this would create a new project, not update one; pass--create". Console creates the Project when a repository is connected, so CI never needs to create one. A one-time platform script gives older Projects alogicalId. This is thedeployment-identityproject, which we are executing now.Every node gets a required
logicalIdand an optionalname. One string per node is today at once its identity, its label and its address, which is why a cosmetic edit can change identity. With two fields, editingnameonly changes the Console label, and editinglogicalIdis a visible, deliberate identity change. A deploy that would delete or re-identify a database or bucket asks for consent the way the ORM's migration commands do:--delete <subject>or--rename <old>:<new>,--yesnever consents, and CI fails closed. CI tokens scoped to one Project and Console approval of consent follow on the platform. This is thenode-identity-and-consentproject, written for handoff.Composer stays unaware of GitHub throughout.
What to read
.drive/projects/deployment-identity/spec.mdandplan.md: the immediate fix, in three slices..drive/projects/node-identity-and-consent/design-notes.md: every decision with its reasoning and the alternatives rejected..drive/projects/node-identity-and-consent/assets/user-scenarios.md: the user scenarios the design covers.Alternatives considered
--name. It hides the identity from the repository, so a local deploy and a CI deploy of the same file can reach different Projects.logicalIdon the platform. An opaque id breaks on fork or delete; a readable slug with a suffix invites people to "tidy" it, which creates a new Project.renamedFrom, Terraform-stylemovedblocks). The ORM tried and rejected this: the code would keep describing a system that no longer exists, when only the command doing the change needs the information.Agent: parmenides-88
🤖 Generated with Claude Code