Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
08cc2f1
docs(drive): platform-variables slice 2 spec and dispatch plan
wmadden-electric Nov 6, 2026
b31a0b9
feat(prisma-cloud): the input document carries $platform markers, and…
wmadden-electric Nov 6, 2026
29006e9
feat(prisma-cloud): one checker for platform variable names, and an e…
wmadden-electric Nov 6, 2026
cfdb120
feat(prisma-cloud): deploy preflight writes no variable, and fails on…
wmadden-electric Nov 6, 2026
43671a5
feat(prisma-cloud): the deploy lists every platform variable with no …
wmadden-electric Nov 6, 2026
2c519c0
fix(prisma-cloud): no doc comment or message says the deploy fills a …
wmadden-electric Nov 6, 2026
b110774
docs(prisma-cloud): ADR-0064, and every record says the deploy fills …
wmadden-electric Nov 6, 2026
0800bd6
docs(drive): slice 2 spec says what preflight runs after, and which r…
wmadden-electric Nov 6, 2026
45706b5
ci(e2e): the storefront deploy job sets AUTH_SIGNING_SECRET on the pl…
wmadden-electric Nov 6, 2026
0ba55b1
fix(prisma-cloud): core carries the preflight findings to the deploy …
wmadden-electric Nov 6, 2026
168f2fa
docs(prisma-cloud): records say how the findings reach the deploy rep…
wmadden-electric Nov 6, 2026
01cb893
fix(cli): redaction keeps the <value> placeholder in a command that s…
wmadden-electric Nov 6, 2026
c6d9b2d
style(cli): format the redaction test
wmadden-electric Nov 6, 2026
8b553a8
fix(cli): redaction keeps only an exact <value> placeholder
wmadden-electric Nov 6, 2026
48b5625
Merge remote-tracking branch 'origin/main' into claude/platform-varia…
wmadden-electric Nov 6, 2026
f6f5df6
fix(prisma-cloud): round-2 review fixes for slice 2
wmadden-electric Nov 6, 2026
a000253
test(integration): an unset per-key param with a default no longer st…
wmadden-electric Nov 6, 2026
3b45c3a
fix(prisma-cloud): round-3 review fixes for slice 2
wmadden-electric Nov 6, 2026
2717472
Merge remote-tracking branch 'origin/main' into claude/platform-varia…
wmadden-electric Nov 10, 2026
95632e0
docs(prisma-cloud): say the deploy does not copy local values to plat…
wmadden-electric Nov 10, 2026
3ac355d
ci(e2e): the first-deploy check is a script, not inline shell
wmadden-electric Nov 10, 2026
ee044ad
docs(drive): review questions that catch shortcuts, comments, inline …
wmadden-electric Nov 10, 2026
c586b2b
ci(e2e): match variable names as words instead of building a regular …
wmadden-electric Nov 10, 2026
5de000e
test(e2e): prove the signing secret reached auth with an HMAC of a ra…
wmadden-electric Nov 10, 2026
f1a1b6d
docs(platform-variables): state the deploy choice as current and in p…
wmadden-electric Nov 10, 2026
8d815b3
fix(input-document): plain boot errors, and $secret is no longer escaped
wmadden-electric Nov 10, 2026
762c480
fix(core): plain wording in preflight errors and test names
wmadden-electric Nov 10, 2026
4a08e0f
fix(prisma-cloud): one formatter for missing variables, no re-exports…
wmadden-electric Nov 10, 2026
b1c1e3d
fix(cli): set the findings variable through preflightFindingsEnv on e…
wmadden-electric Nov 10, 2026
72dc631
test(storefront-auth): prove the signing key with a key check value, …
wmadden-electric Nov 10, 2026
1fddd57
ci(e2e): one script runs the first deploy and sets the platform varia…
wmadden-electric Nov 10, 2026
a348f6c
docs(platform-variables): quote the rewritten param error and the at-…
wmadden-electric Nov 10, 2026
ac2414d
test(integration): prisma dev names a missing variable with "read by …
wmadden-electric Nov 10, 2026
d9eb1c1
Merge remote-tracking branch 'origin/main' into claude/platform-varia…
wmadden-electric Nov 10, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 46 additions & 17 deletions .drive/CODE-REVIEW.md
Original file line number Diff line number Diff line change
@@ -1,24 +1,53 @@
# Code review context

