Fallout-build / Fallout-build/Fallout
Review ADR-0001 / ADR-0002 — CD primitive patterns + secret conventions
- Dominant language
- C#
- Stars
- 154
- Forks
- 19
- Avg merge
- 1d 22h
- Merged PRs (30d)
- 15
Description
Two ADRs were drafted in the same pass during early CD planning:
- [ADR-0001 — CD primitives: attributes for config, tasks for state](../blob/main/docs/adr/0001-cd-primitives-attributes-vs-tasks.md)
- [ADR-0002 — Cross-provider auth and secret conventions](../blob/main/docs/adr/0002-cross-provider-auth-and-secret-conventions.md)
Both are **Proposed**. This issue tracks the review pass that promotes them to **Accepted** (or supersedes them with a new ADR).
## What this is
Architectural framing for the v13 CD work, written before substantial code lands. Reviewing now lets the first concrete CD implementation slot into a settled model rather than retrofit one.
## What review should cover
**ADR-0001 — the two patterns split**
- Is the file-shaped vs API-shaped split the right cut? (Alternative: collapse everything to tasks, lose the declarative environment story.)
- Hybrid attribute (`[GitHubEnvironment]`, `[OctopusProject]`) — capped at *stable* config, dynamic stuff in tasks. Is that line defensible, or do we need a sharper rule?
- The monorepo-fan-out sketch (12 agents + 3 UIs + N libraries) — does inventory-in-static-fields hold at that scale, or is there pressure for a richer DSL?
- Octopus-as-v12-sample-plugin — agreed it's the right validation vehicle for the plugin SDK?
**ADR-0002 — the resolution chain + naming**
- ✅ Canonical `SCREAMING_SNAKE` derived from PascalCase field name — lock this in across all providers?
- ✅ Plugins receive resolved values, never raw stores — agreed this is the right trust boundary for v12 SDK?
- ⚠️ **Log masking** — the ADR flags that no explicit `RegisterSensitiveValue` / output scrubber is wired to `[Secret]`. Needs verification before either ADR moves to Accepted. If missing, a small framework PR adds it; if present, the ADR should cite the file.
- ⚠️ **Windows / Linux `CredentialStore`** — macOS-only today. Non-blocking for the ADRs but worth surfacing (in ADR-0002's Open Questions); a separate issue (#180) should land before plugin authors hit it.
- Naming escape hatch (`[Parameter("custom_name")]`) — enough for the cases that matter, or do we need provider-side mapping dictionaries?
**Cross-cutting**
- Any CD shapes not sketched that would invalidate either ADR? Specifically: K8s-native deployments (Helm releases, ArgoCD applications), serverless (AWS Lambda, Cloudflare Workers), package-manager promotion (NuGet → public feed gates).
## Outcome
- Either ADR → **Accepted** via this review, date updated.
- Or one/both → **Superseded by NNNN**, with the replacing ADR explaining the divergence.
Don't silently rewrite either ADR. Per `docs/adr/README.md`, ADRs are history; corrections happen via new ADRs.
## Related
- v13 milestone (this one)
- v12 milestone — plugin SDK consumes both ADRs' decisions
- RFC #106 — CD vision (parent)
- RFC #113 — deployment agent (consumes the ADR-0002 resolution chain)
Contributor guide
Assessment
This issue has not been assessed yet.