diff --git a/.agents/review/decisions.md b/.agents/review/decisions.md deleted file mode 100644 index 889bbe89..00000000 --- a/.agents/review/decisions.md +++ /dev/null @@ -1,115 +0,0 @@ -# Review decisions - -## STRUCTURE-001: Consumer workspace versions - -Every workspace in a standards consumer is internal and must use version `0.0.0`. Versioned publishable workspaces are outside the consumer structure contract because consumers do not expose workspace packages as independently released artifacts. - -## STRUCTURE-002: Supported workspace declarations - -Workspace declarations must be arrays of literal paths or one-level `/*` patterns. Broader Bun glob patterns and object-shaped workspace schemas are rejected explicitly because the structure gate intentionally supports a small, deterministic consumer layout contract. - -## SYNC-001: Checked-in sync policy hard cutover - -Starting with CLI 0.7.0, `sync-standards.local.json` is the only standards-sync cadence and ref policy source. The canonical workflow and CLI do not consult `STANDARDS_AUTO_SYNC` or `STANDARDS_SYNC_REF`; consumers must upgrade the package and lockfile and materialize any required policy in the same migration change. - -## DEPENDABOT-001: Deliberately lean local overlay - -The repo-owned Dependabot overlay is additive but intentionally not a general policy override. It may define new ecosystem update blocks, top-level private registries, and `ignore` or `registries` additions on a canonical normalized target. Matching blocks reject labels, groups, cooldowns, pull-request limits, and every other policy key; broader per-repository policy must be proposed as an explicit seam decision. - -## POLLER-001: Shared service identity risk - -The fix poller and its approved Codex runs share one host service identity, HOME, and service PAT. Codex therefore inherits the PAT and technically holds all capabilities that credential grants; prompt restrictions and the poller's approval, claim, target, protected-path, and pre-write publication revalidation are workflow safeguards, not credential isolation or a reduced capability boundary. This capability exposure is an accepted risk because only an admin- or maintain-approved exact issue revision or pull-request head may run, while the poller retains commit pushes, protocol labels, final reports, and ready transitions. This finding is explicitly discarded: do not reintroduce token scrubbing as its fix. Reopen the decision if untrusted users can approve runs, the PAT permission or repository scope broadens, agent writes escape their approved repository target, the poller stops revalidating immediately before its writes, or the host threat model begins to require credential isolation. The host infrastructure repository owns the identity, PATH, token wiring, lingering, and declarative unit deployment. - -## RELEASE-001: Installed manifest version asserts CLI capability - -Under the frozen non-hostile-consumer threat model, the installed `@davidvornholt/standards` manifest version plus the frozen lockfile is the capability assertion. The consumer already controls that dependency and executes its code, so extra package-name, bin, or capability probes do not establish official identity; defending against a falsely versioned or malicious substitution is out of scope. - -## RELEASE-002: Standards CLI bootstrap error architecture - -`packages/standards-cli` deliberately uses plain TypeScript async code, discriminated result unions, and a minimal runtime dependency surface because its published bin bootstraps dependency-free consumers through `bunx`; adding Effect would either defeat that bootstrap boundary or split the package into two architectures. Reviews may challenge this decision only if the CLI no longer bootstraps dependency-free consumers, its runtime moves to a separately installed package, or the minimal-dependency premise otherwise changes. - -## TOOLING-001: Root-owned Biome pin - -`@biomejs/biome` is pinned only at the repository root (and `template/package.json` for consumers); workspaces deliberately do not declare it. Workspace lint scripts resolve the root-hoisted Biome executable, while `packages/standards-cli/src/template-biome.test.ts` invokes that executable by its root path; a missing install fails at invocation. Reviews must not request per-workspace `@biomejs/biome` declarations; the pin moves with the root/template dependency-hold policy. - -## GITHUB-SETTINGS-001: No durable settings credential - -The fixed GitHub-hosted `check` aggregator job verifies settings with the workflow token and provisions no credential of its own, never a token-selection mechanism. Its grants are `contents: read` and `issues: read`: Issues read exposes private issue content beyond the declared-label metadata the checker needs, and that read-only expansion is an accepted tradeoff, because a private-repository probe on PR #94 confirmed that an Administration-plus-Metadata PAT can read merge settings through GraphQL but receives `FORBIDDEN` for `repository.labels`, so GraphQL offers no Metadata-only label path. - -The credential was originally a durable repository-scoped fine-grained PAT stored as `ci.github_settings_read_token`. PR #174 retires it without replacement, in two steps that each rested on a probe against the private `personal-infra` repository. First, minted installation tokens differing only in Administration read answered identically — both listed all three rulesets over REST, both were refused `bypass_actors` and repository merge settings over REST, both received full merge settings and exact bypass-actor counts over GraphQL — so Administration read buys an installation token no visibility the implicit Metadata read already gives it, and it is not requested anywhere, including `DEFAULT_PERMISSIONS`. That is narrower than the Administration-plus-Metadata *user* PAT of the PR #94 probe, and the difference is real: user-scoped tokens do gain ruleset bypass-actor identities from Administration, installation tokens never do. Second, a token holding exactly the `check` aggregator job's `contents: read` and `issues: read` answered identically to the installation token on all four surfaces as well. A brokered token therefore sees nothing the workflow token does not, so minting one would decrypt the durable App key on every gate run to gain nothing — and the fallback that a broker path needs would silently degrade to the workflow token on a rotated age key, which reads as a green gate rather than a broken one. - -One capability is deliberately given up, and it is not recoverable by any CI credential. The retired PAT was *user-scoped*, so per `GITHUB-SETTINGS-002` it could read ruleset `bypass_actors` over REST; a third probe confirmed it, reading `bypass_actors: []` from a live ruleset where an installation token sees the field absent entirely. CI can therefore no longer confirm a declared *non-empty* bypass list, and a repo declaring one has a required check that fails until someone re-runs `github --check` locally with admin `gh` auth. That loss belongs to eliminating the PAT, not to choosing the workflow token over a broker token — an installation token never had the capability either — and the accepted tradeoff is a durable user-scoped credential that no API can rotate against a fail-closed gap on a declaration no canonical repo currently makes (`.github/settings.json` declares `[]`, which the GraphQL count verifies exactly). Everything else the gate compares is unchanged, and every fail-closed path stays: state the token cannot see is drift, not a pass. - -`ci.broker_app` remains required by the weekly sync and is now the only durable GitHub credential a consumer stores. Revisit this decision if GitHub changes the label permission boundary, exposes bypass actors to a token CI can hold, moves any compared setting behind a permission the workflow token cannot hold, or a consumer's declared bypass actors make the local-admin remedy routine rather than exceptional — at which point the broker path returns as a fail-closed requirement, not a fallback. - -## GITHUB-SETTINGS-002: Ruleset bypass-actor visibility boundary - -REST serves ruleset `bypass_actors` only to a user-scoped token holding repository Administration read, while GraphQL answers an exact `bypassActors.totalCount` to tokens it will not let name the actors. A probe on PR #176 against this repository's own `Protect main` ruleset, using a temporary bypass actor since removed, established both halves: admin `gh` auth over REST saw `[{repository_role 5, always}]`; a broker App installation token saw the field absent at its full permission ceiling and again at `administration: read` + `issues: read`; that same installation token over GraphQL saw `totalCount: 1` with `nodes: [null]`. A second reviewer corroborated the non-zero case independently against public repositories with a non-admin classic-scope viewer — `oven-sh/bun` `totalCount: 2` and `vercel/next.js` `totalCount: 4`, both with nulled nodes, against an exact `0` for `microsoft/vscode` — so the count is not visibility-filtered; only the identities are. The count is therefore trusted as exact, which is what makes zero a verifying answer rather than merely an absent one, and the checker cross-checks `totalCount` against the returned `nodes` length on every call so the assumption is checked rather than trusted. The boundary is narrow: a matching non-zero count still says nothing about which actors bypass, so that case fails closed instead of passing. Revisit this decision if GitHub filters `totalCount` by viewer permission, exposes the actor list to installation tokens over REST, or the cross-check begins to fire. - -## WORKFLOW-ACTIONS-001: Major-version action tags - -Production workflows use maintained major-version tags for external actions instead of full commit SHAs. The owner accepts the minimal risk that an upstream tag could be retargeted; reviews must not request immutable action pins unless that risk assessment changes. - -## WORKFLOW-CODEBUILD-001: Sole-maintainer workflow trust - -`CI_CODEBUILD_PROJECT` is available only under the personal-repository premise that the sole maintainer and their agents are trusted to edit workflow files before merge. Pull request application code remains untrusted. The selected CodeBuild project must therefore stay permissionless: no useful service-role permissions, secret-bearing environment variables, persistent local cache, or trusted network path. AWS project and account concurrency limits cap parallel use, while an external budget alarm makes unexpected spend visible. - -The canonical runner-contract tests are merge-time ratchets against accidental workflow routing changes. They are not runtime request authorization: a PR-controlled workflow can request a known CodeBuild runner label, and CodeBuild may start the runner before repository tests inspect that workflow. This tradeoff is accepted because only the trusted maintainer and their agents author workflow changes in the intended repositories, while ordinary pull request code receives only the unprivileged quality job. Do not add a controller or webhook authorization subsystem under this premise. Reopen the decision if an untrusted contributor can edit workflows, the CodeBuild project gains useful authority or persistence, external concurrency or budget controls disappear, or runner requests require authorization before AWS incurs cost. - -## WORKFLOW-CODEBUILD-002: Route every compatible canonical consumer job together - -This decision supersedes the quality-only routing limit in WORKFLOW-CODEBUILD-001 while preserving its sole-maintainer premise and project safeguards. When `CI_CODEBUILD_PROJECT` is set, the canonical quality gate, required verdict, weekly sync, and pause notification all use that project. The quality gate requests a medium instance because it runs the complete workspace gate and a Postgres service; easier jobs explicitly request a small instance so a project-default change cannot silently increase their cost. GitHub supplies the same job-scoped token permissions and referenced secrets that it would supply to a GitHub-hosted runner; moving those jobs does not grant the CodeBuild service role any additional authority. The Standards source repository's architecture-specific Nix matrix keeps its native runner mapping because consumers skip it and its runners encode the target architecture. - -The broader routing is accepted because the intended repository trusts its workflow authors and its CodeBuild environment, while one project and one default compute profile are simpler than a second secret-bearing runner pool. Reopen this decision if workflow authors are no longer trusted, the project stops being ephemeral and permissionless, a canonical job proves incompatible with the configured image, or separate compute profiles become justified by measured duration or cost. - -## WORKFLOW-CODEBUILD-003: Let measured quality gates select a larger instance - -The canonical quality job uses CodeBuild's `medium` instance by default and accepts a repository Actions variable, `CI_CODEBUILD_QUALITY_SIZE`, for an explicit larger size. The override affects only the complete quality gate; the required verdict, weekly sync, and pause notification remain pinned to `small`. This keeps the common case inexpensive without forcing a repository whose real gate exceeds 8 GiB into test serialization or a forked workflow. - -The first exception is ProsaBridge. Its uncapped gate lost the admin build to exit 137 while concurrent Next.js builds and browser suites ran. Limiting Turbo to two tasks moved the failure later but PostgreSQL and then the admin server still died during the browser gate after nearly fourteen minutes. Those two independent failures establish that the workload is incompatible with the medium runner rather than merely too concurrent. ProsaBridge may therefore select `large`, which provides 16 GiB and eight vCPUs, while repositories without the variable continue to use `medium`. Reopen this decision if measured gates fit medium again, the override is used without failure evidence, CodeBuild changes the label contract, or per-job sizing no longer produces a meaningful reliability or cost benefit. - -## WORKFLOW-GATES-001: Exact security-sensitive workflow step keys - -Security-sensitive workflow steps deliberately match their exact YAML mapping key sets. Every new field, including a benign field such as `timeout-minutes`, must therefore be added to the contract explicitly before the workflow passes. This conservative change-control boundary matches the existing Check-step strategy and does not weaken runtime behavior. The accepted cost is that safe metadata additions require a coordinated contract-test update instead of landing independently. This premise holds while every field on these steps deserves explicit review because workflow metadata can alter execution or failure semantics. Reopen this decision if GitHub requires an unavoidable field that cannot be represented in the contract, or if a different validation strategy preserves explicit review of every new field while materially reducing the maintenance cost. - -## STANDARDS-SYNC-001: Canonical sync changes execute automatically - -An automated standards sync branch may trigger consumer `push` and `pull_request` workflows before a maintainer reviews the generated pull request. This is an accepted trust boundary because the sync branch contains upstream-owned canonical standards from `davidvornholt/standards`, and those standards are trusted to execute in consumers. Do not add skip-check commits or a separate approval commit to suppress that automatic execution. Reopen this decision if canonical changes can enter the upstream source without trusted review, the sync branch can include consumer-controlled changes, consumer workflows expose credentials outside the accepted canonical trust boundary, or the threat model begins to require human review in each consumer before canonical code executes there. - -## IMAGE-PROMOTION-001: Superseded promotions never reactivate - -When promotion B provably supersedes open promotion A, A enters the terminal `superseded` phase. Later announcements of A attach evidence but never reopen its PR, advance its phase, merge it, or deploy it. If B fails, recovery continues by repairing or retrying B or by announcing a newer source build. Restoring a previously deployed A digest is possible only through a distinct approved rollback operation with its own identity and audit trail; it never reactivates A. - -This policy deliberately gives up an older ready candidate as an immediate availability fallback and can lengthen recovery while B or a newer build becomes deployable. The tradeoff is accepted because source-build retry and explicit rollback remain available, while terminal identities prevent stale branches from re-entering the deployment path and keep one unambiguous history for each operation. Reopen this decision if source builds or retries cannot reliably produce a candidate during an incident, the recovery-time objective cannot tolerate the delay in practice, the terminal rule is observed to prolong an outage, or a proposed fallback can preserve provenance, monotonic desired state, and an unambiguous audit trail without reactivating the superseded operation. - -## CREDS-CUSTODY-001: Machine-global plaintext broker custody - -The broker store is machine-global state outside every repository, so the repository rule requiring secret values in SOPS-encrypted YAML does not govern it. Plaintext `0600` custody shares the same local-account trust root as the plaintext personal age identity at `~/.config/sops/age/keys.txt`; encrypting the store to a recipient whose private key sits beside it was rejected as theater. Store writes must remain crash-atomic and concurrency-safe so interrupted or simultaneous logins cannot corrupt the file or lose one provider's credential. Reopen this decision if the threat model expands beyond the trusted local account, hardware-backed custody becomes part of the design, or the store moves into a repository, sync, or backup boundary. - -## CREDS-GITHUB-001: One private broker GitHub App per repository owner - -The machine-global broker stores one private GitHub App per owning user or organization. Repository ownership selects the App automatically, and a live App-authenticated installation lookup must prove access to that exact repository before its durable key enters SOPS. Installation on selected repositories remains the binding boundary, never an instruction to install on all repositories. This replaces the earlier single-App acceptance because private Apps cannot span accounts and sharing one public App would make unrelated account credentials share a compromise radius. Per-purpose Apps within one owner remain outside the current scope. Reopen this decision if repository ownership stops identifying the intended trust domain, one owner needs separate permission ceilings, or App installation lookup no longer proves selected-repository access. - -## CREDS-CLOUDFLARE-001: Two-source Cloudflare reconciliation - -Cloudflare reconciliation has exactly two sources of truth: the plaintext SOPS key structure in git and the provider token list under deterministic broker names. A third checked-in credential manifest is deliberately rejected. The absence of a desired-policy record means live policy drift is accepted as out of scope while the bootstrap token remains uncompromised; provider policy remains inspectable. A repository rename or transfer changes the deterministic namespace and is a documented human-visible re-mint-and-revoke event, not a reason to add persistent identity. This acceptance does not relax lifecycle invariants: login must functionally prove token-list authority, inactive tokens must not count as healthy, renewal must create a fresh-expiry replacement from the live policy, durably write and verify its value, then revoke the old token, and account-only minting must reject zone-scoped permission groups without a zone resource. Reopen this decision if policy-drift enforcement becomes required, bootstrap-token compromise enters the threat model, renames or transfers must reconcile automatically, or the two sources can no longer identify managed credentials unambiguously. - -## STANDARDS-CLI-001: Effect-free bootstrap package - -`packages/standards-cli` is deliberately plain TypeScript rather than Effect because its published bin must run through `bunx` before a consumer has installed any project dependencies. The package keeps a minimal runtime dependency surface (currently only `yaml`) and uses its established async/error idiom consistently; the credential broker follows the same package-level exception to the root Effect standards. Do not add Effect merely to align this bootstrap package with application architecture. Reopen this decision if the CLI no longer needs to bootstrap dependency-free consumers, its runtime is split into a separately installed package, or the minimal-dependency premise otherwise changes. - -## CREDS-CLOUDFLARE-002: Account-scoped token visibility - -Every broker surface that reads or deletes Cloudflare tokens addresses `/accounts/{account_id}/tokens`, so tokens created under a user profile rather than the account are invisible to `creds plan` and unreachable by `creds revoke`. This is structural, not an oversight: the bootstrap credential holds Account / Account API Tokens / Edit and nothing else, so reaching user-owned tokens would mean widening the one credential whose compromise costs the most. A review of PR #209 raised the concern that the pre-broker credentials this tooling exists to retire might themselves be user-owned, which would mean the orphan report misses the exact case motivating it. That was checked rather than reasoned about: running the branch CLI read-only against the live account listed `personal-infra-opentofu` and `personal-infra-opentofu-state` — both motivating credentials — as unmanaged, so they are account-scoped and fully reachable. The accepted limit is therefore that a user-owned token is retired in the Cloudflare dashboard, and the premise making that acceptable is that no credential this tooling replaces has turned out to be user-owned. Reopen this decision if a credential the broker is asked to adopt or retire is found under a user profile, if the bootstrap credential's permission ceiling widens for another reason, or if Cloudflare merges the two token namespaces. - -## CREDS-CLOUDFLARE-003: Namespace-squatting names block reconciliation - -A token whose name claims this repository's brokered namespace but is not a name the broker mints is a blocking finding: `creds apply` refuses to act at all until it is resolved, repository-wide. The blast radius is deliberate and was weighed — one hand-typed name in the Cloudflare dashboard stops every renewal in the repository, so a brokered token inside its renewal window can expire while reconciliation is halted. Failing closed is still correct, because the broker identifies what it owns by name alone and a name it cannot parse is state it cannot interpret; acting on ambiguous ownership is how credentials get destroyed. What makes the tradeoff acceptable is that the finding names its own remedy and the remedy works: the operator either renames the token in the dashboard or retires it with `standards creds revoke --account --token-id `, which deliberately accepts malformed in-namespace names for exactly this reason. That is the command the finding itself prints, `--account` included because it is required whenever more than one Cloudflare account is configured. Reopen this decision if the remedy stops being reachable from the CLI, if a legitimate workflow starts producing names in the broker namespace, or if the halt is observed to cause an expiry in practice. - -## DEV-ENV-001: Brokered reference objects are independently bound - -Each dev environment variable holds its own complete brokered S3 reference object, and resolution validates the exact target/key destination as a complete pair before selecting the requested part. Two environment variables are not implicitly bound merely because one selects `access_key_id` and the other selects `secret_access_key`; they may intentionally consume parts from different authorized pairs. Cross-variable pair binding is rejected because it would invent ordering or naming relationships outside the document schema. Reopen this decision if the schema grows an explicit pair-level declaration that maps both output variables together, or a consumer contract requires the two variables to share one credential identity. - -## DEV-DB-001: Canonical lifecycle recipe-name hard cutover - -The canonical root `justfile` owns `dev-db-start`, `dev-db-stop`, and `dev-db-status` as the generic local PostgreSQL lifecycle. A consumer whose repo-owned `local.just` declares any of those names must rename or remove the collision in the same sync change; a Just duplicate-recipe parse failure is the accepted hard-cutover signal. The canonical recipes do not probe for an old recipe, dispatch to repository-specific behavior, or preserve a compatibility alias. This is acceptable because supported consumers can migrate their local recipe names alongside the canonical sync, while repositories whose lifecycle does not fit the generic `packages/db/.env.local` contract can keep distinctly named local recipes. FESK is such a separate lifecycle and remains tracked by issue #242 rather than being migrated here. Reopen this decision if a consumer cannot atomically rename a colliding local recipe during sync, if the generic lifecycle stops being the intended canonical owner, or if a concrete coexistence requirement cannot be met with distinct repo-owned names. diff --git a/.agents/skills/ci-pipelines/SKILL.md b/.agents/skills/ci-pipelines/SKILL.md index 87fbc40c..e04e038e 100644 --- a/.agents/skills/ci-pipelines/SKILL.md +++ b/.agents/skills/ci-pipelines/SKILL.md @@ -5,6 +5,8 @@ description: Use when changing continuous integration, GitHub Actions workflows, # CI pipelines +Canonical workflows use maintained major-version tags for external actions. + ## Billing shapes the job graph - Jobs bill per minute, rounded up, minimum one. Fold sub-minute checks into an existing job on the same trust level instead of giving them their own. diff --git a/.agents/skills/review-fix/SKILL.md b/.agents/skills/review-fix/SKILL.md index 8ca3e79f..308d19c2 100644 --- a/.agents/skills/review-fix/SKILL.md +++ b/.agents/skills/review-fix/SKILL.md @@ -13,7 +13,7 @@ An explicit user choice wins. Otherwise use Claude Opus 5 at high effort in Clau ## Scope -Read `.agents/review/decisions.md` when present. Post one scope comment with the intent, threat model, out-of-scope work, and selected lenses; its timestamp starts the cycle. Ask about splitting only when the PR contains independent product outcomes. +Post one scope comment with the intent, threat model, out-of-scope work, and selected lenses; its timestamp starts the cycle. Ask about splitting only when the PR contains independent product outcomes. Choose distinct lenses: @@ -37,14 +37,14 @@ Reuse a successful equivalent exact-head gate. Otherwise run the repository gate For every required review or verification lens, spawn a separate read-only subagent using the [review skill](../review/SKILL.md). `review-pass` is an optional workflow helper. If delegation is unavailable, report incomplete coverage and stop. -Review the PR base → initial head with the scope, gate result, decisions registry, lenses, and any model override. Retry a skipped lens once, then stop if coverage is still incomplete. Merge duplicate findings while preserving every reporting lens, and assign one decision: +Review the PR base → initial head with the scope, gate result, lenses, and any model override. Retry a skipped lens once, then stop if coverage is still incomplete. Merge duplicate findings while preserving every reporting lens, and assign one decision: - `block`: demonstrated, in scope, material under the threat model, and worth stopping the merge; - `defer`: real but outside this PR or below the merge bar; - `discard`: refuted, speculative, already accepted, or not worth scheduling; - `ask`: a costly, durable product or architecture choice remains unresolved. -Do not ask about inferable implementation details, naming, local refactors, test shape, or other reversible choices. Choose the smallest sound option and record durable assumptions. Collect every unavoidable `ask` into one decision brief with the options, consequences, and a recommendation. +Do not ask about inferable implementation details, naming, local refactors, test shape, or other reversible choices. Choose the smallest sound option. Collect every unavoidable `ask` into one decision brief with the options, consequences, and a recommendation. ## Fix diff --git a/.agents/skills/review/SKILL.md b/.agents/skills/review/SKILL.md index 2d7db3f5..9ff4709d 100644 --- a/.agents/skills/review/SKILL.md +++ b/.agents/skills/review/SKILL.md @@ -13,7 +13,6 @@ Review the requested change without editing it. The goal is a trustworthy merge - Ground findings in inspected code, repository contracts, tests, command output, or documented framework behavior. - Show a reachable failure scenario. Suspicious patterns or theoretical possibilities alone are not findings. - Judge materiality against the supplied intent and threat model. Repository-rule drift is evidence, not automatically a blocker. -- Read `.agents/review/decisions.md` when present and do not reopen a still-valid decision without new evidence. - If an exact-head gate result was supplied, do not rerun the full gate. Use focused probes only. Instrumented probes belong in a disposable worktree; never modify the shared checkout. Enumerate the surfaces owned by the lens rather than sampling them. Read other files when they prove an in-lens finding, but do not duplicate another lens’s charter. @@ -24,7 +23,7 @@ Return exactly one decision per finding; do not add a separate severity: - **block** — demonstrated, in intent, material under the threat model, and serious enough to stop this merge. - **defer** — real and actionable, but outside the PR or below the merge bar. -- **discard** — refuted, speculative, already accepted, or too low-value to schedule. Report only durable discards worth recording. +- **discard** — refuted, speculative, already accepted, or too low-value to schedule. - **ask** — the repository cannot choose between materially different durable product or architecture outcomes, and choosing wrongly would be expensive to reverse. Do not ask about inferable implementation details, reversible choices, local refactors, naming, or test shape. Prefer the smallest in-scope correction and defer optional machinery. diff --git a/.agents/skills/standards-sync/references/github.md b/.agents/skills/standards-sync/references/github.md index 770df505..829433ac 100644 --- a/.agents/skills/standards-sync/references/github.md +++ b/.agents/skills/standards-sync/references/github.md @@ -20,3 +20,5 @@ bun standards creds add github --dest ci:ci.broker_app ``` The workflow mints two short-lived tokens for the current repository: a branch writer for contents and workflows, and a pull-request opener. Neither token enters the sync process, and there is no fallback credential. A repository with `autoSync: false` does not need these permissions until automatic sync is re-enabled. + +Canonical sync branches contain trusted upstream code and may run consumer CI before the generated PR is reviewed. diff --git a/.agents/skills/unslop/LICENSE.txt b/.agents/skills/unslop/LICENSE.txt deleted file mode 100644 index 6b540023..00000000 --- a/.agents/skills/unslop/LICENSE.txt +++ /dev/null @@ -1,21 +0,0 @@ -MIT License - -Copyright (c) 2026 Lauren Tan - -Permission is hereby granted, free of charge, to any person obtaining a copy -of this software and associated documentation files (the "Software"), to deal -in the Software without restriction, including without limitation the rights -to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -copies of the Software, and to permit persons to whom the Software is -furnished to do so, subject to the following conditions: - -The above copyright notice and this permission notice shall be included in all -copies or substantial portions of the Software. - -THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -SOFTWARE. diff --git a/.agents/skills/unslop/SKILL.md b/.agents/skills/unslop/SKILL.md deleted file mode 100644 index 2a93c06b..00000000 --- a/.agents/skills/unslop/SKILL.md +++ /dev/null @@ -1,80 +0,0 @@ ---- -name: unslop -description: Cut AI tells from any writing. Must always apply. ---- - -# Unslop - -Edit text to remove AI patterns and add human voice. - -## Process - -1. Scan for the patterns below. -2. Rewrite. Preserve meaning, match intended tone. -3. Add soul (see next section). -4. Self-audit: "What makes this obviously AI generated?" Fix remaining tells. - -## Adding soul - -Removing patterns is half the job. Sterile, voiceless writing is just as obvious. - -- **Have opinions.** React to facts instead of neutrally listing pros and cons. -- **Vary rhythm.** Short sentences. Then longer ones that take their time. Mix it up. -- **Acknowledge complexity.** "Impressive but also kind of unsettling" beats "impressive." -- **Use "I" when it fits.** First person isn't unprofessional. -- **Let some mess in.** Perfect structure looks machine-made. -- **Be specific.** Not "this is concerning" but "there's something unsettling about agents churning away at 3am." - -## Patterns to detect and fix - -### Content - -1. **Puffery.** "pivotal moment", "testament to", "evolving landscape", "setting the stage for", "indelible mark", "deeply rooted". Cut puffery, state what happened. -2. **Name-dropping.** Listing media outlets without context. Pick one, say what was said. -3. **Superficial -ing phrases.** "highlighting...", "ensuring...", "reflecting...", "showcasing...", "fostering...". Delete or expand with real sources. -4. **Promotional language.** "nestled", "vibrant", "breathtaking", "groundbreaking", "renowned", "stunning", "must-visit". Use neutral descriptions. -5. **Vague attributions.** "Experts believe", "Industry reports suggest", "Some critics argue". Name the source or delete. -6. **Formulaic challenges.** "Despite challenges... continues to thrive." Replace with specific facts. - -### Language - -7. **AI vocabulary.** Additionally, crucial, delve, enduring, enhance, fostering, garner, interplay, intricate, landscape (abstract), pivotal, showcase, tapestry (abstract), testament, underscore, vibrant. Replace with plain words. -8. **Fancy ways to say "is".** "serves as", "stands as", "boasts", "features". Just say "is" or "has". -9. **"Not just X, but Y."** State the point directly instead. -10. **Rule of three.** Forcing ideas into groups of three. Use the natural number. -11. **Synonym cycling.** Protagonist, main character, central figure, hero all in one paragraph. Pick one, repeat it. -12. **False ranges.** "from X to Y" where X and Y aren't on a meaningful scale. List topics directly. - -### Style - -13. **Em dash overuse.** Avoid em dashes entirely. Use periods or commas only (no parentheses, no en dashes, no hyphen-as-dash substitutes). Em dashes are an AI tell, and reaching for parentheses instead just trades one tell for another. If a thought needs separation, end the sentence or use a comma. -14. **Colon overuse.** Colons are fine before a list or example. Not as mid-sentence connectors. "If you're coming from traditional automation: instead of registering event handlers, you describe conditions" adds nothing with the colon. Rewrite to let the point stand on its own without comparison framing. "Describing when the scheduler should fire works best as plain English." Same meaning, no crutch punctuation. -15. **Boldface overuse.** Don't bold every proper noun or acronym. -16. **Inline-header lists.** The tell is a bold label and colon that restates the line: "**Performance:** Performance improved...". Convert those to prose. A bold lead-in that ends in a period, names the item, and is followed by genuinely new detail ("**Schema in TypeScript.** Tables live in one file.") is fine, not a tell. -17. **Title case headings.** Use sentence case. -18. **Decorative emojis.** Remove from headings and bullets. -19. **Curly quotes.** Replace with straight quotes. - -### Communication artifacts - -20. **Chatbot phrases.** "I hope this helps!", "Let me know if...", "Of course!", "Certainly!", "Found the smoking gun!" Remove. -21. **Cutoff disclaimers.** "While specific details are limited..." Find sources or remove. -22. **Sycophantic tone.** "Great question! You're absolutely right!" Respond directly. - -### Filler - -23. **Filler phrases.** "In order to" becomes "To". "Due to the fact that" becomes "Because". "It is important to note that" gets deleted. -24. **Excessive hedging.** "could potentially possibly be argued that it might" becomes "may". -25. **Generic conclusions.** "The future looks bright." State specific plans or facts. - -### Jargon - -26. **Abstract metaphor nouns.** Substrate, wedge, vector, locus, vantage, nexus, primitive (as noun), harness (as metaphor), surface (as in "API surface"), bedrock, scaffolding (as metaphor), modality, paradigm, gold-plating, ratchet (as metaphor), evacuate (for moving code), endgame, north star, flywheel. These read as technical but usually have a plainer concrete word. "Substrate" becomes "base". "Wedge in" becomes "add". "Vector" becomes "way" or "method". "Gold-plating" becomes "more than the job needs". "Ratchet" becomes the mechanism's real name or "a limit that only tightens". "Evacuate" becomes "move out". "Endgame" becomes "the last phase". Pick the concrete word. - -### Plain speech - -27. **Say what it does, not how it feels.** "the database stays close at hand", "SQL you can read", "types that follow your schema" name a feeling. The fix names the mechanism or a number: "`.toSQL()` returns the exact string sent to the database", "a column rename fails the build". Ask what the sentence tells the reader to do or know, then write that. If you can't restate it as a concrete instruction, fact, or number, cut it. One more check: if the sentence could appear unchanged in another project's docs, it says nothing about this one. Cut it. -28. **Shorten or split dense sentences.** If the reader has to backtrack to parse a sentence, break it in two or drop clauses. One idea per sentence. -29. **Active voice.** Prefer it. Catch "is/are/was/were + past participle" and name the actor: "queries are validated" becomes "the compiler validates queries", "the file is parsed by the loader" becomes "the loader parses the file". Passive is fine only when the actor is unknown or genuinely doesn't matter. -30. **Cut adverbs, or use a stronger verb.** "runs quickly" becomes "is fast" or the number. "significantly improves" becomes the measured delta. An adverb propping up a weak verb means the verb is wrong. -31. **Prefer the plain word.** "utilize" becomes "use", "leverage" becomes "use", "facilitate" becomes "help", "numerous" becomes "many", "in the event that" becomes "if". The fancier synonym is rarely clearer. diff --git a/.agents/skills/unslop/agents/openai.yaml b/.agents/skills/unslop/agents/openai.yaml deleted file mode 100644 index 8dc1480f..00000000 --- a/.agents/skills/unslop/agents/openai.yaml +++ /dev/null @@ -1,4 +0,0 @@ -interface: - display_name: "Unslop" - short_description: "Remove AI tells and add a natural human voice" - default_prompt: "Use $unslop to rewrite this text without AI tells while preserving its meaning and tone." diff --git a/AGENTS.md b/AGENTS.md index 1e2d3103..02bec9be 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -36,9 +36,14 @@ Do not weaken quality gates to make a change pass. Explain inline suppressions. ## Writing style +- Write plainly and directly. Avoid mannered prose, decorative metaphors, and stock phrases. Prefer literal wording and sentences that are easy to follow. - Use sentence case for reader-facing text — UI copy, labels, command-style actions, Markdown headings — preserving proper nouns, acronyms, filenames, package names, and domain terms. - Do not hard-wrap Markdown prose; keep each paragraph or list item on one logical line. +## Documentation + +Write documentation when it helps someone use, operate, or change the project. Keep it concise and current; do not narrate the implementation or repeat what the code makes clear. Put local rationale near the code and change history in PRs. + ## Project-specific rules This is a canonical file from the standards repository. Project-specific rules belong in `AGENTS.local.md`. diff --git a/README.md b/README.md index 778ca107..9d266c3c 100644 --- a/README.md +++ b/README.md @@ -72,7 +72,6 @@ Read [Sync and ownership](docs/sync-and-ownership.md) before changing a path who | [Operations](docs/operations.md) | CodeBuild, local PostgreSQL, the poller, Nix packaging, and releases. | | [CLI reference](packages/standards-cli/README.md) | Commands, configuration, and focused examples. | | [Agent contract](AGENTS.md) | Rules enforced in every consumer. | -| [Review decisions](.agents/review/decisions.md) | Durable architecture and trust decisions. | | [Infrastructure skill](.agents/skills/declarative-infra/SKILL.md) | NixOS hosts, OpenTofu, secrets, previews, and image promotion. | ## Packages diff --git a/packages/standards-cli/src/creds-cloudflare.ts b/packages/standards-cli/src/creds-cloudflare.ts index f8070a88..c62e83eb 100644 --- a/packages/standards-cli/src/creds-cloudflare.ts +++ b/packages/standards-cli/src/creds-cloudflare.ts @@ -3,6 +3,8 @@ // itself, lists and mints scoped tokens, and revokes them. Token values // returned by create flow only into SOPS writers, // never stdout. +// User-owned tokens are intentionally outside this API and the bootstrap +// credential's authority; they must be managed separately in Cloudflare. import { type CfResult, diff --git a/packages/standards-cli/src/creds-github-apps.ts b/packages/standards-cli/src/creds-github-apps.ts index c6510585..c2f34772 100644 --- a/packages/standards-cli/src/creds-github-apps.ts +++ b/packages/standards-cli/src/creds-github-apps.ts @@ -57,6 +57,8 @@ export const loadOwnedGithubStore = async ( return { ok: true, value: migrated }; }; +// Private Apps belong to one owner. Selecting by repository owner keeps each +// owner's credentials separate; installation access is verified before export. export const selectGithubAppForRepo = ( apps: ReadonlyArray, repo: string, diff --git a/packages/standards-cli/src/creds-plan.ts b/packages/standards-cli/src/creds-plan.ts index a68df6e6..30831d8e 100644 --- a/packages/standards-cli/src/creds-plan.ts +++ b/packages/standards-cli/src/creds-plan.ts @@ -7,6 +7,8 @@ // tokens it minted for another repository, are reported // (creds-plan-unmanaged.ts) but never mutated. Execution lives in // creds-plan-run.ts. +// Live policy is preserved, not compared with a separate desired-policy manifest. +// Adding policy-drift enforcement would require a new source of truth. import { cloudflareExpiresOn } from './creds-cloudflare-expiry'; import { groupByIntersectingFootprint } from './creds-plan-groups'; diff --git a/packages/standards-cli/src/creds-store.ts b/packages/standards-cli/src/creds-store.ts index f7ac0431..9bef0de3 100644 --- a/packages/standards-cli/src/creds-store.ts +++ b/packages/standards-cli/src/creds-store.ts @@ -24,6 +24,9 @@ export type BrokerStore = { readonly cloudflare: ReadonlyArray; }; export const EMPTY_BROKER_STORE: BrokerStore = { github: [], cloudflare: [] }; +// This machine-global store trusts the local account, like its age private key. +// Encrypting it with a colocated key would not protect against that account. +// Repository secrets still belong in SOPS; this store stays outside repositories. const OWNER_ONLY_FILE_MODE = 0o600; const OWNER_ONLY_DIR_MODE = 0o700; const FILE_MODE_MODULUS = 0o1000; diff --git a/packages/standards-cli/src/dev-env-brokered-resolve.ts b/packages/standards-cli/src/dev-env-brokered-resolve.ts index a9328d93..b9346805 100644 --- a/packages/standards-cli/src/dev-env-brokered-resolve.ts +++ b/packages/standards-cli/src/dev-env-brokered-resolve.ts @@ -22,6 +22,8 @@ export type ResolvedDevEnv = { readonly problems: ReadonlyArray; }; +// Each variable independently selects a complete pair and then one part. +// Variable names and neighboring references do not imply a shared credential. const lookupReference = ( document: unknown, reference: BrokeredS3Reference, diff --git a/packages/standards-cli/src/github-graphql.ts b/packages/standards-cli/src/github-graphql.ts index 0c86b69f..d52336a2 100644 --- a/packages/standards-cli/src/github-graphql.ts +++ b/packages/standards-cli/src/github-graphql.ts @@ -120,6 +120,8 @@ const BYPASS_ACTOR_PROBE = 1; // `totalCount` means the connection is being filtered rather than merely // redacted, so it supplies no count; a null *element* is the documented // identity-withholding shape, so only the length is checked. +// Probes with restricted tokens confirmed that hidden actors still count: +// https://github.com/davidvornholt/standards/pull/176 const countedRuleset = (node: unknown): readonly [number, number] | null => { if (!isRecord(node) || typeof node.databaseId !== 'number') { return null; diff --git a/packages/standards-cli/src/poller-codex.ts b/packages/standards-cli/src/poller-codex.ts index 850e6615..fcbb6b14 100644 --- a/packages/standards-cli/src/poller-codex.ts +++ b/packages/standards-cli/src/poller-codex.ts @@ -163,6 +163,9 @@ export const runCodex = async ( { detached: true, stdio: ['ignore', 'ignore', 'pipe'], + // Approved runs share the host service identity and its GitHub token. + // Approval and write revalidation constrain the workflow; they do not + // isolate credentials. This assumes trusted, maintainer-approved work. env: { ...process.env }, }, ); diff --git a/packages/standards-cli/src/standards-sync-broker-workflow-contract.ts b/packages/standards-cli/src/standards-sync-broker-workflow-contract.ts index 192154ff..dccfd41c 100644 --- a/packages/standards-cli/src/standards-sync-broker-workflow-contract.ts +++ b/packages/standards-cli/src/standards-sync-broker-workflow-contract.ts @@ -72,6 +72,8 @@ const writerConsumerName = 'Commit and push mirror changes'; const prConsumerName = 'Open a pull request if the mirror changed'; export const syncPolicyRefName = ['SYNC', 'POLICY', 'REF'].join('_'); +// Any new field can change execution or failure semantics. Even safe metadata +// additions require an explicit contract update so reviewers see the change. const assertExactStep = ( workflow: ParsedWorkflow, name: string, diff --git a/sync-standards.json b/sync-standards.json index 1659a294..86a6df3a 100644 --- a/sync-standards.json +++ b/sync-standards.json @@ -17,7 +17,6 @@ ".agents/skills/secrets-and-config", ".agents/skills/screenshots-in-prs", ".agents/skills/standards-sync", - ".agents/skills/unslop", ".agents/skills/ux-ui", ".claude/skills", ".claude/agents/reviewer.md", diff --git a/template/.agents/review/decisions.md b/template/.agents/review/decisions.md deleted file mode 100644 index fe70d9a8..00000000 --- a/template/.agents/review/decisions.md +++ /dev/null @@ -1,7 +0,0 @@ -# Review decisions registry - -Durable, already-litigated review decisions. How reviewers must treat entries and when orchestrators append them is defined in the `review` and `review-fix` skills. - -Entry format: heading `### D-NNN (date, status) — title`, where status is `decided` or `open`, followed by the decision and its rationale in prose. Entries are never edited silently; superseding an entry means a new entry that references the old id. - -## Entries diff --git a/turbo.json b/turbo.json index f55180b4..ee5177c7 100644 --- a/turbo.json +++ b/turbo.json @@ -34,7 +34,6 @@ "$TURBO_ROOT$/.agents/skills/secrets-and-config/**", "$TURBO_ROOT$/.agents/skills/screenshots-in-prs/**", "$TURBO_ROOT$/.agents/skills/standards-sync/**", - "$TURBO_ROOT$/.agents/skills/unslop/**", "$TURBO_ROOT$/.agents/skills/ux-ui/**", "$TURBO_ROOT$/.claude/skills/**", "$TURBO_ROOT$/.claude/agents/reviewer.md",