Project-specific review focus areas for this repo. Reviewers load this file
before reviewing a diff.
Project-specific review questions for this repo. Both review passes load this file before reviewing a diff, and answer every question below for every file in the diff.

## Jargon sweep (every review)
## Cover the whole diff

Sweep every piece of human-readable text in the diff — comments, test
descriptions, error messages, log output, doc comments — for jargon and
overlong comments. Implementer models (Sonnet and Opus are the worst
offenders) write contracted, highly technical, incomprehensible jargon all
over the place, and especially write extremely long comments.
Review every changed file, not only the product code under `packages/`. CI workflows and actions under `.github/`, scripts under `scripts/`, examples under `examples/`, docs, ADRs, the glossary, skills and the PR title and description all count. A review that read only the source files has not reviewed the PR.

Flag and rewrite:
For each file, ask:

- Invented or coined terms where a plain description of what happens works.
- Contracted, telegraphic phrasing that has to be reread to be understood.
- Long comments. A comment should state only what the code cannot express;
most should be deleted or replaced with clearer code.
- Test descriptions written in implementation vocabulary instead of plain
English describing the observable behavior.
- Did I read every added and changed line, and every unchanged line this PR made wrong?
- If this PR edits a line that already held a shortcut or a bad comment, did I flag it? A PR that touches a line owns it.

The standard: a reader outside this session understands the text on the
first read.
## Comments

Implementer models write long comments that explain, justify or narrate. Most should not exist.

- Does any comment explain or justify a shortcut? Flag it, and say what the proper code is. The fix is the code, not a better comment.
- Does any comment tell history ("used to", "before this change", "was renamed"), restate what the code says, or narrate a decision? Delete it.
- Does any comment run longer than two lines? Ask what the code cannot express, keep only that, and delete the rest.
- Is a doc comment on something that is not a user-facing interface? Delete it. On a user-facing interface, is it terse?

## Shortcuts

- Is one value hard-coded in two places, such as the app and CI, and kept in step by a comment? Generate it in one place and pass it, or prove the behaviour without a shared constant.
- Does a test or a CI check compare against a constant it also wrote, so it proves nothing about the behaviour? It should exercise the real behaviour, such as signing a challenge with a key instead of comparing the key to a known string.
- Is a special case, workaround or "temporary" path explained by a comment instead of removed or recorded in the project's `deferred.md`?

## CI, workflows and scripts

- Does any `run:` step in `.github/` hold more than one command, or any shell logic: captured output, exit-code checks, `if`, loops, `grep`? Move it into a script under `scripts/` and make the step one line.
- Does a script print a secret, a token or a value it was given? It must not.
- Does every new script follow the existing ones in `scripts/`: TypeScript run with `bun`, a short usage line, plain error messages?

## Wording

- Is every human-readable string plain English that a reader outside the team understands on the first read? That covers comments, error messages, log lines, test names, docs and the PR text.
- Flag invented or coined terms where a plain description of what happens works.
- Flag contracted, telegraphic phrasing that has to be read twice.
- Flag the banned words: "load-bearing", "gate", "smoking gun", "belt and suspenders".
- Does a test name describe the observable behaviour, not the implementation?
- Does any record, guide, comment or PR text state a current choice as a permanent rule, with "never" and no reason? State what is decided, why, and whether it is expected to change. Keep "never" for real invariants, such as a secret never appearing in an error.
- Is any markdown prose hard-wrapped? One paragraph is one line.

## The PR title and description

Read them as a teammate with none of the author's context.

- Does the title say what changes for a user, not name a mechanism?
- Does the description open with a concrete example, such as code and the output it gives?
- Does it state the decision next, then build the explanation step by step?
- Does it end with the alternatives considered, rather than lead with them?
- Does every claim in it match the code?
11 changes: 9 additions & 2 deletions .drive/projects/platform-variables/deferred.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,24 @@ Each needs a Management API change. None blocks this project (design section 3).
- **A platform record of which variables each service reads.** Composer's input documents already name them; the target would also send them to the platform, which could then deliver only those. Until then every deployment receives every row of its class.
- **A service scope.** Today a name resolves at two scopes, project class and branch, and preflight lists both. Will expects the platform to add a service scope. When it does, preflight's existence check gains one list call per reader; nothing else in the deploy changes, because the node never says where its value lives and preflight is the only part of the deploy that knows which scopes exist. `prisma dev` supplies one value per name from local files, so reproducing two readers with two values locally needs a per-reader local source, decided when the scope exists.
- **A void value.** Let an operator set a variable to "deliberately nothing", so the deploy report stops listing an intended absence as not set. Today the platform refuses an empty value.
- **Workspace variables and rules.** Values shared across projects, with rules for which branches see them. Composer creates no values, so this needs nothing from Composer.
- **Workspace variables and rules.** Values shared across projects, with rules for which branches see them. Composer does not create platform variable values today, so this needs nothing from Composer.
- **A topology entry for a platform variable node.** The platform's application topology has a fixed list of node kinds, owned by the Portal team. The node is not sent to it until the platform has a kind for it.

