diff --git a/AGENTS.md b/AGENTS.md index 804cdd5344..4ef2efa308 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -24,6 +24,7 @@ Detailed guidance lives in `docs/contributing/` and topic-specific guides under |------|-------------| | [Go Code](docs/contributing/go-code.md) | Changing Go code under `cmd/` or `internal/` — covers mint sync, coverage, vet, e2e tests, concurrency testing, and suite-timeout policy | | [Behaviour Testing](docs/guides/dev/behaviour-testing.md) | Modifying behaviour test repo provisioning, fork handling, or workflow dispatch — covers forge API constraints (`auto_init`, fork name derivation, Actions readiness, CI timeout budgeting) | +| [Workflow Contracts](docs/contributing/workflow-contracts.md) | Changing GHA reusable workflows — covers dispatch sync, secret/input threading across installation-mode chains, and review rules | | [Shell Scripting](docs/contributing/shell-scripting.md) | Writing or reviewing shell scripts — covers `gh api --paginate` pitfalls and jq patterns | | [Forge Abstraction](docs/contributing/forge-abstraction.md) | Adding forge operations — covers `forge.Client` interface rules | | [CEL Triggers](docs/contributing/cel-triggers.md) | Writing or reviewing harness `trigger` CEL expressions or `.feature` CEL filters — covers normalized transition kinds | diff --git a/docs/contributing/go-code.md b/docs/contributing/go-code.md index ead32d54f6..7d4df677c6 100644 --- a/docs/contributing/go-code.md +++ b/docs/contributing/go-code.md @@ -19,7 +19,7 @@ The `internal/mintcore/` module is shared between the mint and devmint. Its file 2. **Register in `embeddedMintFiles`:** If the file will be included in the GCF bundle — either no build tag (e.g., `config.go`) or `//go:build !js` (e.g., `sts_verifier.go`, `gcp_pem.go`, `wif.go`) — add it to `embeddedMintFiles` in `internal/dispatch/gcf/provisioner.go` and to the `go:embed` directive. 3. **Add to `gcfSkip`:** If the file should NOT be in the GCF bundle — Worker-only files (`//go:build js`) or standalone-mint-only files — add it to the `gcfSkip` map in `TestEmbeddedMintSource_MatchesOriginal` in `provisioner_test.go` instead of `embeddedMintFiles`. The three current entries are `fetch_js.go` and `pem_js.go` (Worker-only, `//go:build js`) and `file_pem.go` (standalone-mint-only, `//go:build !js`). -**Dispatch workflows:** The scaffold `dispatch.yml` (at `internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml`) and the repo's `reusable-dispatch.yml` (at `.github/workflows/reusable-dispatch.yml`) share identical routing logic for different installation modes (per-org vs per-repo). When changing the jq payload construction, stage routing, or input/secret threading in one, apply the same change to the other. The GitLab scaffold has its own dispatch template at `internal/scaffold/fullsend-repo-gitlab/.gitlab/ci/fullsend-dispatch.yml` — it follows the same two-path model (native MR events + cron-polled events) but constructs a NormalizedEvent v1 payload (ADR 0061) from GitLab CI variables. Stage routing uses shell checks annotated with equivalent CEL trigger expressions; when built-in harness triggers land (#2896-2901), the routing can be replaced by `fullsend dispatch --input-driver json`. +**Dispatch workflows:** See [Workflow Contracts](workflow-contracts.md) for dispatch sync rules, secret/input threading across installation-mode chains, and review instructions. **Interface documentation:** When extending a Go interface with new methods (e.g., adding methods to `ci.Driver` in `pkg/behaviourtest/drivers/ci/driver.go`), check `docs/guides/dev/` for documentation that lists or enumerates the interface's methods (e.g., `behaviour-drivers.md`). If found, update the method list to include all current methods, not just the newly added one. The `lint-interface-doc-sync` pre-commit hook enforces this for `ci.Driver`. diff --git a/docs/contributing/workflow-contracts.md b/docs/contributing/workflow-contracts.md new file mode 100644 index 0000000000..39447db991 --- /dev/null +++ b/docs/contributing/workflow-contracts.md @@ -0,0 +1,14 @@ +# Workflow Contracts + +**Dispatch sync:** The scaffold `dispatch.yml` (at `internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml`) and the repo's `reusable-dispatch.yml` (at `.github/workflows/reusable-dispatch.yml`) share identical routing logic for different installation modes (per-org vs per-repo). When changing the jq payload construction, stage routing, or input/secret threading in one, apply the same change to the other. The GitLab scaffold has its own dispatch template at `internal/scaffold/fullsend-repo-gitlab/.gitlab/ci/fullsend-dispatch.yml` — it follows the same two-path model (native MR events + cron-polled events) but constructs a NormalizedEvent v1 payload (ADR 0061) from GitLab CI variables. Stage routing uses shell checks annotated with equivalent CEL trigger expressions; when built-in harness triggers land (#2896-2901), the routing can be replaced by `fullsend dispatch --input-driver json`. + +**Secret threading:** GHA reusable workflows do not inherit secrets — they must be explicitly forwarded by every caller. (Repository and organization-level `vars` are automatically visible inside called reusable workflows and do not need forwarding.) When any reusable workflow (`.github/workflows/reusable-*.yml`) adds a new `secrets:` or `inputs:` (`workflow_call`) entry, trace both installation-mode chains and ensure every hop forwards the new entry: + +- **Per-org chain** (deprecated per [ADR 44](../ADRs/0044-deprecate-per-org-installation-mode.md)): scaffold thin callers (`internal/scaffold/fullsend-repo/.github/workflows/.yml`) → reusable workflow (`.github/workflows/reusable-.yml`). Note: `dispatch.yml` also participates in this chain for routing and event-derived inputs (forwarded via `gh workflow run -f ...`), though not for secrets — thin callers pull secrets from their own repo/org context. This entire chain is scheduled for removal per ADR 44. +- **Per-repo chain:** shim template → `reusable-dispatch.yml` (stage logic for triage/code/review/fix/retro/prioritize is inlined directly as jobs per [ADR 62](../ADRs/0062-dispatch-version-skew.md) — there is no separate `reusable-.yml` hop for per-repo mode; thread new secrets/inputs into the relevant inline job). Standalone `reusable-.yml` files still exist but now serve only the per-org chain until it is removed per ADR 44. + +**Silent failures and required-flag consistency:** Omitting a secret that is `required: true` at every hop in the chain fails loudly at workflow-call validation time and self-enforces. However, a secret whose `required` flag is `false` at any upstream hop can still arrive as an empty string at a downstream `required: true` consumer — GitHub Actions' required-secret validation only checks key presence, not that the resolved value is non-empty. For example, `FULLSEND_GCP_WIF_PROVIDER` is `required: false` in `reusable-dispatch.yml` but `required: true` in every downstream `reusable-.yml`, so an installer that never sets it satisfies the key-presence check while the actual value is empty. Treat a missing forwarding hop the same as a missing sync — it is a correctness bug, not a cosmetic issue. Required-flag consistency across the *whole* chain matters, not just the flag at the final consumer. + +**Security — consuming threaded inputs:** When a newly-threaded entry carries user- or event-controlled data, consume it via `env:` in the final `run:` step — never interpolate `${{ ... }}` directly into a shell block (see the Security note atop `reusable-dispatch.yml`). This prevents the GHA script-injection class of bugs the project defends against elsewhere. + +**When reviewing PRs:** If a diff adds or renames a `secrets:` or `inputs:` entry in a reusable workflow, check that all callers in both chains have been updated. Flag a missing forwarding hop as a medium-severity or higher finding. New secrets/inputs must be forwarded only to the hop(s)/stage(s) that need them — do not use `secrets: inherit` as a substitute for explicit, scoped forwarding (`OTEL_EXPORTER_OTLP_TRACES_HEADERS`, for example, is deliberately forwarded only to the `triage` job in `reusable-dispatch.yml`). `workflow_call_alignment_test.go` already automates much of this verification — see `TestWorkflowCallInputAlignment` (validates required inputs/secrets are threaded through both chains) and `TestOTELHeadersSecretThreading` (bespoke test for optional secrets). For new optional secrets/inputs, extend those tests or add a similar one rather than relying solely on manual tracing.