|
| 1 | +## Context |
| 2 | + |
| 3 | +The private gateway now rejects value-bearing deploy secret specs and exposes a value-free contract: |
| 4 | + |
| 5 | +```ts |
| 6 | +secrets?: { |
| 7 | + require?: string[]; |
| 8 | + delete?: string[]; |
| 9 | +} |
| 10 | +``` |
| 11 | + |
| 12 | +The public repo still has several old-shape entry points: SDK `SecretsSpec.set` / `replace_all`, MCP `deploy` schema, CLI `deploy apply` examples, `apps.bundleDeploy` translation, legacy `bundle_deploy` docs, `SecretSummary.value_hash`, and skills/docs that tell agents a hash can verify secret values. If left unchanged, agents will create manifests that fail with `INVALID_SPEC`, or worse, will keep writing secret values to manifests/repos while believing the deploy layer owns them. |
| 13 | + |
| 14 | +This change is client/agent contract work only. Backend KMS, migrations, Bugsnag redaction, `value_hash` removal at the API, and warning generation are treated as shipped by the private commits. The public repo should consume that contract cleanly. |
| 15 | + |
| 16 | +## Goals / Non-Goals |
| 17 | + |
| 18 | +**Goals:** |
| 19 | + |
| 20 | +- Make the SDK `ReleaseSpec` and all public deploy surfaces value-free for secrets. |
| 21 | +- Preserve a smooth coding-agent workflow: set secret values out-of-band, then deploy with `secrets.require`. |
| 22 | +- Surface `warnings: WarningEntry[]` consistently enough that agents can branch on `MISSING_REQUIRED_SECRET`. |
| 23 | +- Remove `value_hash` from all public types, formatted output, docs, and examples. |
| 24 | +- Update legacy compatibility paths so they never put secret values into a `ReleaseSpec`. |
| 25 | +- Keep the implementation small and aligned with existing SDK/CLI/MCP shim patterns. |
| 26 | + |
| 27 | +**Non-Goals:** |
| 28 | + |
| 29 | +- No gateway/database/KMS/migration work in this repo. |
| 30 | +- No new "get secret value" API. |
| 31 | +- No Lambda environment variable redesign. |
| 32 | +- No broad new secrets dashboard/browser-management feature. Closed issue #10 is related context, not scope. |
| 33 | +- No local manifest validation tool in this change. Open issue #151 remains a follow-up that can build on the new schema. |
| 34 | + |
| 35 | +## Decisions |
| 36 | + |
| 37 | +### D1. `ReleaseSpec.secrets` is declaration-only |
| 38 | + |
| 39 | +Change `SecretsSpec` to: |
| 40 | + |
| 41 | +```ts |
| 42 | +export interface SecretsSpec { |
| 43 | + require?: string[]; |
| 44 | + delete?: string[]; |
| 45 | +} |
| 46 | +``` |
| 47 | + |
| 48 | +Remove `set` and `replace_all` from SDK types and docs. SDK `validateSpec` should reject obvious old-shape objects locally with `Run402DeployError` code `INVALID_SPEC`, `phase: "validate"`, and `resource: "secrets.set"` or `"secrets.replace_all"` before network work. This gives agents a clean local error instead of an opaque gateway rejection. |
| 49 | + |
| 50 | +Alternatives considered: |
| 51 | + |
| 52 | +- Leave old fields in the type as deprecated optional fields and let the gateway reject them. Rejected because TypeScript would still teach agents the unsafe shape. |
| 53 | +- Translate `set` to `require` silently. Rejected for plain `deploy.apply` because the values would still appear in manifests and callers would assume deploy is setting them. |
| 54 | + |
| 55 | +### D2. Secret values move through the secrets namespace, not deploy specs |
| 56 | + |
| 57 | +The canonical two-step workflow is: |
| 58 | + |
| 59 | +1. `r.secrets.set(project, key, value)` or `run402 secrets set <project> <key> <value>`. |
| 60 | +2. `r.deploy.apply({ project, secrets: { require: [key] }, ... })`. |
| 61 | + |
| 62 | +Do not add a new deploy helper in the first slice. The two-step primitive flow is already simple, transparent, and available across SDK, CLI, and MCP. The "ultimate DX" move is to make the docs and errors so obvious that agents choose the safe two-step path without inventing value-bearing manifests. |
| 63 | + |
| 64 | +Alternatives considered: |
| 65 | + |
| 66 | +- Add `--set-secret KEY=VALUE` to `deploy apply`. Rejected for v1 because it encourages shell-history leakage and mixes two authorities into one command. |
| 67 | +- Require every caller to hand-roll the two-step flow. Accepted for v1 because it keeps authority boundaries clear and uses tools that already exist. |
| 68 | + |
| 69 | +### D3. Legacy compatibility surfaces pre-set or fail safely |
| 70 | + |
| 71 | +`apps.bundleDeploy`, legacy CLI deploy shims, and `bundle_deploy` MCP can still accept older options for non-secret resources, but they must not translate secret values into `ReleaseSpec.secrets.set`. |
| 72 | + |
| 73 | +When a compatibility surface receives secret values and has a project-scoped SDK available, it should: |
| 74 | + |
| 75 | +- call `secrets.set` for each `{key, value}`; |
| 76 | +- add those keys to `ReleaseSpec.secrets.require`; |
| 77 | +- proceed with deploy; |
| 78 | +- surface that secret writes happened before deploy and are not part of release rollback semantics. |
| 79 | + |
| 80 | +If the surface cannot safely pre-set secrets, it should return a structured/actionable error telling the agent to call `set_secret` or `run402 secrets set` first, then deploy with `secrets.require`. |
| 81 | + |
| 82 | +Alternatives considered: |
| 83 | + |
| 84 | +- Remove all legacy secret options immediately. Rejected because a compatibility shim can preserve most agent workflows while still keeping the deploy spec value-free. |
| 85 | +- Try to emulate old `replace_all` semantics. Rejected because backend deploy specs no longer carry values and exact replacement semantics are not representable without a separate admin operation. |
| 86 | + |
| 87 | +### D4. `WarningEntry` is typed and propagated through apply |
| 88 | + |
| 89 | +Add: |
| 90 | + |
| 91 | +```ts |
| 92 | +export interface WarningEntry { |
| 93 | + code: string; |
| 94 | + severity: "info" | "warn" | "high"; |
| 95 | + message: string; |
| 96 | + affected: string[]; |
| 97 | + requires_confirmation: boolean; |
| 98 | + confidence?: "heuristic"; |
| 99 | + details?: Record<string, unknown>; |
| 100 | +} |
| 101 | +``` |
| 102 | + |
| 103 | +`PlanResponse` SHALL include `warnings: WarningEntry[]`. `DeployResult` SHOULD include the plan warnings as `warnings: WarningEntry[]` so one-shot `deploy.apply` callers do not have to drop to `plan` to inspect non-fatal problems. `DeployEvent` should add a `plan.warnings` event when the array is non-empty; CLI event stderr and MCP markdown should display the codes and affected keys. |
| 104 | + |
| 105 | +For `MISSING_REQUIRED_SECRET`, agents can branch on either `warning.code === "MISSING_REQUIRED_SECRET"` or `warning.details?.missing_keys`. |
| 106 | + |
| 107 | +Alternatives considered: |
| 108 | + |
| 109 | +- Keep warnings only on `plan()`. Rejected because `apply()` is the recommended agent primitive. |
| 110 | +- Use `string[]`. Rejected because agents need stable fields. |
| 111 | + |
| 112 | +### D5. `value_hash` disappears everywhere |
| 113 | + |
| 114 | +Remove `value_hash` from SDK `SecretSummary`, SDK tests, CLI/MCP formatted output, docs, and skills. `list_secrets` should show only keys and timestamp fields if the gateway returns them; if timestamps are absent in current test fixtures, key-only output is enough. |
| 115 | + |
| 116 | +Alternatives considered: |
| 117 | + |
| 118 | +- Keep `value_hash?: string` as optional for older gateways. Rejected because the goal is to stop teaching value-derived verification. Unknown extra runtime fields can be ignored without typing them. |
| 119 | + |
| 120 | +### D6. Docs are part of the contract |
| 121 | + |
| 122 | +Use `documentation.md` as the checklist. Every agent-facing doc surface must teach: |
| 123 | + |
| 124 | +- never put secret values in deploy manifests; |
| 125 | +- use `set_secret` / `run402 secrets set` / `r.secrets.set` to write values; |
| 126 | +- use `secrets.require[]` to assert deploy-time dependencies; |
| 127 | +- use `secrets.delete[]` to remove keys at activation; |
| 128 | +- `list_secrets` is keys-only and cannot verify values by hash; |
| 129 | +- secret values still appear as function environment variables at runtime, so do not over-promise total runtime secrecy. |
| 130 | + |
| 131 | +Related issues should be referenced in the implementation notes, especially #151 for future local manifest validation and #198 for help/doc drift. |
| 132 | + |
| 133 | +## Risks / Trade-offs |
| 134 | + |
| 135 | +[Risk: compatibility shim writes secrets before a deploy that later fails] -> Mitigation: document non-atomicity, return clear output, and keep raw `deploy.apply` declaration-only. |
| 136 | + |
| 137 | +[Risk: agents miss warnings because they only call `deploy.apply`] -> Mitigation: carry `warnings` onto `DeployResult` and emit a `plan.warnings` event. |
| 138 | + |
| 139 | +[Risk: docs still contain old value-bearing examples] -> Mitigation: add targeted `rg`-based tests or sync assertions for `secrets.set`, `replace_all`, and `value_hash` in public docs. |
| 140 | + |
| 141 | +[Risk: TypeScript accepts old runtime JSON through `as any`] -> Mitigation: SDK `validateSpec` rejects old keys before uploads or plan calls. |
| 142 | + |
| 143 | +[Risk: old gateway fixtures still include `value_hash`] -> Mitigation: update tests to prove the client ignores absent hashes and docs never rely on them. |
| 144 | + |
| 145 | +## Migration Plan |
| 146 | + |
| 147 | +1. Update SDK types/validation/warning propagation first; tests should fail on old secret shape. |
| 148 | +2. Update compatibility shims and CLI/MCP schemas to use `require` / `delete`. |
| 149 | +3. Remove `value_hash` formatting and tests. |
| 150 | +4. Update docs, skills, help snapshots, and sync tests. |
| 151 | +5. Run build, focused SDK/MCP tests, CLI e2e/help tests, sync tests, skill tests, and a final `rg` scan for old contract strings. |
| 152 | + |
| 153 | +Rollback is a source rollback only. The public client must match the deployed gateway; if the backend contract is live, reintroducing old fields is not a safe rollback. |
| 154 | + |
| 155 | +## Open Questions |
| 156 | + |
| 157 | +- Should a future local manifest-validation tool from #151 also validate that every `secrets.require[]` key exists locally/remotely before deploy? This change only updates the live deploy client contract. |
0 commit comments