## Later Composer work

- **Destroying production deletes the operator's platform variables.** Destroying an app's production environment deletes its Prisma Cloud project, and with it every variable the operator set; the platform never returns a value, so an operator who kept no copy must make a new secret. Options: keep the project when it holds rows that are not Composer's, or warn before deleting. Waits because destroy's behaviour is not this slice's and needs its own decision.
- **An author-declared per-reader variable,** one that maps to the service scope the platform is expected to add, so that two readers of one name are guaranteed different values. Not built: where a value lives is the operator's choice on the platform, and nobody has a case that needs the author to force it. Wait for a real case.
- **A warning when the app names a variable another module expects under a different name.** Not needed: the deploy report lists every name with its readers, so a module wired to the wrong name shows up there.
- **An edge kind for "a service reads a platform variable".** Core's edges are dependencies (ADR-0063 names two relationships: depends on, exposes). The reads relation is recorded as a list beside each service's binding, not as an edge, because adding an edge kind changes core's graph types and the platform's edge families at once. Reason to wait: nothing reads the relation as an edge yet; the deploy report and errors read the list.
- **`schema` on a platform secret.** A secret box whose content is not text is undefined in this design and in module-input's. A `secret(inner)` field already checks a secret's text at boot. Needs a generic box type first.
- **Removing `envSecret()` and `envParam()`** after their deprecation.
- **A seeding command** that copies values from local files to the platform. Shelved by Will on 2026-11-05.
- **An explicit way to set platform variables through `deploy`.** The operator names the values and the environments they go to, and the deploy writes those rows. Expected later. Not now: it needs its own design, and an implicit copy of local values is what slice 2 removed.
- **Isolating a service's environment on Prisma Cloud** without a platform record of who reads what, for example by having boot remove variables a service does not read. Not planned: it would hide variables an app reads directly from `process.env`.
- **Reading a deployment's logs from `prisma deploy`.** Composer does not tail deployment logs, so a boot error on the platform is read with `prisma service logs <service>`. The guide says so; tailing is a later change.
- **A value given by a resource that also provides a contract.** `provision(resource)` flattens a contract onto the returned object, so a `value` member would collide. Allowed only on a resource with no contract until a second case needs both.
- **A separate export for `envParam()` in per-key params.** One function serves both forms today, so its `@deprecated` tag strikes through the per-key form too, which is not deprecated. A second export, with the deprecation only on the binding form, gives a clean signal. Deferred from slice 2's code review (F03): the replacement for the binding form arrives with slice 3.
- **Renaming `absentSecrets`** in the serializer and the dev payload to say unset variables. In slice 2 every document marker is an `envSecret()`, so the name is accurate. Slice 3 adds markers for plain variables, and the rename belongs there.
- **A report channel for preflight checks that are not about platform variables.** `PreflightInput.report` carries platform-variable findings and a count only. Designing a general channel waits for a second preflight check that needs one.
- **The CI script's deep imports from source paths.** `scripts/e2e-first-deploy.ts` imports from `packages/1-prisma-cloud/0-lowering/lowering/src/`, as four other scripts already do. Changing it is a change to that existing practice, not to this project.
- **The platform variables with no value in the `--report` run report.** The printed deploy report and the JSON output carry the list; the run report file holds steps, timings and the failure cause, and does not. Found in slice 2 QA. Reason to wait: no CI consumer reads the list from that file yet.
- **The one-meaning-per-name check misses an `envParam()` passed indirectly.** The check sees `envParam()` only in a service binding or in per-key params. An `envParam()` passed through a module's input or a `resolve()` dependency becomes text at Load, so a clash with a root input field of the same name is not caught. Fixing it needs Load to keep the source's name and location, which is a core change outside slice 2. Found in PR #426 review.
Loading
Loading