diff --git a/.abcd/config.json b/.abcd/config.json index 57c12bdced..4e5a972036 100644 --- a/.abcd/config.json +++ b/.abcd/config.json @@ -12,6 +12,7 @@ "backend": "host-delegated" }, "outbound": { + "declare_mentions": true, "forge": true, "hook": true }, diff --git a/.abcd/development/brief/04-surfaces/17-guard.md b/.abcd/development/brief/04-surfaces/17-guard.md index 2e61ae0b47..e41aa8d686 100644 --- a/.abcd/development/brief/04-surfaces/17-guard.md +++ b/.abcd/development/brief/04-surfaces/17-guard.md @@ -101,7 +101,13 @@ a checkout with more than one worktree), `interpreter-reads-stream` (a shell handed its script through a pipe, as in `cat x | sh`) or `forge-text-outbound` (a posting `gh pr` or `gh issue` command whose title or body carries a live agent-session URL or a tool attribution footer, in a repository that opted into -the forge-text check, iss-2610142210511831), is enforced but not +the forge-text check, iss-2610142210511831), and its two pull-request checks, +`forge-text-disclosure` (a `gh pr create`, `new` or `edit` whose body carries no +`Assisted-by:` line in an accepted form, judged as the attribution gate's body +arm judges it, iss-2609061525437969) and `forge-text-mention` (a pull request +whose title and body name an issue id no `Resolves:` or `Refs:` line declares, +RS004 before the push, in a repository that also sets +`outbound.declare_mentions`, iss-2610261152125261), are enforced but not taught; the scripts the guard reads, and the variable-built `bash -c` or `sh -c` script that deletes, which the guard refuses as `shell-c-variable-delete` (iss-2610161612200864), are taught by two rules the generator adds after the diff --git a/.abcd/development/brief/04-surfaces/38-land.md b/.abcd/development/brief/04-surfaces/38-land.md index 34336607a9..cecc41d4d1 100644 --- a/.abcd/development/brief/04-surfaces/38-land.md +++ b/.abcd/development/brief/04-surfaces/38-land.md @@ -140,7 +140,11 @@ bound: the refusal says so and gives the push to run by hand. says the pull request is now disarmed and gives the command that arms it. With no pull request the step stops with the `gh pr create` command; the verb never opens one (ruling R2), and the next run - queues it. + queues it. The command names a body land composed from the branch's commits (iss-220): each subject, then their + `Refs:`, `Resolves:` and `Delivers:` lines once each, then abcd's own `Assisted-by: abcd:` label and the + models the commits' `Assisted-by:` lines name (ruling R4), written to the checkout's local tier (land makes none) and + judged with the shell guard's checks of a posted body, the failing check named in the stop's note, so the body + the hosted checks read was checked before it was posted. ## The merge gate diff --git a/.abcd/development/brief/05-internals/03-configuration.md b/.abcd/development/brief/05-internals/03-configuration.md index 5d01c26136..ea70270c5d 100644 --- a/.abcd/development/brief/05-internals/03-configuration.md +++ b/.abcd/development/brief/05-internals/03-configuration.md @@ -43,13 +43,15 @@ file is gone. `attribution.hook` is written by `ahoy install --attribution`, and one outbound switch; the shell guard reads `outbound.forge`, and an install completes a repository that records only one of the two and leaves the other absent, while a key set to `false` is the repository's choice and is left as it -is. The remaining nine — +is. The remaining ten — whether abcd should enable the forge's own secret scanning, the rules loader's refresh backstop, the filing-time match's threshold and compared fields, whether `abcd land` pushes and queues, whether a reservation may be made on -an origin GitHub names a fork, and the GitHub settings the remote apply keeps -and the checks it requires — are hand-set: the binary reads them and never -writes them. +an origin GitHub names a fork, the GitHub settings the remote apply keeps +and the checks it requires, and whether the shell guard also holds a pull +request's title and body to RS004 (`outbound.declare_mentions`, a rule a +repository adopts, so no install sets it) — are hand-set: the binary reads them +and never writes them. ```json { @@ -96,8 +98,13 @@ writes them. "hook": true, // the committed commit-msg gate refusing a session URL or a tool // footer; runs only the abcd `git config --local abcd.binary` names, // which --outbound sets, clone-local, to the copy it verified - "forge": true // the shell guard refuses a posting gh pr/issue command whose title, - // body or body file carries either shape (forge-text-outbound) + "forge": true, // the shell guard refuses a posting gh pr/issue command whose title, + // body or body file carries either shape (forge-text-outbound), and + // a gh pr create/new/edit whose body has no Assisted-by line in an + // accepted form (forge-text-disclosure) + "declare_mentions": true // beside forge only: the shell guard refuses a gh pr create/new/edit + // whose title and body name an iss-N with no Resolves:/Refs: line + // declaring it (forge-text-mention, RS004 before the push) }, "rules": { "force_refresh_every_n": 15 // prompt-router refresh backstop (itd-3). The primary refresh is diff --git a/.abcd/development/intents/drafts/itd-2610161647248332-abcd-should-find-out-what-the-forge-can.md b/.abcd/development/intents/drafts/itd-2610161647248332-abcd-should-find-out-what-the-forge-can.md index 547f135ee3..82d13a7d7e 100644 --- a/.abcd/development/intents/drafts/itd-2610161647248332-abcd-should-find-out-what-the-forge-can.md +++ b/.abcd/development/intents/drafts/itd-2610161647248332-abcd-should-find-out-what-the-forge-can.md @@ -22,6 +22,8 @@ production_mode: hand-written Graduated from `iss-2610161632245527`: abcd should find out what the forge can do instead of asking the person or assuming. On 2026-10-16, setting up a throwaway lab to test abcd land, the agent asked the product thinker whether the team organisation allowed private copies; the product thinker did not know and should not have to. A read-only query answered it (members_can_fork_private_repositories is false for the organisation's own repositories), and a real attempt showed a private fork of a repository the person owns works anyway. The general want: abcd tests the forge's functionality itself (can repositories be private, can they be forked privately, is a merge queue available) and goes with the route that delivers the wanted outcome; only when no route exists does it ask, offering the options. It needs a state-of-the-art pass on forge capability detection before planning. Read that issue record for the source observation. +itd-2610261025516272 builds on this intent: it chooses a managed repository's landing route from what the forge's plan allows (a merge queue, required checks, or nothing at all), and its research note records GitHub's tiers as read on 2026-10-26, among them that a private repository has a merge queue only when its organization is on Enterprise Cloud. + ## Mechanism > _Prompted (the claim-recording gradient): why the authors expect this to work, as a falsifiable "we expect X because Y" — not the outcome restated. Replace this line with the claim, or with the exact token `None stated.` alone on its line to record the claim as considered and declined._ diff --git a/.abcd/development/intents/drafts/itd-2610261025516272-an-abcd-managed-private-repository-lands.md b/.abcd/development/intents/drafts/itd-2610261025516272-an-abcd-managed-private-repository-lands.md new file mode 100644 index 0000000000..376532fc07 --- /dev/null +++ b/.abcd/development/intents/drafts/itd-2610261025516272-an-abcd-managed-private-repository-lands.md @@ -0,0 +1,127 @@ +--- +id: itd-2610261025516272 +slug: an-abcd-managed-private-repository-lands +spec_id: null +kind: null +suggested_kind: null +reclassification_history: [] +builds_on: [itd-2610161647248332] +severity: minor +impact: additive +origin: researcher-authored +production_mode: hand-written +related_issues: [iss-2609012313465609, iss-2610110939559446, iss-172] +supersedes: [itd-115] +refines: [itd-106] +--- + +# Private repos land checked work without renting CI + +## Press Release + +> **An abcd-managed private repository lands each change after one check sized to what the change touches, run on the person's own machine, and buys hosted CI minutes only where the person chose to.** While agents work, a check reruns only the tests whose inputs changed and reuses every result whose inputs are the same, whichever worktree produced it. A feature is checked in full once, as it will land, not once per step, and a change to records alone reruns only the tests that read those records. To land a change, abcd takes the default branch as it stands, tests the merge on the same machine, again only as far as the default branch's new commits reach, and merges exactly the tree it tested, one change at a time, so no pull request waits behind another for a re-run. abcd finds out what the repository's plan allows (a merge queue, required checks, or nothing at all), says plainly what enforces the check and what does not, and shows what each hosted run would cost per landing before the person picks one. A contribution from anyone but the person and their own agents still runs in hosted CI. +> +> "My repositories are private, my laptop has ten cores, and I was paying a cloud runner to repeat what my agents had just run," said Alice, a solo founder. "Now the agents check their own work on my machine, a change to one package tests that package, and the only minutes I buy are for the one Linux run I asked for." + +## Why This Matters + +abcd's own repository is the one repository abcd manages with the whole regime (a preflight, a push receipt, a merge queue), and it shows where the time goes ([research note](../../research/notes/2026-10-26-local-first-landing-for-private-repos.md), "What abcd's own repository measures"): + +- One preflight runs for about 24 minutes, and the wait at the machine's load gate before it took 37 to 62 minutes, with about 20 waiters across seven sessions on 2026-10-16 (iss-2610161639025702). +- Every lane starts with a cold cache. A fresh worktree's first preflight reused 0 of about 110 packages' test results, and a rerun in the same worktree reused 47 to 88, because the go command keys a build on the package's directory unless `-trimpath` is set. +- Every reviewed step of a feature pays its own full run, and a change to records alone pays the same as a change to code. +- The hosted pull-request run repeats the local preflight (iss-2610110939559446), and the queue then runs again, 1.27 times per merged pull request, with the macOS leg dropping entries at its 45-minute cap (iss-2609012313465609). + +The record has owed this intent since 2026-09-29: iss-2609012313465609 was deferred on the ruling that an intent should choose among its directions, starting with which comes first. + +In a private repository the same habits cost money, and the route abcd planned is closed: + +- Hosted minutes beyond the plan's 2,000 (Free) or 3,000 (Pro, Team) a month cost $0.006 a minute on Linux and $0.062 on macOS, so a macOS minute costs about ten Linux minutes. +- GitHub's merge queue, the route itd-115 configures, exists for a private repository only when its organization is on Enterprise Cloud. A personal account or a Team organization has no queue. +- Rulesets, and with them required checks, need Pro or Team for a private repository. On GitHub Free nothing on the forge enforces a check at all. +- Nothing abcd installs gives a managed repository a preflight, a receipt or a CI check workflow: `ahoy` installs four commit hooks, and `launch scaffold` writes the release workflows. + +Two drafts already hold parts of this, and neither reaches that repository. itd-115 configures GitHub's merge queue by default, with a fallback that polls and updates a branch left behind. itd-2610110940216682 runs the full hosted suite once per change in the queue, and by its own scope condition a repository without a queue keeps the full suite on its pull requests. Neither makes a local check cheaper, and neither serves a private repository without a queue, which is most of the repositories abcd manages. + +Local CI with a commit status, the gh-signoff pattern that Rails 8.1 made a framework default, answers the cost for a single developer: the laptop runs the suite and tells the forge so. It leaves two things to answer: an agent's sign-off is the agent's word, and a branch tested alone is not its merge. + +## Mechanism + +We expect the local check per landing to cost in proportion to what a change touches, and the hosted minutes per landing to fall to none, or to the one Linux run the person chose, because three costs paid on every landing today can be avoided. Results whose inputs are byte-identical are rerun only because the cache key carries the worktree's directory. A feature's steps each pay a full run that one check of the landing tree covers. And the merge, the one thing a branch's own run cannot prove, is tested in hosted CI after a local run whose result a status could have carried. Shown wrong if, over a measured run of at least twenty landings in one private managed repository and twenty in abcd's own, the local check time per landing, the wait before it or the hosted minutes per landing is not lower than the 2026-10 baseline in the research note; or if a landed tree ever differs from the tree its check ran on; or if the release cut's run without reuse finds a defect that a reused result had passed; or if escapes, hosted failures a check run before the push would have caught, do not fall from the 13 of 32 failed runs of 2026-10-18 to 2026-10-26 (dcn-2610261152329451). + +## Scope Conditions + +- Holds for repositories whose pushers are the person and the agents working under the person's identity. A contribution from anyone else runs the full hosted check. +- Holds for GitHub at the plan tiers its documentation states on 2026-10-26: a merge queue for organization-owned public repositories and for private repositories on Enterprise Cloud only, and rulesets on private repositories from Pro and Team up. Another forge reaches the same routes through the forge seam. +- Holds for tests that are deterministic given their inputs. A flaky test breaks reuse and selection alike, and is a defect to fix, not a reason to rerun everything. +- Holds where the person's machine runs the repository's full check in tens of minutes, not hours, under the machine's load gate. +- Holds where the person's own operating system is one the project ships to. Another operating system is covered by a hosted run the person chooses, or by the nightly and release runs. + +## Acceptance Criteria + +- Given a commit whose check has passed in one worktree, when the same commit is checked in a fresh worktree on the same machine, then every test whose inputs are unchanged is reused rather than rerun, and the check's report counts the tests it ran and the tests it reused. +- Given a test that reads an input the result cache cannot see (a file a subprocess reads, or a file outside the repository), when the repository declares that test, then it reruns on every check; and given a release cut, when its gates run, then every test runs without reuse on the tree being tagged. +- Given a change to records alone, when its check runs, then only the tests whose inputs include those records rerun, and the report says which ran and why. +- Given a check that ran only a part of the gates, as an agent's quick check of one step does, when anyone lands the branch, then the landing is refused, naming the gates the check left out; only a check that covered every gate on the landing tree, each run or reused, lets it land. +- Given a feature built in several reviewed steps, when it lands, then it lands as one change with one landing check, and each step in between was checked with reuse rather than with a full run. +- Given a private repository with no merge queue and two branches ready to land, when abcd lands them, then it lands one at a time, each tested merged onto the default branch as it stands at that moment, and the tree that reaches the default branch is the tree that check ran on. +- Given the default branch moves between a branch's landing check and its merge, when abcd merges, then the stale merge is refused, and abcd tests the merge again, rerunning only what the new commits on the default branch reach. +- Given landings started from two computers at once, when both reach their merge, then they land one at a time, ordered by a hold on the shared remote, and neither lands an untested merge. +- Given a repository whose plan allows rulesets, when a landing check passes, then abcd records it on the tested commit as a status the ruleset requires; and given a plan that allows no rulesets, then abcd says plainly that nothing on the forge enforces the check and the local hooks are the whole fence. +- Given a repository that runs hosted CI and a pull request whose head carries a passed landing check, when the pull request is opened or updated, then the hosted pull-request run does no work beyond confirming that status. +- Given a pull request from anyone but the person and their agents, when it is opened, then the full hosted check runs on a hosted runner, a status its author posts is not accepted in place of it, and no self-hosted runner runs its code. +- Given abcd sets up a managed repository, when it offers the landing routes the plan allows, then each route states the hosted minutes and their price per landing at the published rates, with the date those rates were read, and states what the forge enforces under it. +- Given a private repository, when abcd sets up its hosted runs, then no macOS run is placed on every landing unless the person chooses it with its price shown; macOS runs nightly and at the release cut instead. +- Given a hosted run that fails, when abcd reads it, then it names the check that failed and whether that check could have run before the push (an escape), and given the failures since a stated date, then it counts escapes per check and names, for each check with any, the earliest local point that sees its input (the commit, the posted text, the landing, or the preflight before its test stages). +- Given the baseline in the research note, when this intent ships, then a measured run of at least twenty landings reports the local check time, the wait before it and the hosted minutes per landing against that baseline. + +## SOTA + +Surveyed 2026-10-26 against primary sources ([research note](../../research/notes/2026-10-26-local-first-landing-for-private-repos.md)): + +- **Local CI with a commit status (gh-signoff; Rails 8.1 `bin/ci`):** usable, and a framework default since Rails 8.1. Adopted as the pattern: the local result is posted as a status a ruleset can require. Not adopted as a dependency: posting a status is one forge call abcd makes itself. +- **The not-rocket-science rule and its queues (bors, Jane Street, GitHub's merge queue):** mature. Adopted: test the merge before the default branch moves, one landing at a time, with fewer and larger landings. GitHub's queue is used where the plan offers it; where it does not, abcd runs the queue on the person's machine. +- **Selection by dependency (Pants `--changed-dependents`, Nx affected):** mature inside their build systems, each a toolchain a repository would have to adopt. Not adopted now. The seam is the declared gate list's inputs, which a repository already on one of them can fill from it. +- **Reuse of results whose inputs are unchanged (Go's test cache; build-system caches; `GOCACHEPROG`):** mature. Adopted as the first lever: Go's own cache, shared across worktrees by `-trimpath`, with the tests whose inputs it cannot see declared. A cache shared between machines is a later backend behind `GOCACHEPROG`. +- **Learned test selection (Meta's predictive test selection):** mature at Meta's scale, and needs a change history one repository lacks. Not adopted. +- **Self-hosted runners:** mature, free today with a postponed fee, and unsafe for code from outside contributors. Offered only as a route for the person's own changes. + +Path taken: 2. A native floor (the declared gate list and its inputs, the receipt posted as a status, the local landing queue, reuse across worktrees) with seams for the field's backends: a forge merge queue where the plan offers one, and a dependency selector or a shared cache where a repository already runs one. No new dependency. The independent fit challenge runs at planning, with the collisions the research note lists. + +## Decomposition (itd-84 hand-run, 2026-10-26) + +Confirmed by the product thinker on 2026-10-26 (decision 1 below). Verdict: **SPLIT**. + +| Part | Type | Home | +|------|------|------| +| How a managed repository's changes are checked and landed: reuse, one check per landing, the local queue, the status, the route and its price | capability | this intent | +| A self-reported local result may replace duplicate hosted work, and may gate a landing, only for the person's and their agents' changes; it never gates an outside contribution, and a release cut never reuses a result | trust rule | an ADR at planning, then a brief invariant | +| Each check runs once, where it is cheapest to trust | stance | a principle at planning | +| The gate list's schema, the receipt's fields, `-trimpath`, the declared tests | plumbing | the spec and the brief | + +Typed links, as confirmed: + +- `supersedes` itd-115: its want, a ready pull request that never waits behind another, is met here on every plan, through GitHub's queue where the plan has one and through the local queue elsewhere, which replaces its poll-and-update fallback. +- itd-2610110940216682 stays the shape of hosted CI where a queue gates the default branch, as a sibling with no typed link. This intent's pull-request criterion (no work beyond confirming a passed landing check) tightens that draft's first criterion (the quick checks on one platform), flagged for the product thinker as a refinement of it, not a reversal. +- `refines` itd-106: that intent sets up which checks a repository has; this one decides where each runs and how a landing uses them. +- `builds_on` itd-2610161647248332: the route is chosen from what the forge can do, which that intent finds out. +- The issues it answers, joined from both ends: iss-2609012313465609 (the intent that issue's deferral says is owed), iss-2610110939559446 (the receipt as a status) and iss-172 (pull-request queue hygiene in managed repositories). iss-2610161639025702 (preflight throughput) is answered here too, but is already promoted to itd-2610170634394993, so it is cited rather than joined. + +No reversal flag. iss-172's invariant protects testing the merge before the default branch moves, and every route here still does that. The strict up-to-date setting stays on wherever the plan allows it; where the plan allows no rulesets, the local queue's own refusal of a stale merge does that job. + +## Open Questions + +- **The default route for a private repository with rulesets.** The local queue with its self-reported status, which spends no hosted minutes, or the local queue plus one hosted Linux run of the merge per landing, which is an independent check at about $0.006 a minute. Either way the person sees both prices at setup. +- **Who counts as the person's own.** The person's identity and the agents working under it only, or every collaborator with write access. +- **The self-hosted runner route.** Offer it for the person's own changes, since it gives a check that the pushing agent does not run itself and costs no minutes, or leave it out, since it shares the machine and the load gate with the local checks. +- **abcd's own repository.** It is public, so its minutes are free and its queue gives an independent check of every merge. Keep the queue and adopt only reuse and one check per landing, or move it to the local queue too. + +## Decisions + +1. **The routing is confirmed as proposed (2026-10-26).** The product thinker accepted the split and the typed links with a plain yes, given the agent's recommendation. This intent holds the capability alone. The trust rule (a self-reported local result may replace duplicate hosted work and gate a landing only for the person's and their agents' changes, never an outside contribution, and a release cut never reuses a result) becomes an ADR at planning, then a brief invariant. The stance that each check runs once, where it is cheapest to trust, becomes a principle at planning. itd-115 is superseded by this intent, the link to itd-106 is a refinement, and itd-2610161647248332 is a builds-on edge. The graded table is in the decomposition-calibration note under this date. + +2. **Escapes are recorded and the fixes are not bundled (2026-10-26).** Asked whether to record what wastes hosted runs, the product thinker accepted the recommendation: the recording, counting each hosted failure a check run before the push would have caught, is a criterion of this intent, and the fixes for the escapes found so far are issues worked first, outside it (dcn-2610261152329451): iss-2609061525437969, iss-220, iss-2610241424265464, iss-2610261151381604, iss-2610261151481013 and iss-2610261152125261. + +## Audit Notes + +_Empty. Populated by intent-auditor when intent moves to shipped/._ diff --git a/.abcd/development/intents/drafts/itd-115-abcd-managed-repos-merge-prs-without-the.md b/.abcd/development/intents/superseded/itd-115-abcd-managed-repos-merge-prs-without-the.md similarity index 90% rename from .abcd/development/intents/drafts/itd-115-abcd-managed-repos-merge-prs-without-the.md rename to .abcd/development/intents/superseded/itd-115-abcd-managed-repos-merge-prs-without-the.md index 2d2b30ec5d..f1a7a90a04 100644 --- a/.abcd/development/intents/drafts/itd-115-abcd-managed-repos-merge-prs-without-the.md +++ b/.abcd/development/intents/superseded/itd-115-abcd-managed-repos-merge-prs-without-the.md @@ -2,15 +2,20 @@ id: itd-115 slug: abcd-managed-repos-merge-prs-without-the spec_id: null -kind: null +kind: standalone suggested_kind: null -reclassification_history: [] +reclassification_history: + - { date: 2026-10-26, from: standalone, to: superseded, reason: "Its want, a ready pull request that never waits behind another, is met on every plan by itd-2610261025516272: GitHub's queue where the plan has one, the local landing queue elsewhere (product thinker, 2026-10-26)" } builds_on: [] severity: minor +kind_at_supersession: standalone +superseded_by: itd-2610261025516272 --- # A Ready PR Merges Without Ever Wedging BEHIND +> **Superseded by itd-2610261025516272** on 2026-10-26: Its want, a ready pull request that never waits behind another, is met on every plan by itd-2610261025516272: GitHub's queue where the plan has one, the local landing queue elsewhere (product thinker, 2026-10-26) + ## Press Release > **abcd-managed repos stop losing time to out-of-sync pull requests.** Onboarding @@ -123,5 +128,5 @@ _Empty. Populated by intent-fidelity-reviewer when intent moves to shipped/._ [iss-172]: ../../../work/issues/open/iss-172-abcd-managed-repos-need-pr-queue-hygiene.md [itd-1]: ../disciplines/itd-1-acceptance-gates.md -[itd-92]: itd-92-abcd-verifies-branch-protection-on.md +[itd-92]: ../drafts/itd-92-abcd-verifies-branch-protection-on.md [itd-114]: itd-114-abcd-mints-collision-proof-record-ids.md diff --git a/.abcd/development/research/notes/2026-08-15-decomposition-calibration.md b/.abcd/development/research/notes/2026-08-15-decomposition-calibration.md index 2d56ef2984..43a1fec7ab 100644 --- a/.abcd/development/research/notes/2026-08-15-decomposition-calibration.md +++ b/.abcd/development/research/notes/2026-08-15-decomposition-calibration.md @@ -2642,6 +2642,35 @@ Verdict at filing: SPLIT, confirmed by the product thinker. The initial routing Grade: the first routing was right at the level of intent versus issue, and wrong at the level of how many intents one setup rethink holds; both reviewers caught it independently. +## 2026-10-26: private repos land checked work without renting CI (product thinker, routing confirmed) + +| part | type | home | +|---|---|---| +| how a managed repository's changes are checked and landed: results reused across worktrees, one check per landing, the local landing queue, the result posted as a status, the route chosen from the plan with its price | capability | itd-2610261025516272 (draft) | +| a self-reported local result may replace duplicate hosted work and gate a landing only for the person's and their agents' changes, never an outside contribution, and a release cut never reuses a result | trust rule | proposed ADR and brief invariant at planning; not filed | +| each check runs once, where it is cheapest to trust | stance | proposed principle at planning; not filed | +| the gate list's schema, the receipt's fields, `-trimpath`, the declared tests whose inputs the cache cannot see | plumbing | the spec and the brief | + +Verdict: SPLIT, confirmed. The routing was the agent's, put to the product +thinker as a recommendation with its reasons, and accepted with a plain yes +on the same day, unchanged; the initial routing survived confirmation. + +Typed links, as confirmed: supersedes itd-115 (its no-wait landing want is met on +every plan, and the local queue replaces its poll-and-update fallback); +refines itd-106; builds on itd-2610161647248332. itd-2610110940216682 stays +the hosted-CI shape for a repository whose default branch a queue gates, a +sibling with no typed link. Advisory reversal flags: none; the strict +up-to-date setting stays on wherever the plan allows it, so iss-172's +invariant holds. One refinement is flagged rather than reversed: the draft's +pull-request criterion tightens itd-2610110940216682's first criterion. + +The routing moved once, at filing. The drafting missed itd-2610110940216682; +the create command's near-miss list surfaced it (score 0.297, below the +threshold). The draft was then scoped to the repositories without a queue, +plus the local half for every repository, and the sibling keeps the hosted +shape. A lexical near-miss at 0.3 was the only thing that found a draft with +the same title idea, which is worth a look by the automated rung. + ## 2026-10-26: abcd sizes its own test admission from what it measures (product thinker, routing confirmed) The proposal: "is there a way to autonomously adjust the number of preflights depending on load?", with the earlier ask that `load.preflights` be adjusted up or down as more evidence comes in. diff --git a/.abcd/development/research/notes/2026-10-26-local-first-landing-for-private-repos.md b/.abcd/development/research/notes/2026-10-26-local-first-landing-for-private-repos.md new file mode 100644 index 0000000000..e7bfde7955 --- /dev/null +++ b/.abcd/development/research/notes/2026-10-26-local-first-landing-for-private-repos.md @@ -0,0 +1,206 @@ +# Landing checked work in private repositories without renting CI twice: state of the art + +Dated 2026-10-26. A state-of-the-art pass behind the draft intent +[itd-2610261025516272](../../intents/drafts/itd-2610261025516272-an-abcd-managed-private-repository-lands.md), +local-first landing for abcd-managed repositories, asked for by the technical facilitator, +who also set the assumption the pass works under: most repositories abcd +manages are private. The question: how a managed repository should be set up +so that agents check their own work before it merges (quality), only the +checks a change needs are run (speed), and checks run on the person's own +machine, with a hosted runner used only where nothing local can stand in +(cost). Every source below was opened in the pass. Tiers: T1 is primary +documentation, source code or peer-reviewed work, T2 a credible engineering +write-up, T3 a blog or anecdote. + +## What abcd's own repository measures + +abcd's own repository is the one managed repository with the full regime (a +preflight, a push receipt, a merge queue), so it is the baseline. It is public, +so its hosted minutes are free, and its costs are wall-clock and queue time. + +- One `make preflight` runs for about 23 to 24 minutes, and the wait at the + load gate before it took 37 to 62 minutes; about 20 waiters were queued + across seven sessions on 2026-10-16 (iss-2610161639025702). +- Every lane starts with a cold Go cache. The first preflight in a fresh + worktree reused 0 cached test results out of about 110 packages; reruns in + the same worktree reused 47 to 88 (eight preflight logs in the main + checkout's local tier, 2026-10-24 to 2026-10-26). The cause is in the go + command: without `-trimpath`, a package's build action ID includes its + directory, so every worktree is a different cache key. T1, + `cmd/go/internal/work/exec.go` line 299 in go1.26.10 ("the object file may + refer to the absolute directory containing the package"). +- Every reviewed step branch gets its own full run, and a records-only change + pays the same Go test cost as a code change (iss-2610161639025702). +- The pull-request CI run repeats the local preflight (iss-2610110939559446). + Over 71 merged pull requests, 2026-10-05 to 2026-10-11, the queue ran 1.27 + `merge_group` runs per merged pull request, and the macOS leg dropped queue + entries at its 45-minute cap (iss-2609012313465609). +- Nothing reaches managed repositories. `ahoy` installs the commit-msg, + pre-commit, pre-merge-commit and prepare-commit-msg hooks + (`internal/core/ahoy/defaults/`), and `launch scaffold` writes the release + workflows and reads the repository's own CI check names + (`internal/core/launch/scaffold/cichecks.go`). Neither installs a preflight, + a receipt, a pre-push hook or a CI check workflow. + +## Findings + +### What a private repository pays, and what its plan allows + +- Hosted minutes included each month for private repositories: 2,000 on + GitHub Free, 3,000 on Pro and Team, 50,000 on Enterprise Cloud. Standard + runners cost $0.006 a minute on Linux (2-core), $0.010 on Windows and $0.062 + on macOS, so a macOS minute costs about ten Linux minutes. Public + repositories on standard runners and all self-hosted runners are free. T1, + [GitHub Actions billing](https://docs.github.com/en/billing/concepts/product-billing/github-actions), + read 2026-10-26. +- A $0.002-a-minute platform fee on self-hosted runners in private + repositories was announced on 2025-12-16 for 2026-03-01 and then postponed, + not withdrawn. The billing page read today states self-hosted use is free. + T2, [The Register, 2025-12-17](https://www.theregister.com/2025/12/17/github_charge_dev_own_hardware/); + T2, [Techzine](https://www.techzine.eu/news/devops/137396/github-bends-to-criticism-and-delays-paid-self-hosting-of-runners/). +- GitHub's merge queue is available in public repositories owned by an + organization, and in private repositories only when the owning organization + uses Enterprise Cloud. T1, + [Merging a pull request with a merge queue](https://docs.github.com/en/pull-requests/how-tos/merge-and-close-pull-requests/merging-a-pull-request-with-a-merge-queue). + So for most private managed repositories the queue route of itd-115 is + closed: a personal account or a Team organization has no queue to configure. +- Rulesets work in public repositories on GitHub Free, and in public and + private repositories on Pro, Team and Enterprise Cloud. T1, + [Managing rulesets](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-rulesets). + A private repository on GitHub Free can therefore enforce nothing on the + forge: the local hooks are the whole fence. +- A required status check can accept a status from any source, which anyone + with write access can set, or only from a named app. T1, + [Available rules for rulesets](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-rulesets/available-rules-for-rulesets). + With required checks on, commits are either pushed to another branch and + merged, or pushed directly once their checks have passed; strict checks + (the branch up to date with its base) mean more builds, and loose checks + mean fewer builds but checks that can fail after the merge. T1, + [About protected branches](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-protected-branches/about-protected-branches). +- Where a queue exists, it "provides the same benefits as the Require branches + to be up to date before merging" setting without the author updating the + branch, needs the `merge_group` trigger or the required checks never report, + and admits a pull request only once it has passed its required checks. Its + settings include build concurrency (1 to 100), minimum and maximum group + size, a wait time and "only merge non-failing pull requests". T1, + [Managing a merge queue](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/managing-a-merge-queue). + +### Local CI that the forge can see + +- gh-signoff records a local pass as a commit status (context `signoff`, or + `signoff/` for a partial sign-off) on a commit already pushed, and its + `install` writes a ruleset requiring it. It vouches by the developer's word: + nothing proves the run happened, and repository admins can bypass it. Its + stated rationale: "Dev laptops are super fast these days. They're + chronically underutilized. And you already own them." T1, + [basecamp/gh-signoff](https://github.com/basecamp/gh-signoff). +- Rails 8.1 ships local CI as a framework default: `config/ci.rb` declares the + steps, `bin/ci` runs them, and a last step signs off with gh-signoff. The + HEY suite took over ten minutes in cloud CI and runs locally in 1m 23s on a + Framework Desktop and 2m 22s on an M4 Max. T1, + [Rails 8.1 release notes](https://rubyonrails.org/2025/10/22/rails-8-1). +- Self-hosted runners are free but carry no guarantee of an ephemeral clean + machine, "can be persistently compromised by untrusted code in a workflow", + and on a private repository anyone who can fork it and open a pull request + (generally anyone with read access) can reach the runner. T1, + [Secure use reference](https://docs.github.com/en/actions/reference/security/secure-use); + [Self-hosted runners](https://docs.github.com/en/actions/concepts/runners/self-hosted-runners). + +### Testing the merge, not the branch + +- The "not rocket science rule": keep a branch that always passes its tests, + by testing each change integrated with the branch before the branch moves, + not after. Jane Street scaled it with speculation (test several requests in + parallel), batching (merge a group and retest individually on a failure), + faster incremental builds so small changes verify quickly, and fewer + requests: feature branches reviewed in full before integration cut their + weekly requests from hundreds to thirty or forty. Their conclusion: the + tricks "tweak the constants, but don't change the asymptotics". T2, + [Making "never break the build" scale](https://blog.janestreet.com/making-never-break-the-build-scale/). + Graydon Hoare's original post could not be fetched (HTTP 504); the rule is + cited through this write-up. +- GitHub's strict up-to-date setting is the same rule enforced per pull + request. Where no queue exists, re-testing every armed pull request after + each merge is the cost of keeping it (iss-2609012313465609, cycle one). + +### Running only what a change can affect + +- Pants selects targets from git: `--changed-since= + --changed-dependents=transitive` runs every target a change can reach, the + form it recommends for pull requests. A change to a lockfile can mark all of + its users changed, and transitive third-party dependencies are not + understood in this mode. T1, + [Pants advanced target selection](https://www.pantsbuild.org/stable/docs/using-pants/advanced-target-selection). +- Nx has moved from project-graph selection to task inputs: a task is affected + when one of its declared inputs changed, or when it reads the outputs of an + affected task; a lockfile change affects every task unless narrowed. The + earlier project-graph mode remains behind `affected.legacy`. T1, + [Nx affected](https://nx.dev/docs/features/ci-features/affected). +- Meta's predictive test selection still reports over 95% of individual test + failures and over 99.9% of faulty changes while halving the cost of testing + changes. It is a learned model trained on a large change history, not a fit + for a single repository's few hundred changes. T1, Machalica, Samylkin, + Porth and Chandra, [Predictive Test Selection](https://arxiv.org/abs/1810.05286), + camera-ready 2019. + +### Reusing results whose inputs did not change + +- Go caches successful package test results. A run matches when the test + binary is the same and only cacheable flags are passed (`-run`, `-short`, + `-timeout`, `-v` and a few others); tests that open files within the module + or read environment variables match only while those are unchanged, and + `-count=1` disables the cache. T1, `go help test` (go1.26.10). The cache + sees only files opened inside the module and environment variables read by + the test process itself: a file read by a subprocess the test runs (git, for + example) or outside the module is invisible to it, so such a test can reuse + a stale pass. +- `GOCACHEPROG` names an external program that implements the build cache, + the seam for a cache shared between machines or with CI. T1, + `go help environment` (go1.26.10). +- Content-addressed reuse and dependency selection answer the same question + from opposite ends: selection guesses what a change can reach from the + graph, while reuse reruns whatever's inputs differ, byte for byte. Reuse is + exact where the inputs are visible and wrong where they are not; selection + is approximate everywhere. Nx's move to declared task inputs, above, is the + field moving towards reuse. + +## Fit challenge (to run at planning) + +The independent fit challenge that [prefer-sota](../../principles/prefer-sota.md) +requires was not run in this pass: a peer session's preflight was in its test +stage and the machine's agent rule allowed no new sub-agent. These are the +collisions to hand it: + +- **Self-attested results against [evaluator-outside-the-loop](../../principles/evaluator-outside-the-loop.md).** + The gh-signoff stance trusts the person's word. abcd's agents push under the + person's identity, so a sign-off is the agent's word too. The challenge: + does a deterministic test run need an independent evaluator, or only an + honest one, and what makes an agent's run honest? +- **No new dependency.** gh-signoff, Pants, Nx, Bazel and a remote cache are + each a dependency or a toolchain. Posting a commit status is one forge API + call abcd already has the client for, and Go's own cache needs only a flag. +- **Host-agnostic and forge-agnostic.** The tier facts above are GitHub's. + A route chosen from what the forge offers keeps GitHub specifics behind the + forge seam (itd-2610161647248332 finds out what the forge can do). +- **The strict gate's invariant (iss-172).** Strict up-to-date was kept as the + duplicate-id gate. Ids are collision-proof since itd-114 shipped, and a local + queue that tests the merge result before the branch moves keeps the + invariant's purpose without the forge setting. The challenge: is that a + reversal of iss-172 or the same rule enforced elsewhere? + +## What this suggests for abcd + +- Reuse before selection: make results reusable across worktrees + (`-trimpath`) and let the cache decide what reruns, declaring the tests whose + inputs it cannot see, rather than building a dependency-graph selector first. +- One full check per landing, on the tree that lands. A feature lands as one + change; its steps are checked with reuse. +- For a private repository without a queue, test the merge locally, one + landing at a time, and merge exactly the tree tested: the rule's guarantee + at no hosted cost. +- Post the local result as a commit status so a ruleset can require it where + the plan allows rulesets, and say plainly where it does not. +- Spend hosted minutes only where a local run cannot stand in: outside + contributions, and whichever merge-result or other-OS run the person chose, + priced per landing before they choose. Never put macOS on every landing in a + private repository. diff --git a/.abcd/work/decisions/dcn-2610261152329451-the-product-thinker-adopted-the-remedies.md b/.abcd/work/decisions/dcn-2610261152329451-the-product-thinker-adopted-the-remedies.md new file mode 100644 index 0000000000..4420ffc62d --- /dev/null +++ b/.abcd/work/decisions/dcn-2610261152329451-the-product-thinker-adopted-the-remedies.md @@ -0,0 +1,6 @@ +--- +id: dcn-2610261152329451 +slug: the-product-thinker-adopted-the-remedies +date: 2026-10-26 +--- +2026-10-26 — The product thinker adopted the remedies of iss-2609061525437969 (narrowed to pull-request bodies), iss-220 (its compose half only) and iss-2610241424265464, and asked for them and the newly filed iss-2610261151381604, iss-2610261151481013 and iss-2610261152125261 to be built first. Grounds: of 32 failed hosted CI runs between 2026-10-18 and 2026-10-26, 13 were deterministic checks that could have run before the push (a pull-request body without its Assisted-by line 7, an undeclared issue id on the pull-request form 4, gitleaks 2), and 10 more were one govulncheck finding that stood on the default branch itself. Each check moves to the earliest local point that sees its input: the commit-msg hook, the shell guard before gh pr create, abcd land's composed body, or the preflight before its test stages. diff --git a/.abcd/work/issues/open/iss-172-abcd-managed-repos-need-pr-queue-hygiene.md b/.abcd/work/issues/open/iss-172-abcd-managed-repos-need-pr-queue-hygiene.md index 7d6b0237a0..511266e4b3 100644 --- a/.abcd/work/issues/open/iss-172-abcd-managed-repos-need-pr-queue-hygiene.md +++ b/.abcd/work/issues/open/iss-172-abcd-managed-repos-need-pr-queue-hygiene.md @@ -7,7 +7,7 @@ category: "observation" source: "user-observation" found_during: "explainability-plan merge stall" found_at: ".github/workflows" -related_intents: [itd-107] +related_intents: [itd-107, itd-2610261025516272] related_issues: [iss-178] remedy: "Waits on the itd-107 planning interview (ruling H): either way, render rung 1 into the merge-lifecycle steps itd-107 emits (after arming auto-merge, poll mergeStateStatus and run `gh pr update-branch` on BEHIND, strict never relaxed); if rung 2 is scaffolded, emit a merge queue with a `merge_group` trigger on every required workflow where the forge offers one, and otherwise an update-branch workflow on a GitHub App token, proven by a scaffold test that the emitted files carry that trigger or name that secret and keep strict true; if rung 1 only, close rung 2 as wontfix naming the credential cost." deferred_after: "v0.11.1" diff --git a/.abcd/work/issues/open/iss-2609012313465609-every-pull-request-pays-two-full-ci.md b/.abcd/work/issues/open/iss-2609012313465609-every-pull-request-pays-two-full-ci.md index 6062095316..6637ca153d 100644 --- a/.abcd/work/issues/open/iss-2609012313465609-every-pull-request-pays-two-full-ci.md +++ b/.abcd/work/issues/open/iss-2609012313465609-every-pull-request-pays-two-full-ci.md @@ -12,6 +12,7 @@ found_at: ".github/workflows/ci.yml" remedy: "Waits on the M19 planning interview (which direction first): if queue batching, raise min_entries_to_merge_wait_minutes above 0 in .abcd/work/rulesets/main-protection.json and apply it; if the full matrix in the queue only, run the macOS, race and smoke jobs on the merge_group event alone in .github/workflows/ci.yml; if caching, profile the macOS job first, since setup-go caches by default; if hooksPath, have ahoy install set core.hooksPath as an owned change. Prove the pick by the CI cycles per landed pull request, measured before and after." deferred_after: v0.11.1 deferral_reason: "The product thinker's ruling M19 of 2026-09-23: planned next cycle as its own intent that chooses among the record's directions, not folded into itd-115. Since filing, the pre-push hook checks a preflight receipt instead of running the preflight (2026-09-25) and the macOS leg's cap rose to 45 minutes (ruling Z, 2026-09-28); neither removes the second CI cycle. Owed: that intent, which opens on one question: which direction first, hooksPath at install, the full matrix in the queue only, caching, or queue batching?" +related_intents: [itd-2610261025516272] --- Measured on 2026-09-01 with thirteen auto-merge pull requests in flight: each one pays two full CI cycles before it lands, and the macOS check job alone takes about 13 minutes (ubuntu 9). Cycle one: the pull request is armed, main moves, the strict up-to-date policy makes it BEHIND, the keep-current script updates the branch, and the full CI re-runs on the updated head before the pull request is CLEAN enough to enter the merge queue. Cycle two: the merge queue runs the full CI again on the merge group. Every merge moves main and knocks the not-yet-queued pull requests back to cycle one, so a batch of thirteen cost roughly twenty-six 13-minute cycles serialised in ALLGREEN groups, and a one-line record change waited an hour. The maintainer asks how to speed the gates up, for example by running them locally first. Directions, none adopted: (1) local-first is already built and not wired on every account: make preflight is the pre-push gate and .githooks/pre-push runs it, but core.hooksPath is unset on at least one active account, so nothing runs before a push; wire it at ahoy install (an owned ConfigChange) and record which accounts have it; note that a local pass shortens nothing on the forge, it only stops red pushes. (2) Run the full matrix once, in the queue: on the pull_request event run the fast lane only (format, record gates, ubuntu build and test) and keep the macOS leg, the race lane and the smoke harness for the merge_group event, which already runs everything; the CI classifier that stands macOS down for docs-only changes shows the seam exists. (3) Cache the Go build and test cache across runs (actions/setup-go cache keyed on go.sum) and check whether the macOS job's 13 minutes is test time or cold-build time. (4) Let the queue batch: min_entries_to_merge_wait_minutes is 0, so each pull request tends to get its own group; a short wait lets several share one CI run. (5) The strict policy is the multiplier and was kept on 2026-09-01 (iss-2609012202237613); revisit only with the duplicate-id gate argument answered. diff --git a/.abcd/work/issues/open/iss-2610110939559446-every-push-already-carries-a-local-make.md b/.abcd/work/issues/open/iss-2610110939559446-every-push-already-carries-a-local-make.md index a920fda374..6660a1720e 100644 --- a/.abcd/work/issues/open/iss-2610110939559446-every-push-already-carries-a-local-make.md +++ b/.abcd/work/issues/open/iss-2610110939559446-every-push-already-carries-a-local-make.md @@ -10,6 +10,7 @@ origin: researcher-authored production_mode: hand-written found_at: "Makefile" remedy: "post the local preflight receipt as a commit status (the gh-signoff pattern) only to skip duplicate pull-request-level work, never in the merge queue, and only once a receipt can be attested rather than asserted" +related_intents: [itd-2610261025516272] --- Every push already carries a local make preflight receipt that ran the same gates CI runs, but CI cannot see it, so the pull-request run repeats the work. The receipt is the pusher's word, not an attestation, and with many agents pushing that is too weak to replace the queue run; it could only spare duplicate work before the queue. diff --git a/.abcd/work/issues/open/iss-2610261852072295-the-shell-guard-s-tokenizer-has-no.md b/.abcd/work/issues/open/iss-2610261852072295-the-shell-guard-s-tokenizer-has-no.md new file mode 100644 index 0000000000..fbfb6adc22 --- /dev/null +++ b/.abcd/work/issues/open/iss-2610261852072295-the-shell-guard-s-tokenizer-has-no.md @@ -0,0 +1,15 @@ +--- +schema_version: 1 +id: "iss-2610261852072295" +slug: "the-shell-guard-s-tokenizer-has-no" +severity: "minor" +category: "bug" +source: "agent-finding" +found_during: "fix/catch-ci-failures-before-push: script corpus warn after the commit-msg hook gained its RS004 run" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/core/guard/tokenize.go" +remedy: "Teach tokenize that [[ at command position opens one compound command that ends at its ]]: && and || inside it join tests and do not end a segment, and < and > inside it are comparisons, not redirections; a regression test holds this repro, and maxScriptCorpusWarns goes back to 0." +--- + +The shell guard's tokenizer has no notion of [[ … ]]: it cuts [[ $n -gt 1 && $a == *x* && $d == "$c" ]] at each && into separate segments, so $d == "$c" ]] reads as a command whose program the line does not fix. The guard then guesses $d may be a shell (a guessed sh -c payload) and re-reads its words as a shell string with the variables' values written in, so a quoted value such as c=" >8 " becomes a redirect to a file 8. A shell run of any script later in the same script then warns script-written-then-run (a write the guard cannot place). Found on .githooks/commit-msg's is_scissors test (cut_line holds ' ------------------------ >8 ------------------------') once the hook ran check-issue-resolution.sh through bash after it; minimal repro: a script of c=" >8 ", that [[ ]] line, then bash /abs/script.sh. Traced in a debug build: payload.go's kindShell ref with guessed:true, payload '== >8 ]]', from a segment whose tokens are [ == ' >8 ' ]]. diff --git a/.abcd/work/issues/open/iss-2610262219309883-the-shell-guard-reads-a-syntax-check-of.md b/.abcd/work/issues/open/iss-2610262219309883-the-shell-guard-reads-a-syntax-check-of.md new file mode 100644 index 0000000000..c99b4ebec5 --- /dev/null +++ b/.abcd/work/issues/open/iss-2610262219309883-the-shell-guard-reads-a-syntax-check-of.md @@ -0,0 +1,15 @@ +--- +schema_version: 1 +id: "iss-2610262219309883" +slug: "the-shell-guard-reads-a-syntax-check-of" +severity: "minor" +category: "bug" +source: "agent-finding" +found_during: "commit-msg hook review fixes, fix/catch-ci-failures-before-push" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/core/guard" +remedy: "Treat bash/sh -n (and --noexec, set -n) as reading the script, not running it: judge none of its lines, with a regression case in the script-run tests." +--- + +The shell guard reads a syntax check of a script, bash -n , as running it: abcd guard check --command 'bash -n .githooks/commit-msg' judges the hook's every line as a run would, and refused the check outright once one line of the hook read as a hazard (script-runs-hazard), though bash -n executes nothing. diff --git a/.abcd/work/issues/open/iss-2610262219399974-in-githooks-commit-msg-a-top-level-line.md b/.abcd/work/issues/open/iss-2610262219399974-in-githooks-commit-msg-a-top-level-line.md new file mode 100644 index 0000000000..bde1931917 --- /dev/null +++ b/.abcd/work/issues/open/iss-2610262219399974-in-githooks-commit-msg-a-top-level-line.md @@ -0,0 +1,15 @@ +--- +schema_version: 1 +id: "iss-2610262219399974" +slug: "in-githooks-commit-msg-a-top-level-line" +severity: "minor" +category: "bug" +source: "agent-finding" +found_during: "commit-msg hook review fixes, fix/catch-ci-failures-before-push" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/core/guard" +remedy: "Bisect the hook at this branch's head with the if restored to find the construct that changes how the later build line is read, fix the reading, and add the hook-shaped reproduction as a script-corpus regression case." +--- + +In .githooks/commit-msg, a top-level line 'if git rev-parse -q --verify MERGE_HEAD >/dev/null 2>&1; then' placed after the hook's RS004 setup made the guard's script reading block the hook's later '"$go_bin" build -buildvcs=false -o "$work/abcd" ./cmd/abcd' line as kill-by-holder (a program name from a command substitution reading as a refused command), where the hook without that line reads as a warn only. The same if without the redirection, or the check read into a variable first, reads as before; a six-line script holding only go_bin, that if and the build is allowed, and respelling is_scissors without && inside [[ ]] does not clear it, so the earlier context that triggers it is not isolated. diff --git a/.abcd/work/issues/open/iss-220-deterministic-ci-failures-lack-compose.md b/.abcd/work/issues/resolved/iss-220-deterministic-ci-failures-lack-compose.md similarity index 64% rename from .abcd/work/issues/open/iss-220-deterministic-ci-failures-lack-compose.md rename to .abcd/work/issues/resolved/iss-220-deterministic-ci-failures-lack-compose.md index bd8f49ca27..957df52a89 100644 --- a/.abcd/work/issues/open/iss-220-deterministic-ci-failures-lack-compose.md +++ b/.abcd/work/issues/resolved/iss-220-deterministic-ci-failures-lack-compose.md @@ -6,9 +6,11 @@ severity: "minor" category: "process" source: "user-observation" found_during: "manual-capture" -remedy: "Waits on the product thinker's ruling on compose-and-repair: if adopted, add a script-first compose step that writes a pull-request body from the branch's commits, appends the Assisted-by line and runs scripts/check-attribution.sh body on it before the pull request is created, and limit self-repair to an allowlist (trailer append, make fmt) applied by the author's own session, never by a bot push; if declined, move the record to wontfix naming the CI body arm as the standing detector. Prove the adopted form with a smoke case that composes a body and passes the body check." +remedy: "Adopted by the product thinker on 2026-10-26 (dcn-2610261152329451), the compose half only: when abcd land finds no pull request for the branch, it composes the body from the branch's own commits (their subjects, their Refs:, Resolves: and Delivers: lines, and their Assisted-by: lines), judges it with the checks the shell guard applies to a posted body, writes it to the local tier, and prints the gh pr create command with --body-file naming it. The self-repair allowlist is not adopted. Prove with a test that composes a body from commits carrying each trailer and passes the guard's body checks." deferred_after: "v0.11.1" deferral_reason: "ruling owed to the product thinker (lapsed-deferral triage, run A 2026-09-29): Should abcd compose and verify the pull-request surfaces it requires in managed repos, with an allowlisted class of deterministic check failures (trailer append, formatting) repaired automatically? This repository judges the body only after the fact (scripts/check-attribution.sh, body arm) and nothing composes it." +resolution: "with no pull request open, abcd land composes the body from the branch's commits (subjects, Refs:, Resolves:, Delivers: and Assisted-by: lines), judges it with the shell guard's body checks, writes it to the checkout's local tier and prints gh pr create with --title and --body-file naming it" +impact: additive --- simple deterministic CI failures have no compose-time prevention or self-repair path: the attribution gate detects a missing PR-body Assisted-by trailer post-hoc (PR 230), but nothing composes or verifies the body at creation time and no loop is authorised to apply the mechanical fix. PR-body composition is facilitator-owned manual work and therefore automation backlog: abcd should compose/verify the PR surfaces it requires for managed repos, and an allowlisted class of deterministic check failures (trailer append, formatting) should be self-repairing diff --git a/.abcd/work/issues/open/iss-2609061525437969-the-ai-assistance-disclosure-convention.md b/.abcd/work/issues/resolved/iss-2609061525437969-the-ai-assistance-disclosure-convention.md similarity index 73% rename from .abcd/work/issues/open/iss-2609061525437969-the-ai-assistance-disclosure-convention.md rename to .abcd/work/issues/resolved/iss-2609061525437969-the-ai-assistance-disclosure-convention.md index 704961c1ea..c9eeb29e4d 100644 --- a/.abcd/work/issues/open/iss-2609061525437969-the-ai-assistance-disclosure-convention.md +++ b/.abcd/work/issues/resolved/iss-2609061525437969-the-ai-assistance-disclosure-convention.md @@ -9,9 +9,11 @@ found_during: "release-gate adoption in a managed public repo, 2026-09-06" origin: researcher-authored production_mode: hand-written found_at: "rules (COMMITTING domain), hooks, internal (guard)" -remedy: "Waits on the product thinker's ruling: if adopted, extend the bundled COMMITTING Assisted-by line to pull-request bodies and agent-written issue text, and teach the shell guard to check gh pr create and gh issue create for the line (in the body or body file) beside the outbound session-link check; if declined, move the record to wontfix, naming the CI body arm as the enforcement in force. Prove the adopted form with guard tests that refuse a create command whose body lacks the line and pass one that carries it." +remedy: "Adopted by the product thinker on 2026-10-26 (dcn-2610261152329451), narrowed to pull-request bodies: the shell guard's forge-text check refuses a gh pr create or gh pr edit whose body or body file carries no Assisted-by line in one of the three accepted forms, judged with internal/core/assistedby's grammar after fenced blocks are stripped as the CI body arm strips them, and the bundled COMMITTING rule names pull-request bodies beside commits. Agent-written issue text is not part of the ruling. Prove with guard tests that refuse a create whose body lacks the line and pass one carrying each accepted form." deferred_after: "v0.11.1" deferral_reason: "ruling owed to the product thinker (lapsed-deferral triage, run A 2026-09-29): Should the bundled COMMITTING rule require the Assisted-by line on pull-request bodies and agent-written issue text in managed repos, with the shell guard checking the forge CLI's create commands for it? The session-link half is already in force (the bundled outward-text rule and the outbound policy), and this repository enforces the body trailer in CI only (scripts/check-attribution.sh, body arm)." +resolution: "the shell guard blocks a gh pr create, new or edit whose body or body file carries no Assisted-by line in an accepted form (forge-text-disclosure), judged by assistedby.JudgeBody, the Go copy of the attribution gate's body arm tied to it by a test that runs the gate over a shared corpus; the bundled COMMITTING rule names pull-request bodies beside commits" +impact: additive --- The AI-assistance disclosure convention covers commits only: the COMMITTING rule requires an Assisted-by trailer on AI-assisted commits, but says nothing about pull-request descriptions or issue text, and nothing checks either. In a managed repo two PRs went out with the trailer on every commit and none on the PR body, which the maintainer read as an oversight. Extend the convention to PR descriptions (and issue text written by an agent), teach the guard's GitHub-CLI coverage to require the trailer there, and make the same guard refuse session links and other tool footers — the two halves of one disclosure rule: say a tool assisted, never link the session. diff --git a/.abcd/work/issues/open/iss-2610241424265464-a-commit-message-that-names-an-issue-id.md b/.abcd/work/issues/resolved/iss-2610241424265464-a-commit-message-that-names-an-issue-id.md similarity index 77% rename from .abcd/work/issues/open/iss-2610241424265464-a-commit-message-that-names-an-issue-id.md rename to .abcd/work/issues/resolved/iss-2610241424265464-a-commit-message-that-names-an-issue-id.md index 73a02b071d..e7f1366b3c 100644 --- a/.abcd/work/issues/open/iss-2610241424265464-a-commit-message-that-names-an-issue-id.md +++ b/.abcd/work/issues/resolved/iss-2610241424265464-a-commit-message-that-names-an-issue-id.md @@ -10,6 +10,8 @@ origin: researcher-authored production_mode: hand-written found_at: ".githooks/commit-msg" remedy: "Run the message-only half of RS004 (every issue id the message names has a Refs: or Resolves: declaration) in the commit-msg hook, beside abcd lint outbound, so the refusal comes at the commit; keep RS004 in preflight and CI for the range." +resolution: "the committed commit-msg hook runs scripts/check-issue-resolution.sh's new message form over the message without its comment lines, so a bare issue mention is refused at the commit; preflight and CI keep RS004 over the range" +impact: internal --- A commit message that names an issue id without a Refs: or Resolves: line passes the commit-msg hook and is refused only by lint-issues RS004 inside make preflight. On 2026-10-24 that cost a whole preflight run (tens of minutes under the load gate) for a one-line amend, on a machine where preflight capacity was the scarce resource. diff --git a/.abcd/work/issues/resolved/iss-2610261151381604-make-preflight-does-not-run-the-two.md b/.abcd/work/issues/resolved/iss-2610261151381604-make-preflight-does-not-run-the-two.md new file mode 100644 index 0000000000..ffbbae882a --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2610261151381604-make-preflight-does-not-run-the-two.md @@ -0,0 +1,17 @@ +--- +schema_version: 1 +id: "iss-2610261151381604" +slug: "make-preflight-does-not-run-the-two" +severity: "minor" +category: "process" +source: "agent-finding" +found_during: "CI-escape analysis, 2026-10-26" +origin: researcher-authored +production_mode: hand-written +found_at: "Makefile" +remedy: "Add a vulncheck target running the same pinned command CI runs (go run golang.org/x/vuln/cmd/govulncheck@v1.7.0 ./...) and a secrets-check target running gitleaks over origin/main..HEAD with the repository's .gitleaks.toml, both preflight prerequisites before the load gate so a finding stops the run before its test stages; pin the govulncheck version to ci.yml's by a test; when gitleaks is absent or its version differs from CI's pin, say so loudly rather than pass silently; when the module cannot be scanned offline, say so and name what was not checked" +resolution: "make preflight runs secrets-check and vulncheck as prerequisites before the load gate (scripts/preflight-scans.sh): gitleaks over the branch's own commits and govulncheck over the module, each at the version ci.yml pins, read from ci.yml, each skipped loudly where it cannot run" +impact: internal +--- + +make preflight does not run the two hosted scans CI runs on every pull request, gitleaks over the commits the pull request would land and govulncheck over the module, so a finding either of them makes is found only after the push, in a hosted run. Between 2026-10-18 and 2026-10-26 they failed 12 of 32 failed CI runs: govulncheck 10 (one newly published standard-library vulnerability, on every pull request and queue entry from 2026-10-21 until the toolchain moved to go1.26.10 on 2026-10-22) and gitleaks 2. diff --git a/.abcd/work/issues/resolved/iss-2610261151481013-when-the-default-branch-itself-fails-a.md b/.abcd/work/issues/resolved/iss-2610261151481013-when-the-default-branch-itself-fails-a.md new file mode 100644 index 0000000000..8743a26862 --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2610261151481013-when-the-default-branch-itself-fails-a.md @@ -0,0 +1,18 @@ +--- +schema_version: 1 +id: "iss-2610261151481013" +slug: "when-the-default-branch-itself-fails-a" +severity: "minor" +category: "process" +source: "agent-finding" +found_during: "CI-escape analysis, 2026-10-26" +origin: researcher-authored +production_mode: hand-written +found_at: "Makefile" +remedy: "In the preflight vulncheck step (iss-2610261151381604), when govulncheck reports a finding and the branch changes neither go.mod nor go.sum against its merge base with origin/main, say that the finding most likely stands on origin/main too, name each fix govulncheck names (the toolchain or module version it is fixed in), and stop before the test stages; a branch that does change them is told the finding may be its own" +blocked_by: [iss-2610261151381604] +resolution: "on a govulncheck finding the vulncheck step names each fix govulncheck reports and, when the branch changes neither go.mod nor go.sum against origin's default branch, says the finding most likely stands there, then stops the preflight before its test stages" +impact: internal +--- + +When the default branch itself fails a hosted check, every branch pushed meanwhile pays a hosted run that is certain to fail, and nothing local says so before the push. From 2026-10-21 to 2026-10-22 a newly published Go standard-library vulnerability failed govulncheck on the nightly run and then on every pull request and queue entry, 10 runs, for about a day and a half, until the toolchain bump landed. A branch that changed neither go.mod nor go.sum could not have introduced it, and its author could have been told to land the bump first. diff --git a/.abcd/work/issues/resolved/iss-2610261152125261-rs004-on-the-pull-request-form-every.md b/.abcd/work/issues/resolved/iss-2610261152125261-rs004-on-the-pull-request-form-every.md new file mode 100644 index 0000000000..e3b53666e1 --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2610261152125261-rs004-on-the-pull-request-form-every.md @@ -0,0 +1,17 @@ +--- +schema_version: 1 +id: "iss-2610261152125261" +slug: "rs004-on-the-pull-request-form-every" +severity: "minor" +category: "process" +source: "agent-finding" +found_during: "CI-escape analysis, 2026-10-26" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/core/guard/forge.go" +remedy: "In the shell guard's forge-text check, refuse a gh pr create or gh pr edit whose title or body names an issue id that no Refs: or Resolves: line in the title or body declares, judged with the Go twins of MENTION_RE and DECLARE_RE in internal/core/capture/mentions.go and pinned to scripts/check-issue-resolution.sh by a parity test; it runs only where .abcd/config.json carries outbound.declare_mentions: true, because RS004 is this repository's rule, not a bundled one" +resolution: "the shell guard blocks a gh pr create, new or edit whose title and body name an issue id with no Resolves: or Refs: line declaring it (forge-text-mention), under outbound.declare_mentions beside outbound.forge; the vocabulary moved to core/issuemention, the one Go copy of RS004's, tied to scripts/check-issue-resolution.sh by a test that runs the gate over a shared corpus; abcd's own repository opts in" +impact: additive +--- + +RS004 on the pull-request form, every issue id the title and body name declared on a Refs: or Resolves: line, is checked only by the hosted attribution workflow, although the title and body exist before gh pr create runs and the shell guard already reads them for the outbound policy. Between 2026-10-18 and 2026-10-26 it failed 4 pull-request runs, each fixed by adding one line to the body. diff --git a/.githooks/commit-msg b/.githooks/commit-msg index 9623e96b0f..0b4500b808 100755 --- a/.githooks/commit-msg +++ b/.githooks/commit-msg @@ -30,6 +30,26 @@ # check gives a finding. Go is a prerequisite for developing this repository, so # the refusal asks for nothing new. # +# The hook also runs RS004's message half (iss-2610241424265464): every issue id +# the message names must have a `Resolves:` or `Refs:` line declaring it. The +# judgement is scripts/check-issue-resolution.sh's `message` form, the same gate +# preflight and CI run over the range, never a second copy here; a bare mention +# is refused at the commit instead of by a preflight tens of minutes later. It +# runs before the hook writes any file, and reads the message on its standard +# input, so the script a shell runs here follows no write the shell guard cannot +# place (script-written-then-run; the guard still warns, misreading is_scissors' +# test as one, iss-2610261852072295). It +# reads the message WITHOUT its comment lines, the one place the hook judges less +# than the whole: git's own status comments name the branch, a branch is often +# named after its issue (`capture/iss-N`), and git strips those comments from an +# edited message after this hook runs. A comment line a -F message keeps is left +# to the range check, which reads the recorded message, and so is a merge commit, +# whose message git writes. This half is abcd's own: RS004 is a rule this +# repository adopted, so the hook runs it only for a commit in this repository +# (a linked worktree included), never for another repository a global dispatcher +# runs it for, and the commit-msg hook `ahoy install --outbound` writes for a +# managed repository does not run it. +# # REACH, stated rather than implied. git runs this hook for `git commit` (an # amend and a reword through `git commit` included) and for a `git merge` that # creates a commit. `git commit --no-verify`, `git rebase`, `git cherry-pick`, @@ -59,7 +79,7 @@ case $- in *x*) set +x ;; esac # because the sweep below reads it. The list is expansion-free and runs before # `declare` is consulted, so what it names is dropped even when the sweep is # neutered — and the scissors cut below is steered by no builtin at all. -unset -f git grep awk mktemp tr cat mkdir rm chmod printf sed head command read echo exit test [ go cd pwd trap \ +unset -f git grep awk mktemp tr cat mkdir rm chmod printf sed head command read echo exit test [ go cd pwd trap env bash \ declare continue break return local export set true 2>/dev/null || true # A field separator the caller cannot choose, pinned before the sweep splits on it. IFS=$' \t\n' @@ -112,7 +132,7 @@ if [ -n "$guard_path" ]; then PATH="$PATH:$guard_path" fi export PATH -for tool in git mktemp rm; do +for tool in git mktemp rm env bash; do if ! command -v "$tool" >/dev/null 2>&1; then echo "commit-msg: BLOCKED — $tool is not on the hook's pinned PATH ($PATH)." >&2 echo " the hook pins PATH so a repo-scoped override cannot substitute a fake tool," >&2 @@ -264,9 +284,20 @@ if ((first >= 0)); then fi done fi +is_comment() { + if [[ $comment == auto ]]; then + [[ -n $1 && $auto_candidates == *"${1:0:1}"* ]] + else + [[ ${1:0:${#comment}} == "$comment" ]] + fi +} message="" +mentions_text="" for ((i = 0; i < keep; i++)); do message+="${lines[i]}"$'\n' + if ! is_comment "${lines[i]}"; then + mentions_text+="${lines[i]}"$'\n' + fi done # An empty message is git's to refuse (or to allow, under --allow-empty-message); @@ -276,6 +307,64 @@ if ! [[ $message =~ [^[:space:]] ]]; then exit 0 fi +# RS004's message half, for a commit in the hook's own repository only: the +# committing working tree must share its git common directory with $src, so a +# linked worktree of it counts and any other repository a global dispatcher +# runs this hook for does not. A merge commit is left to the range check, which +# skips merges: git's own merge message names the branch it merges. Both are +# read in git's own environment for this commit, before anything is unset. An +# amended merge has no MERGE_HEAD and is judged. +common_dir() { + local d + d="$(git rev-parse --git-common-dir 2>/dev/null)" || return 0 + case "$d" in /*) ;; *) d="$PWD/$d" ;; esac + (cd "$d" 2>/dev/null && pwd -P) || true +} +here_common="$(common_dir)" +src_common="$(cd "$src" && unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE GIT_OBJECT_DIRECTORY \ + GIT_ALTERNATE_OBJECT_DIRECTORIES GIT_COMMON_DIR GIT_PREFIX && common_dir)" || src_common="" +judge_mentions=1 +if [ -z "$here_common" ] || [ "$here_common" != "$src_common" ]; then + judge_mentions=0 +fi +merge_head="$(git rev-parse -q --verify MERGE_HEAD 2>/dev/null || true)" +if [ -n "$merge_head" ]; then + judge_mentions=0 +fi + +# The gate runs as a child bash, started through env with the shell-startup +# variables removed, so neither a BASH_ENV file nor an exported SHELLOPTS the +# committer carries runs inside it; every function was already dropped above, +# which takes its export with it. +if [ "$judge_mentions" -eq 1 ]; then + rs_gate="$src/scripts/check-issue-resolution.sh" + if [ ! -f "$rs_gate" ]; then + echo "commit-msg: BLOCKED — no issue-resolution gate at $rs_gate to judge the message's issue ids with." >&2 + echo " nothing in the message was found wrong; restore scripts/check-issue-resolution.sh and commit again." >&2 + exit 1 + fi + rc=0 + out="$(cd "$src" && unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE GIT_OBJECT_DIRECTORY \ + GIT_ALTERNATE_OBJECT_DIRECTORIES GIT_COMMON_DIR GIT_PREFIX && + env -u BASH_ENV -u ENV -u SHELLOPTS -u BASHOPTS -u CDPATH bash "$rs_gate" message - <<<"$mentions_text" 2>&1)" || rc=$? + case "$rc" in + 0) + say "commit-msg: " "$out" + ;; + 1) + echo "commit-msg: BLOCKED — the commit message names an issue id without declaring its relation to it (RS004)." >&2 + say " " "$out" + echo " add the declaration line named above and commit again." >&2 + exit 1 + ;; + *) + echo "commit-msg: BLOCKED — the issue-resolution gate could not judge the message (exit $rc)." >&2 + say " " "$out" + exit 1 + ;; + esac +fi + work="$(mktemp -d "${TMPDIR:-/tmp}/abcd-commit-msg.XXXXXX")" trap 'rm -rf "$work" || true' EXIT INT TERM HUP artefact="$work/message" diff --git a/.githooks/pre-push b/.githooks/pre-push index c6312499d3..1c56e1745f 100755 --- a/.githooks/pre-push +++ b/.githooks/pre-push @@ -30,14 +30,15 @@ # What the preflight runs: the load check first (load-check, a warning, # never a failure), then the `make fmt-check` format gate, the six lint # gates (lint-reviews, lint-issues, record-lint, issue-drift, docs-lint, -# check-attribution over the branch's commits), +# check-attribution over the branch's commits), the secrets-check and +# vulncheck scans (CI's gitleaks and govulncheck, at its pins), # the site-render gate and both tagged eval lanes # (smoke, evals-cold-reading), then build, vet, test and the race-enabled # internal tests, every Go step on the toolchain go.mod declares, as CI's # are. The eval lanes are named because the untagged `go test` step cannot # compile them, so a defect there would otherwise reach a push unopposed -# (iss-2608311632382737). The secret-scan, workflow-audit, -# dependency-review and govulncheck lanes run in Actions only, and the +# (iss-2608311632382737). The workflow-audit and dependency-review lanes +# run in Actions only, and the # record-lint job there repeats the reviews-charter and issue-resolution # gates preflight already ran here. # diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 32e6d4662d..b51e908baa 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -58,7 +58,9 @@ inbound = outbound statement is the whole of it. steps locally, together with the fmt-check format gate, the lint-reviews, lint-issues, record-lint, issue-drift, docs-lint, check-attribution (the identity and trailers of the branch's - commits) and site-render gates and both tagged eval lanes (smoke, + commits), secrets-check and vulncheck (CI's gitleaks and govulncheck scans, + skipped loudly where they cannot run) and site-render gates and both tagged + eval lanes (smoke, evals-cold-reading, about five seconds each — the untagged test step compiles neither). Every Go step it makes runs on the toolchain `go.mod` declares, which is the one CI runs, and the format gate runs that toolchain's gofmt @@ -86,7 +88,9 @@ inbound = outbound statement is the whole of it. provisions; with no banlist it warns loudly and lets the commit through), the commit-msg outbound check (it refuses a live agent-session URL or a tool's attribution footer in a commit message, through this checkout's own `abcd lint outbound`, built from `./cmd/abcd`, and refuses the commit when it cannot run - the check) and the pre-push receipt check: a push of a commit the remote does + the check; in this repository it also refuses a message that names an issue id with no `Resolves:` or `Refs:` + line declaring it, the message half of the RS004 gate, leaving merge commits to the range check) + and the pre-push receipt check: a push of a commit the remote does not hold yet needs a passing `make preflight` run on that commit with nothing uncommitted beside it, and the hook checks the receipt that run mints rather than running the preflight while the push holds its connection open. The diff --git a/AGENTS.md b/AGENTS.md index 28442c1e25..9cbb6d9589 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -146,15 +146,12 @@ Start with the plan and the design record: Run from the repo root. ```bash -make preflight # the pre-push gate: the load check first (load-check, - # a warning, never a failure), then fmt-check + - # lint-reviews + - # lint-issues + record-lint + - # issue-drift + docs-lint + check-attribution + - # site-render + - # smoke + evals-cold-reading, - # then build + vet + - # test + race (internal), all on the go.mod toolchain +make preflight # the pre-push gate: load-check (a warning, never a + # failure), then fmt-check + lint-reviews + lint-issues + + # record-lint + issue-drift + docs-lint + check-attribution + # + secrets-check + vulncheck + site-render + smoke + + # evals-cold-reading, then build + vet + test + race + # (internal), all on the go.mod toolchain make precheck # minutes, no queue: preflight's gates, build, vet, the # record, page and agent audit packages, and the command # and site audit tests chosen by name; no unit or race @@ -353,9 +350,9 @@ with `*CONCURRENCY` to force it). ## Definition of done -- `make preflight` is clean — the eight gates (`fmt-check`, `lint-reviews`, +- `make preflight` is clean — the ten gates (`fmt-check`, `lint-reviews`, `lint-issues`, `record-lint`, `issue-drift`, `docs-lint`, - `check-attribution`, `site-render`), both tagged eval + `check-attribution`, `secrets-check`, `vulncheck`, `site-render`), both tagged eval lanes (`smoke`, `evals-cold-reading`), plus `go build ./...`, `go vet ./...`, `go test -timeout 40m ./...`, and `go test -race -timeout 40m ./internal/...`, under `ABCD_TEST_SHARDS=3`, diff --git a/Makefile b/Makefile index 59bb520f2e..80f4d5bf8e 100644 --- a/Makefile +++ b/Makefile @@ -17,7 +17,7 @@ LDFLAGS := -s -w$(if $(VERSION), -X github.com/intentdriven/abcd/internal/core.V GO_TOOLCHAIN_VERSION := $(shell sed -n 's/^go \([0-9][0-9.]*\)$$/\1/p' go.mod) .PHONY: build test vet clean preflight precheck load-check lint-reviews lint-issues record-lint issue-drift docs-lint site-render smoke \ - evals-cold-reading check-attribution scaffold-sync scaffold-sync-check asking-sync asking-sync-check fmt fmt-check + evals-cold-reading check-attribution secrets-check vulncheck scaffold-sync scaffold-sync-check asking-sync asking-sync-check fmt fmt-check # Cross-compile every supported target to bin/abcd--. # Pass VERSION=vX.Y.Z to stamp the version (release builds); omit for a dev build. @@ -160,6 +160,22 @@ check-attribution: go build -o "$$tmp/abcd" ./cmd/abcd && \ ABCD_OUTBOUND_BIN="$$tmp/abcd" bash scripts/check-attribution.sh commits origin/main HEAD +# The two scans CI runs on every pull request (iss-2610261151381604), as +# preflight prerequisites so a finding stops the run before its test stages wait +# at the load gate: gitleaks over the branch's own commits (origin's default +# branch..HEAD) and govulncheck over the module, each at the version ci.yml pins, +# read from ci.yml. A scan that cannot run here (no gitleaks on PATH, or no +# network for govulncheck's database) is skipped loudly, never passed in +# silence, and CI's scan still runs. A govulncheck finding on a branch that +# changes neither go.mod nor go.sum most likely stands on the default branch +# (iss-2610261151481013), and govulncheck is not a required check, so it warns +# and does not stop the run; a gitleaks that runs and cannot scan stops it. +secrets-check: + @bash scripts/preflight-scans.sh secrets + +vulncheck: + @bash scripts/preflight-scans.sh vulncheck + # Deterministic drift gate for the .abcd/development design record (first slice # of internal/core/lint). Blocking: any record drift (stale tool names, dropped # concepts, lifecycle or reference breakage) fails preflight and CI. @@ -268,7 +284,8 @@ asking-sync-check: # warning, never a failure: load-check), then the format gate (fmt-check), the # six lint gates # (lint-reviews, lint-issues, record-lint, issue-drift, -# docs-lint, and check-attribution over the branch's commits), the +# docs-lint, and check-attribution over the branch's commits), the two scans +# CI runs on every pull request (secrets-check and vulncheck), the # site-render gate and both tagged eval lanes (smoke, evals-cold-reading) as # prerequisites, then build, vet, test, # and race-enabled internal tests natively — every Go step on the toolchain @@ -332,7 +349,7 @@ asking-sync-check: # test hung, and on 2026-10-25 claim, implement/loop and surface/cli each # outran 20 minutes on a 10-core Mac at a load of 5 to 7, with no test hung # (iss-2610250007404944). -preflight: load-check fmt-check lint-reviews lint-issues record-lint issue-drift docs-lint check-attribution site-render smoke evals-cold-reading +preflight: load-check fmt-check lint-reviews lint-issues record-lint issue-drift docs-lint check-attribution secrets-check vulncheck site-render smoke evals-cold-reading go build ./... go vet ./... go run ./cmd/abcd implement load --wait --timeout 2h @@ -357,7 +374,7 @@ preflight: load-check fmt-check lint-reviews lint-issues record-lint issue-drift # and TestPrecheckRunsTheAuditTests its test lines to what this says. PRECHECK_AUDIT_TESTS := Surface|Page|Brief|Parity|Reference|Snapshot|Appendix|Appendices|Paragraph|Vocabulary|Scope|Relay|Chapter|Baseline|Domain|Definitions|Committed|Docs|Identity -precheck: fmt-check lint-reviews lint-issues record-lint issue-drift docs-lint check-attribution site-render smoke evals-cold-reading +precheck: fmt-check lint-reviews lint-issues record-lint issue-drift docs-lint check-attribution secrets-check vulncheck site-render smoke evals-cold-reading go build ./... go vet ./... go test -timeout 20m ./internal/core/lint ./internal/core/launch/scaffold ./internal/core/mdrecord ./internal/core/issueschema ./internal/core/guard ./internal/core/glossary ./internal/core/reading diff --git a/commands/ahoy.md b/commands/ahoy.md index f2fbb51e4b..f1d4a9bbc1 100644 --- a/commands/ahoy.md +++ b/commands/ahoy.md @@ -1198,7 +1198,10 @@ the flag keeps both and restores a hand-deleted hook. `gh issue` command (create, new, edit, comment) hands the forge are judged against the same outbound policy before the command runs, and a live agent-session URL or a tool attribution footer in them is refused - (`forge-text-outbound`, in `commands/guard.md`). This half writes no hook and + (`forge-text-outbound`, in `commands/guard.md`). A pull request's body + (`gh pr create`, `new` or `edit`) is also held to the disclosure the + bundled `COMMITTING` rule asks of it: one `Assisted-by:` line in an accepted + form, and no `Co-authored-by:` line (`forge-text-disclosure`). This half writes no hook and needs no `abcd.binary`: the guard is the abcd already running in the session, so there is no binary to find and none to be missing. The guard reads `outbound.forge` alone; remove both keys, or set them to `false`, to turn diff --git a/commands/guard.md b/commands/guard.md index 69da64c027..870d79a8c0 100644 --- a/commands/guard.md +++ b/commands/guard.md @@ -998,6 +998,28 @@ read is not the one gh will post). So is a `.abcd/config/pii.json` that does not load, and a line handing the forge more than sixteen texts. A warn never stops a command, and it never passes one in silence. +A pull request's own text is held to two more checks before it is posted, +each of which the hosted checks would otherwise make only after the push. A +`gh pr create`, `gh pr new` or `gh pr edit` whose body or body file carries no +`Assisted-by:` line in one of the three accepted forms +(`Assisted-by: :`, `Assisted-by: None`, or the +`Assisted-by: abcd:` label abcd writes) is a **block** +(`forge-text-disclosure`). So is one carrying a `Co-authored-by:` line, a +tool's "Generated with" footer, or an `abcd:` label in a form abcd never writes. +The body is judged as the attribution gate's body arm judges it: fenced blocks +are stripped first, since the forge renders them as examples, carriage returns +are dropped, and the line must stand alone, so a trailer in the middle of a +sentence does not count. A title, a comment and an issue are not held to it. In +a repository that also sets `outbound.declare_mentions: true`, a pull request +whose title and body name an issue id (`iss-N`) without a `Resolves: iss-N` or +`Refs: iss-N` line declaring it is a **block** too (`forge-text-mention`): the +rule CI's issue-resolution gate applies to the pull request (RS004), read from +both halves together, since a title has no room for a trailer. Neither check +judges a body the line does not fix or the guard could not read, and the mention +check judges nothing when any of the pull request's text is hidden from it (a +variable title, a body on standard input): the hidden part may hold the +declaration. + It does not see: a body or a path the line does not fix (`--body "$BODY"`, `--body-file "$F"`, a substitution other than the `cat` document above), a body on standard input (`--body-file -`), a flag spelled by a substitution @@ -1010,7 +1032,9 @@ script the line runs, and every other posting path: `gh api`, `gh pr review`, after posting stays the control for those. Nothing changes in a repository that did not opt in. The opt-in is read from -the working tree on each check. `--outbound` is one switch for this check and +the working tree on each check, and `outbound.declare_mentions` counts only +beside `outbound.forge`: RS004 is a rule a repository adopts, so `ahoy install` +never sets it. `--outbound` is one switch for this check and the commit-message hook together, so to turn it off remove both `outbound.forge` and `outbound.hook` from `.abcd/config.json` (or set both to `false`): a later `ahoy install` leaves both off as they are, but completes a diff --git a/commands/land.md b/commands/land.md index f4446c7ad5..1f8502306d 100644 --- a/commands/land.md +++ b/commands/land.md @@ -139,7 +139,20 @@ that name with none at this head, since the proof may lie past the bound. is now disarmed and gives the command that arms it again. With no pull request the step is `stopped` with the `gh pr create` command: relay it to the user, who opens - the pull request; the next `abcd land` queues it. + the pull request; the next `abcd land` queues it. The command carries + `--title` (the branch's first commit's subject) and `--body-file`, naming a + body land composed from the branch's commits into the checkout's + `.abcd/.work.local/scratch/land/`: each commit's subject as a list, then + their `Refs:`, `Resolves:` and `Delivers:` lines, once each, then abcd's + own `Assisted-by: abcd:` label, since abcd composed the text, and + the models the commits' `Assisted-by:` lines name (a commit's `None` is not + carried), and no commit's prose. land judges that body as the shell guard judges a + posted one (the outbound policy, the disclosure line, and the issue-id + declarations where the repository opted into them), and a body that fails + one is still written, with the stop's note naming the check, so the user + fixes it before posting. Where git does not ignore the local tier, or the + checkout has none (land makes none), land writes no file and the command + is the bare `gh pr create`. The payload carries `branch`, `head`, `checkout` (home-relative), `default_branch`, `push` and `enqueue` (each `key`, `on`, and the `layer` and diff --git a/docs/how-to/install.md b/docs/how-to/install.md index 12c88e803a..29296d014e 100644 --- a/docs/how-to/install.md +++ b/docs/how-to/install.md @@ -818,7 +818,8 @@ and reports it the same way. make preflight # the pre-push gate: the load check first (load-check, a # warning, never a failure), then fmt-check, lint-reviews, # lint-issues, record-lint, issue-drift, - # docs-lint, check-attribution, site-render, smoke and + # docs-lint, check-attribution, secrets-check, vulncheck, + # site-render, smoke and # evals-cold-reading, then # build, vet, test and race, all on the toolchain go.mod declares go run ./cmd/abcd # bare status board for the current directory diff --git a/docs/reference/cli/commands.md b/docs/reference/cli/commands.md index 0e6a12fb99..df07ea8018 100644 --- a/docs/reference/cli/commands.md +++ b/docs/reference/cli/commands.md @@ -129,7 +129,7 @@ Apply the install gaps the detection finds: Writes .abcd/, the name-guard hooks --live-sessions string answer the live brief session question: where sessions may run, local, cloud or both, or none, which answers no and is not asked again; stored in .abcd/config.json as sessions.live and honoured in any run, the approve-everything flag included; abcd session setup then does the setting up --only-waiting ask only the setup that waits after an abcd update: the questions this repository never answered and the settings whose default changed, never one answered or deferred with decide later; every kind of change is approved up front, as the yes to the offer abcd update news makes approves them; with the approve-everything flag no question is answered, so each still waits --oracle-backend string oracle backend: host-delegated | native | cli | api | mcp - --outbound opt this repo into both outbound checks, which refuse a live agent-session URL or a tool attribution footer: the committed commit-msg hook over every commit message, and the shell guard's check of the title, body and body file a posting gh pr or gh issue command hands the forge; it also sets this clone's git config --local abcd.binary to the abcd copy whose build provenance it verified and, where no core.hooksPath is set and no commit-msg hook is there, writes abcd's copy of that hook into the clone's git hooks directory, and says so when it cannot do either; the choice is recorded in .abcd/config.json as outbound.hook and outbound.forge, so a later install without the flag keeps both + --outbound opt this repo into both outbound checks, which refuse a live agent-session URL or a tool attribution footer: the committed commit-msg hook over every commit message, and the shell guard's check of the title, body and body file a posting gh pr or gh issue command hands the forge, which also refuses a pull request body (gh pr create, new or edit) with no Assisted-by line in an accepted form, or one carrying a Co-authored-by line; it also sets this clone's git config --local abcd.binary to the abcd copy whose build provenance it verified and, where no core.hooksPath is set and no commit-msg hook is there, writes abcd's copy of that hook into the clone's git hooks directory, and says so when it cannot do either; the choice is recorded in .abcd/config.json as outbound.hook and outbound.forge, so a later install without the flag keeps both --readme-parts string answer the README question: the parts of README.md abcd keeps current, comma-separated from licence, links, contents, release-badge, build-badge and abcd-badge, or none, which answers no and is not asked again; stored in .abcd/config.json and honoured in any run, the approve-everything flag included --refuse-adopt decline to adopt an unmanaged repo --scan-deep string enable deep scan: true | false @@ -3773,8 +3773,12 @@ credentials, so run it only on a branch whose content you trust. queue where the merge gate allows it (today: only where a merge queue gates the default branch and a ruleset requires a person's approval), pinned to the head, then confirmed in the queue. An arming someone else made is left as it - is. With no pull request it prints the `gh pr create` command and stops; it - never opens one. + is. With no pull request it composes the body from the branch's commits (each + subject, their Refs:, Resolves: and Delivers: lines, abcd's own Assisted-by label + and the models their Assisted-by: lines name) into the + checkout's .abcd/.work.local/ where it has one, judges it as the shell guard + judges a posted body, prints the `gh pr create` command naming it with + --body-file, and stops; it never opens one. land.push and land.enqueue are read from .abcd/config.json (in the working directory's checkout and the branch's own) and ~/.abcd.noindex/config.json, diff --git a/internal/README.md b/internal/README.md index 15a39168b7..0e8b60e435 100644 --- a/internal/README.md +++ b/internal/README.md @@ -183,7 +183,16 @@ plugin surface, and a future MCP server share one engine. and `core/site`'s authorship tally counts it apart from assistance and `None`. Both also read the model form through it: the loop to judge a receipt's model, the site to chart one. The attribution gate decides the grammar in - shell; a test ties the two. + shell; a test ties the two. It also holds the gate's pull-request body check + (`JudgeBody`, the body arm with its fence stripping), which the shell guard + judges a body with before gh posts it and `core/landing` judges the body it + composes with; a test runs the gate itself over the same corpus. +- **`core/issuemention/`** — RS004's vocabulary: the issue ids a text names and + the `Resolves:`/`Refs:` lines that declare its relation to them. It is a leaf + because the shell guard (`core/guard`) judges a pull request's title and body + with it, and `core/capture`, which reads history with it, imports the guard. + scripts/check-issue-resolution.sh decides the rule; a test runs the gate over + the same corpus. - **`core/cite/`** — the live half of the citation gate: the bounded fetcher and the refresh that writes the committed baseline `core/lint` then enforces with zero network. It is the only place abcd dials out on behalf of documentation, diff --git a/internal/core/assistedby/body.go b/internal/core/assistedby/body.go new file mode 100644 index 0000000000..a3a26b8e6e --- /dev/null +++ b/internal/core/assistedby/body.go @@ -0,0 +1,162 @@ +package assistedby + +import ( + "regexp" + "strings" +) + +// The pull-request body check (iss-2609061525437969): the attribution gate's +// body arm, check_text over strip_fenced_blocks in scripts/check-attribution.sh, +// in Go. The gate runs in CI without Go and decides it; this is its one Go copy, +// which the shell guard judges a body with before gh posts it and `abcd land` +// judges the body it composes with, tied to the gate by TestJudgeBodyMatchesTheGate. +// The gate's session-URL half is not here: it is scanner.CheckOutbound, which +// both callers already run. + +// BodyFault is what the gate's body arm refuses a pull-request body for, the +// first it finds in the gate's own order. +type BodyFault int + +const ( + // BodyClean: the gate accepts the body. + BodyClean BodyFault = iota + // BodyGeneratedFooter: a tool's "Generated with [...]" footer line. + BodyGeneratedFooter + // BodyCoAuthor: a Co-authored-by line, which asserts an authorship no tool + // holds. + BodyCoAuthor + // BodyMalformedLabel: an Assisted-by line naming abcd in a form abcd never + // writes. + BodyMalformedLabel + // BodyNoTrailer: no Assisted-by line in one of the three accepted forms. + BodyNoTrailer +) + +func (f BodyFault) String() string { + switch f { + case BodyClean: + return "clean" + case BodyGeneratedFooter: + return "a tool's 'Generated with' footer" + case BodyCoAuthor: + return "a Co-authored-by line" + case BodyMalformedLabel: + return "an Assisted-by line naming abcd in a form abcd never writes" + case BodyNoTrailer: + return "no Assisted-by line in an accepted form" + } + return "an unknown fault" +} + +// The gate's GENERATED_RE and COAUTHOR_RE, read out of the script by +// TestBodyRefusalPatternsAreTheGates. +const ( + generatedFooterPattern = `^[[:space:]]*(🤖[[:space:]]*)?([_*]{1,3})?(🤖[[:space:]]*)?[Gg]enerated (with|by) \[` + coAuthorPattern = `^[[:space:]]*[Cc]o-[Aa]uthored-[Bb]y:` +) + +var ( + generatedFooterRe = regexp.MustCompile(generatedFooterPattern) + coAuthorRe = regexp.MustCompile(coAuthorPattern) +) + +// trailerKey is the trailer's key with the one space the gate's patterns +// require before the value. +const trailerKey = "Assisted-by: " + +// JudgeBody judges a pull-request body as the gate's body arm does: fenced +// blocks stripped first, since a forge renders them as examples, then CR +// removed, then each line read on its own. +func JudgeBody(body string) BodyFault { + lines := strings.Split(strings.ReplaceAll(stripFencedBlocks(body), "\r", ""), "\n") + for _, l := range lines { + if generatedFooterRe.MatchString(l) { + return BodyGeneratedFooter + } + } + for _, l := range lines { + if coAuthorRe.MatchString(l) { + return BodyCoAuthor + } + } + for _, l := range lines { + if v, ok := strings.CutPrefix(l, trailerKey); ok && NamesAbcd(v) && !IsComposed(v) { + return BodyMalformedLabel + } + } + for _, l := range lines { + if v, ok := strings.CutPrefix(l, trailerKey); ok && (IsModelValue(v) || v == "None" || v == "none" || IsComposed(v)) { + return BodyClean + } + } + return BodyNoTrailer +} + +// stripFencedBlocks is the gate's strip_fenced_blocks: it drops each markdown +// fenced block, opener and closer included. The gate's comment carries the +// reasoning for every rule, and the corpus in body_test.go holds this copy to +// the gate's behaviour on each: an opener is a run of three or more backticks +// or tildes indented at most three spaces (a backtick run may not carry a +// backtick after it); a closer is a run of the same character at least as long +// with nothing but blanks after it; an unterminated fence, or any line that +// opens an HTML block (a `<` indented at most three spaces), strips nothing. +func stripFencedBlocks(text string) string { + if text == "" { + return "" + } + lines := strings.Split(text, "\n") + if strings.HasSuffix(text, "\n") { + lines = lines[:len(lines)-1] + } + for _, l := range lines { + t := strings.TrimLeft(l, " ") + if len(l)-len(t) <= 3 && strings.HasPrefix(t, "<") { + return joinLines(lines) + } + } + var kept []string + inside, openChar, openLen := false, byte(0), 0 + for _, l := range lines { + c, n, rest, ok := fenceRun(l) + if !inside { + if !ok || (c == '`' && strings.Contains(rest, "`")) { + kept = append(kept, l) + continue + } + inside, openChar, openLen = true, c, n + continue + } + if ok && c == openChar && n >= openLen && strings.Trim(rest, " \t\r") == "" { + inside = false + } + } + if inside { + return joinLines(lines) + } + return joinLines(kept) +} + +// fenceRun is the gate's fenceparse: the fence character a line opens or +// closes with, its run length and the text after the run, or ok false when +// the line can do neither. +func fenceRun(l string) (c byte, n int, rest string, ok bool) { + t := strings.TrimLeft(l, " ") + if len(l)-len(t) > 3 || t == "" || (t[0] != '`' && t[0] != '~') { + return 0, 0, "", false + } + c = t[0] + for n < len(t) && t[n] == c { + n++ + } + if n < 3 { + return 0, 0, "", false + } + return c, n, t[n:], true +} + +func joinLines(lines []string) string { + if len(lines) == 0 { + return "" + } + return strings.Join(lines, "\n") + "\n" +} diff --git a/internal/core/assistedby/body_test.go b/internal/core/assistedby/body_test.go new file mode 100644 index 0000000000..6c7e4b5224 --- /dev/null +++ b/internal/core/assistedby/body_test.go @@ -0,0 +1,134 @@ +package assistedby + +import ( + "os" + "os/exec" + "path/filepath" + "testing" +) + +// bodyCorpus is every shape the gate's body arm decides on: the three accepted +// forms and their near misses, the co-author, footer and label refusals, and the +// fence stripping each refusal is read after (the fence kinds, their indent, an +// unterminated fence, an HTML block that turns stripping off, CRLF). +var bodyCorpus = map[string]string{ + "model trailer": "Summary\n\nAssisted-by: Claude:claude-opus-5-5\n", + "bracketed model": "Summary\n\nAssisted-by: Claude:claude-opus-5[1m]", + "two models": "Summary\n\nAssisted-by: Claude:a\nAssisted-by: Vendor:b\n", + "none": "Summary\n\nAssisted-by: None\n", + "lower-case none": "Assisted-by: none", + "upper-case none": "Assisted-by: NONE", + "abcd label dev": "Composed.\n\nAssisted-by: abcd:dev\n", + "abcd label release": "Assisted-by: abcd:v0.15.1", + "abcd label malformed": "Assisted-by: abcd:latest\nAssisted-by: Claude:x\n", + "abcd label wrong case": "Assisted-by: ABCD:dev\nAssisted-by: Claude:x\n", + "no trailer": "Summary\n", + "empty": "", + "mid-line trailer": "Closes #3. Assisted-by: Claude:claude-opus-5-5", + "trailing space": "Assisted-by: Claude:claude-opus-5-5 ", + "leading space": " Assisted-by: Claude:claude-opus-5-5", + "bare vendor": "Assisted-by: Claude", + "crlf": "Summary\r\n\r\nAssisted-by: Claude:x\r\n", + "crlf fence": "```\r\nCo-authored-by: A \r\n```\r\nAssisted-by: Claude:x\r\n", + "co-author": "Co-authored-by: A \nAssisted-by: Claude:x\n", + "co-author mixed case": " co-Authored-By: A\nAssisted-by: Claude:x\n", + "co-author fenced": "```\nCo-authored-by: A \n```\nAssisted-by: Claude:x\n", + "trailer only fenced": "Summary\n\n```\nAssisted-by: Claude:x\n```\n", + "footer": "🤖 Generated with [X](https://example.invalid)\nAssisted-by: Claude:x\n", + "footer emphasised": "_Generated with [X](https://example.invalid)_\nAssisted-by: Claude:x\n", + "footer bold robot": "**🤖 Generated by [X](https://example.invalid)**\nAssisted-by: Claude:x\n", + "footer fenced": "```\n🤖 Generated with [X](https://example.invalid)\n```\nAssisted-by: Claude:x\n", + "footer words without link": "Generated with care.\nAssisted-by: Claude:x\n", + "unterminated fence": "```\nCo-authored-by: A\nAssisted-by: Claude:x\n", + "html block": "
\n\n```\nCo-authored-by: A\n```\n\n
\nAssisted-by: Claude:x\n", + "html indented four": " \n```\nCo-authored-by: A\n```\nAssisted-by: Claude:x\n", + "tilde fence": "~~~\nCo-authored-by: A\n~~~\nAssisted-by: Claude:x\n", + "shorter run does not close": "````\nCo-authored-by: A\n```\nAssisted-by: Claude:x\n````\n", + "longer run closes": "```\nCo-authored-by: A\n`````\nAssisted-by: Claude:x\n", + "other marker does not close": "~~~\nAssisted-by: Claude:x\n```\n", + "backtick in info string": "``` a`b\nCo-authored-by: A\n```\nAssisted-by: Claude:x\n", + "tilde info with backtick": "~~~ a`b\nCo-authored-by: A\n~~~\nAssisted-by: Claude:x\n", + "closer with text": "```\nCo-authored-by: A\n``` x\n```\nAssisted-by: Claude:x\n", + "closer with trailing spaces": "```go\nCo-authored-by: A\n``` \t\nAssisted-by: Claude:x\n", + "fence indented three": " ```\nCo-authored-by: A\n ```\nAssisted-by: Claude:x\n", + "fence indented four": " ```\nCo-authored-by: A\n ```\nAssisted-by: Claude:x\n", + "fence led by a tab": "\t```\nCo-authored-by: A\n\t```\nAssisted-by: Claude:x\n", + "two backticks are no fence": "``\nCo-authored-by: A\n``\nAssisted-by: Claude:x\n", + "two fences": "```\na\n```\ntext\n~~~\nCo-authored-by: A\n~~~\nAssisted-by: Claude:x\n", + "blank line inside fence": "```\n\nCo-authored-by: A\n\n```\nAssisted-by: Claude:x\n", + "no final newline after fence": "Assisted-by: Claude:x\n```\nCo-authored-by: A\n```", +} + +// TestJudgeBodyMatchesTheGate ties JudgeBody to the attribution gate's body arm, +// the one place a pull-request body's disclosure is decided: the gate runs in CI +// without Go, so the two share a test rather than code. Every body in the corpus +// goes through the gate itself, with its session-URL half stubbed clean, and +// JudgeBody must accept exactly the bodies the gate accepts. +func TestJudgeBodyMatchesTheGate(t *testing.T) { + gate, err := filepath.Abs(filepath.Join("..", "..", "..", "scripts", "check-attribution.sh")) + if err != nil { + t.Fatal(err) + } + dir := t.TempDir() + // The gate delegates its session-URL half to `abcd lint outbound`; that half + // is the guard's other check, so a stub that clears everything leaves the + // disclosure half alone deciding. + stub := filepath.Join(dir, "outbound-clean") + if err := os.WriteFile(stub, []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil { + t.Fatal(err) + } + for name, body := range bodyCorpus { + t.Run(name, func(t *testing.T) { + f := filepath.Join(dir, "body.md") + if err := os.WriteFile(f, []byte(body), 0o644); err != nil { + t.Fatal(err) + } + cmd := exec.Command("bash", gate, "body", f) + cmd.Dir = dir + cmd.Env = append(os.Environ(), "ABCD_OUTBOUND_BIN="+stub) + out, err := cmd.CombinedOutput() + gateAccepts := err == nil + if ee, ok := err.(*exec.ExitError); err != nil && (!ok || ee.ExitCode() != 1) { + t.Fatalf("the gate could not judge the body: %v\n%s", err, out) + } + fault := JudgeBody(body) + if got := fault == BodyClean; got != gateAccepts { + t.Fatalf("JudgeBody(%q) = %v, but the gate %s it:\n%s", body, fault, map[bool]string{true: "accepts", false: "refuses"}[gateAccepts], out) + } + }) + } +} + +// TestJudgeBodyNamesTheFirstFault: the fault reported is the one the gate +// reports, in the gate's order (a footer, then a co-author, then a malformed +// label, then no trailer at all), so the remedy handed back is the right one. +func TestJudgeBodyNamesTheFirstFault(t *testing.T) { + for _, tc := range []struct { + body string + want BodyFault + }{ + {"Assisted-by: Claude:x", BodyClean}, + {"🤖 Generated with [X](y)\nCo-authored-by: A", BodyGeneratedFooter}, + {"Co-authored-by: A\nAssisted-by: abcd:latest", BodyCoAuthor}, + {"Assisted-by: abcd:latest", BodyMalformedLabel}, + {"Summary", BodyNoTrailer}, + } { + if got := JudgeBody(tc.body); got != tc.want { + t.Errorf("JudgeBody(%q) = %v, want %v", tc.body, got, tc.want) + } + } +} + +// TestBodyRefusalPatternsAreTheGates: the footer and co-author patterns are the +// gate's GENERATED_RE and COAUTHOR_RE, read out of the script. +func TestBodyRefusalPatternsAreTheGates(t *testing.T) { + if got := gateAssignment(t, "GENERATED_RE"); got != generatedFooterPattern { + t.Errorf("the gate's GENERATED_RE is\n\t%s\nbut internal/core/assistedby reads\n\t%s\none of the two moved alone", got, generatedFooterPattern) + } + if got := gateAssignment(t, "COAUTHOR_RE"); got != coAuthorPattern { + t.Errorf("the gate's COAUTHOR_RE is\n\t%s\nbut internal/core/assistedby reads\n\t%s\none of the two moved alone", got, coAuthorPattern) + } + if got, want := gateAssignment(t, "NONE_RE"), `^Assisted-by: [Nn]one$`; got != want { + t.Errorf("the gate's NONE_RE is\n\t%s\nbut internal/core/assistedby reads\n\t%s\none of the two moved alone", got, want) + } +} diff --git a/internal/core/capture/mentions.go b/internal/core/capture/mentions.go index a816ad7490..82d6973c84 100644 --- a/internal/core/capture/mentions.go +++ b/internal/core/capture/mentions.go @@ -24,10 +24,10 @@ package capture import ( "fmt" - "regexp" "sort" "strings" + "github.com/intentdriven/abcd/internal/core/issuemention" "github.com/intentdriven/abcd/internal/gitutil" ) @@ -50,26 +50,10 @@ const ( // strengthRank orders the vocabulary for "the strongest evidence on this row". var strengthRank = map[string]int{StrengthRecord: 1, StrengthTree: 2, StrengthResolves: 3} -// mentionRe finds an iss-N anywhere in free prose. The leading guard is the Go -// twin of MENTION_RE in scripts/check-issue-resolution.sh, and it exists for the -// same reason: `xiss-1` inside a longer token is not a mention of iss-1, and -// `iss-12` is not a mention of iss-1 either (the greedy digit run takes the -// whole number). Keeping the two spellings side by side is deliberate — the gate -// refuses what this listing would otherwise have to report. -var mentionRe = regexp.MustCompile(`(?:^|[^A-Za-z0-9])(iss-[0-9]+)`) - -// declareRe is the Go twin of DECLARE_RE: the whole declaration vocabulary, -// nothing but the declaration on the line. A closed set of two spellings, for the -// reason the script states — admitting `Ref:`/`See:`/`Related:` would reopen the -// omission the rule closes. The id half is a COMMA-SEPARATED LIST because that is -// the conventional trailer shape (`Refs: iss-1, iss-2`); refusing it made an -// author write the trailer twice, or drop the second id, which is the omission -// the rule exists to close. `\r?` is here because a message may arrive -// CRLF-terminated from a forge-composed squash. -var declareRe = regexp.MustCompile(`(?m)^(Resolves|Refs):[ \t]+(iss-[0-9]+(?:[ \t]*,[ \t]*iss-[0-9]+)*)[ \t]*\r?$`) - -// declaredIDRe takes the ids back out of a declaration line's id list. -var declaredIDRe = regexp.MustCompile(`iss-[0-9]+`) +// The mention and declaration vocabulary is RS004's, in its one Go copy +// (core/issuemention), tied to scripts/check-issue-resolution.sh by a test: +// the gate refuses what this listing would otherwise have to report, so the +// two must read a message alike. // recordTierPrefix is the working-tree tier a mention can be written in without // anything being fixed: the record itself, the brief, the decision log. A commit @@ -280,27 +264,14 @@ func (c mentionCommit) subject() string { // judge applies the evidence rules to one commit and returns the strength it // contributes per open id. An id it says nothing usable about is absent. func (c mentionCommit) judge(open map[string]Issue) map[string]string { - mentioned := map[string]bool{} - for _, m := range mentionRe.FindAllStringSubmatch(c.body, -1) { - mentioned[m[1]] = true - } + mentioned := issuemention.Mentioned(c.body) if len(mentioned) == 0 { return nil } - declared := map[string]string{} - for _, m := range declareRe.FindAllStringSubmatch(c.body, -1) { - // One line declares the same relation for every id it lists. - for _, id := range declaredIDRe.FindAllString(m[2], -1) { - // `Resolves:` outranks `Refs:` if a message carries both for one id: - // the stronger claim is the one its author is answerable for. - if declared[id] != "Resolves" { - declared[id] = m[1] - } - } - } + declared := issuemention.Declared(c.body) out := map[string]string{} - for id := range mentioned { + for _, id := range mentioned { if _, isOpen := open[id]; !isOpen { continue } diff --git a/internal/core/claim/main_test.go b/internal/core/claim/main_test.go index 4e02eb5861..057688be53 100644 --- a/internal/core/claim/main_test.go +++ b/internal/core/claim/main_test.go @@ -1,18 +1,29 @@ package claim import ( + "fmt" "os" "testing" + "github.com/intentdriven/abcd/internal/gittest" "github.com/intentdriven/abcd/internal/testshard" ) // TestMain runs the package as several test processes when make preflight // asks: one is too long for its package budget on a loaded machine -// (iss-2610251004378066). +// (iss-2610251004378066). Each keeps the machine's own GitHub login out of +// reach: an announcement asks the forge whose account it is, and a test that +// reached the real login would pass here and fail on CI, which has none. func TestMain(m *testing.M) { if code, ok := testshard.Shard(); ok { os.Exit(code) } - os.Exit(m.Run()) + cleanup, err := gittest.HideGHLogin() + if err != nil { + fmt.Fprintln(os.Stderr, "claim tests: cannot hide the machine's gh login:", err) + os.Exit(1) + } + code := m.Run() + cleanup() + os.Exit(code) } diff --git a/internal/core/guard/forge.go b/internal/core/guard/forge.go index ec6fa7abb8..c36bb17d9e 100644 --- a/internal/core/guard/forge.go +++ b/internal/core/guard/forge.go @@ -10,6 +10,8 @@ import ( "sync" "github.com/intentdriven/abcd/internal/adapter/scanner" + "github.com/intentdriven/abcd/internal/core/assistedby" + "github.com/intentdriven/abcd/internal/core/issuemention" "github.com/intentdriven/abcd/internal/fsutil" ) @@ -39,6 +41,20 @@ import ( // out in full on the line, a `$(cat <<'EOF' … EOF)` the tokenizer reads as the // document it prints, and a body file read through the guard's guarded read. // What it does not see is named in commands/guard.md. +// +// Two checks a hosted run made after the push are made here too, on a pull +// request's own text (`gh pr create|new|edit`), because a failure either found +// in CI cost a push and a run to learn what the text on the line already said: +// +// - the disclosure (iss-2609061525437969): a body carries an Assisted-by line +// in one of the three accepted forms, judged as the attribution gate's +// body arm judges it (assistedby.JudgeBody), under the same opt-in, since +// the bundled COMMITTING rule holds a pull request's body to it as it holds +// a commit; +// - the mentions (iss-2610261152125261): every issue id the title and body +// name is declared by a `Resolves:` or `Refs:` line (RS004, through +// issuemention), under an opt-in of its own, `outbound.declare_mentions`, +// since RS004 is a rule a repository adopts, not one abcd bundles. const ( // forgeTextEntryID is the reserved id a posting command whose text breaks @@ -50,6 +66,16 @@ const ( // that does not load. A warn, never a block and never a silent pass. forgeUnreadEntryID = "forge-text-unread" + // forgeDisclosureEntryID is the reserved id a pull request's body is + // blocked under when it carries no Assisted-by line in an accepted form + // (iss-2609061525437969). + forgeDisclosureEntryID = "forge-text-disclosure" + + // forgeMentionEntryID is the reserved id a pull request's title and body + // are blocked under when they name an issue id without declaring their + // relation to it (iss-2610261152125261). + forgeMentionEntryID = "forge-text-mention" + familyForge = "forge text" // maxForgeTexts bounds the texts one check judges. Each builds a scanner @@ -78,49 +104,79 @@ const ( // .abcd or config.json, a value that is not true — is not opted in. It is the // one reader of the key; `abcd ahoy` asks it too. func ForgeTextOptedIn(repoRoot string) bool { + forge, _ := forgeOptIns(repoRoot) + return forge +} + +// ForgeMentionsOptedIn reports whether the repository at repoRoot also opted +// into the mention check: `outbound.declare_mentions: true` beside +// `outbound.forge: true`. `abcd land` asks it before it judges the body it +// composes, so a body is held to the rules the guard would hold it to. +func ForgeMentionsOptedIn(repoRoot string) bool { + _, mentions := forgeOptIns(repoRoot) + return mentions +} + +// forgeOptIns reads both of the check's keys from `.abcd/config.json`: +// `outbound.forge` and `outbound.declare_mentions`, each on only as the JSON +// boolean true, and the second only beside the first. +func forgeOptIns(repoRoot string) (forge, mentions bool) { if repoRoot == "" { - return false + return false, false } if di, err := os.Lstat(filepath.Join(repoRoot, ".abcd")); err != nil || di.Mode()&os.ModeSymlink != 0 { - return false + return false, false } data, err := fsutil.ReadGuarded(filepath.Join(repoRoot, ".abcd", "config.json"), maxForgeConfigBytes) if err != nil { - return false + return false, false } var cfg struct { Outbound struct { - Forge bool `json:"forge"` + Forge bool `json:"forge"` + DeclareMentions bool `json:"declare_mentions"` } `json:"outbound"` } if json.Unmarshal(data, &cfg) != nil { - return false + return false, false } - return cfg.Outbound.Forge + return cfg.Outbound.Forge, cfg.Outbound.Forge && cfg.Outbound.DeclareMentions } // forgeGate is the opt-in of the repository a registry was loaded for, read // the first time a posting command asks, so a line with no gh in it costs no // file read. type forgeGate struct { - root string - once sync.Once - armed bool + root string + once sync.Once + armed bool + mentions bool } func newForgeGate(root string) *forgeGate { return &forgeGate{root: root} } +func (g *forgeGate) read() { g.once.Do(func() { g.armed, g.mentions = forgeOptIns(g.root) }) } + func (g *forgeGate) on() bool { - g.once.Do(func() { g.armed = ForgeTextOptedIn(g.root) }) + g.read() return g.armed } +// mentionsOn reports whether the mention check runs too. +func (g *forgeGate) mentionsOn() bool { + g.read() + return g.mentions +} + // forgePost is one posting command on the line: the segment it stands in, the // command as `gh `, and the text values it hands the forge. +// opaque reports a text flag whose value the line does not fix (`--body +// "$BODY"`), which no check reads but which may hold what a check looks for. type forgePost struct { seg int command string values []forgeValue + opaque bool } // forgeValue is one value of --title, --body or --body-file. tok is the token @@ -180,8 +236,8 @@ func forgePostsOf(s segment, seg int) []forgePost { if !ok1 || !ok2 || !forgePosting(group, verb) { continue } - out = append(out, forgePost{seg: seg, command: "gh " + group + " " + verb, - values: forgeValues(s, a.idx+1+idx[1]+1)}) + values, opaque := forgeValues(s, a.idx+1+idx[1]+1) + out = append(out, forgePost{seg: seg, command: "gh " + group + " " + verb, values: values, opaque: opaque}) break } } @@ -222,11 +278,16 @@ func forgeFlag(w string) (name, val string, attached bool) { } // forgeValues reads the text flags in the words from start on, up to `--`. -func forgeValues(s segment, start int) []forgeValue { - var out []forgeValue +// opaque reports a text flag the line sets to a value it does not fix. +func forgeValues(s segment, start int) (out []forgeValue, opaque bool) { for j := start; j < len(s.tokens); j++ { w, fixed, ok := forgeWord(s, j) if !ok { + // `--body="$BODY"` is one word the line does not fix; its flag is + // still read from the word's head. + if name, _, _ := forgeFlag(s.tokens[j]); name != "" { + opaque = true + } continue } if w == "--" { @@ -247,11 +308,30 @@ func forgeValues(s segment, start int) []forgeValue { v, vfixed, vok := forgeWord(s, j) if !vok { // A value the line does not fix (`--body "$BODY"`) is not seen. + opaque = true continue } out = append(out, forgeValue{flag: name, text: v, tok: j, fixed: vfixed}) } - return out + return out, opaque +} + +// forgeSeen gathers the texts of one posting command the guard read in full, +// for the check that judges a pull request's title and body together. opaque +// is set by any text of it the guard did not read in full. +type forgeSeen struct { + titles, bodies []string + opaque bool +} + +// forgeHoldsPR reports whether command posts a pull request's own title and +// body, the text the disclosure and mention checks hold. +func forgeHoldsPR(command string) bool { + switch command { + case "gh pr create", "gh pr new", "gh pr edit": + return true + } + return false } // forgeText is the check over one reading of a command line: every posting @@ -276,6 +356,7 @@ func (r Registry) forgeText(rc *readCtx, segs []segment, recs []scriptSeg) []pay } judged, readBytes := 0, 0 for _, p := range posts { + seen := &forgeSeen{opaque: p.opaque} for _, v := range p.values { if judged >= maxForgeTexts { add(payloadSignal{ @@ -288,11 +369,12 @@ func (r Registry) forgeText(rc *readCtx, segs []segment, recs []scriptSeg) []pay } judged++ if v.flag == forgeBodyFile { - r.forgeFile(rc, segs, recs, p, v, &readBytes, add) + r.forgeFile(rc, segs, recs, p, v, &readBytes, add, seen) continue } - r.forgeJudge(p.command, v.flag, v.text, add) + r.forgeJudge(p.command, v.flag, v.text, add, seen) } + r.forgeMentions(p.command, seen, add) } return out } @@ -304,12 +386,16 @@ func (r Registry) forgeText(rc *readCtx, segs []segment, recs []scriptSeg) []pay // // readBytes is the bytes this check has read for body files so far, on their // own budget (maxForgeReadBytes), never the scripts'. -func (r Registry) forgeFile(rc *readCtx, segs []segment, recs []scriptSeg, p forgePost, v forgeValue, readBytes *int, add func(payloadSignal)) { +// +// Every way the file goes unjudged marks seen opaque. +func (r Registry) forgeFile(rc *readCtx, segs []segment, recs []scriptSeg, p forgePost, v forgeValue, readBytes *int, add func(payloadSignal), seen *forgeSeen) { if v.text == "-" || !v.fixed || strings.TrimSpace(v.text) == "" { + seen.opaque = true return } s := segs[p.seg] if v.tok >= 0 && s.globAt(v.tok) { + seen.opaque = true return } var paths []string @@ -331,9 +417,11 @@ func (r Registry) forgeFile(rc *readCtx, segs []segment, recs []scriptSeg, p for } } if unplaced { + seen.opaque = true add(forgeUnreadSignal(p.command, "is relative to a directory this line leaves unknown")) } if len(paths) == 0 { + seen.opaque = true return } var before []writeTarget @@ -346,11 +434,13 @@ func (r Registry) forgeFile(rc *readCtx, segs []segment, recs []scriptSeg, p for } for _, pth := range paths { if written, _ := writtenBefore(pth, before); written { + seen.opaque = true add(forgeUnreadSignal(p.command, "is written earlier on this line, so the file the guard could read now is not the one gh will post")) continue } real, err := filepath.EvalSymlinks(pth) if err != nil { + seen.opaque = true if errors.Is(err, os.ErrNotExist) { add(forgeUnreadSignal(p.command, "does not exist")) } else { @@ -359,6 +449,7 @@ func (r Registry) forgeFile(rc *readCtx, segs []segment, recs []scriptSeg, p for continue } if *readBytes >= maxForgeReadBytes { + seen.opaque = true add(forgeUnreadSignal(p.command, fmt.Sprintf("is past the %d KiB of body files one check reads", maxForgeReadBytes>>10))) continue } @@ -367,36 +458,132 @@ func (r Registry) forgeFile(rc *readCtx, segs []segment, recs []scriptSeg, p for tally(len(fr.data)) switch { case fr.err != nil: + seen.opaque = true add(forgeUnreadSignal(p.command, "cannot be read as a regular file (a pipe, a directory, or a file this account may not read)")) case fr.tooBig: + seen.opaque = true add(forgeUnreadSignal(p.command, fmt.Sprintf("is over the %d KiB the guard reads", maxScriptBytes>>10))) default: - r.forgeJudge(p.command, forgeBodyFile, string(fr.data), add) + r.forgeJudge(p.command, forgeBodyFile, string(fr.data), add, seen) } } } -// forgeJudge runs the outbound check over one text. The text is never carried -// into the verdict: only the kind and the line of what was found. -func (r Registry) forgeJudge(command, what, text string, add func(payloadSignal)) { +// forgeJudge runs the outbound check over one text, and over a pull request's +// body the disclosure check, and gathers it into seen. The text is never +// carried into the verdict: only the kind and the line of what was found. +// +// A text holding output the line does not fix (an unquoted document's +// variable) is judged for the outbound policy on the part it does fix, and +// marks seen opaque: the part it does not fix may hold the very line the +// disclosure and mention checks look for. +func (r Registry) forgeJudge(command, what, text string, add func(payloadSignal), seen *forgeSeen) { + whole := !strings.Contains(text, unknownText) && !strings.ContainsRune(text, varMark) text = strings.NewReplacer(unknownText, "", string(varMark), "").Replace(text) - if strings.TrimSpace(text) == "" { - return + switch { + case !whole: + seen.opaque = true + case what == forgeTitle: + seen.titles = append(seen.titles, text) + default: + seen.bodies = append(seen.bodies, text) + } + if strings.TrimSpace(text) != "" { + findings, err := scanner.CheckOutbound(r.forge.root, text, "the "+what+" of "+command) + if len(findings) > 0 { + // One refusal per text: the leak is the one that cannot be taken back. + add(forgeLeakSignal(command, what, findings)) + return + } + if err != nil { + add(forgeConfigSignal(command, what)) + } } - findings, err := scanner.CheckOutbound(r.forge.root, text, "the "+what+" of "+command) - if len(findings) > 0 { - add(forgeLeakSignal(command, what, findings)) + if whole && what != forgeTitle && forgeHoldsPR(command) { + if fault := assistedby.JudgeBody(text); fault != assistedby.BodyClean { + add(forgeDisclosureSignal(command, what, fault)) + } + } +} + +// forgeMentions is the mention check over one posting command, run once its +// texts are read: the ids the title and each body name, against the +// declarations the two carry. It judges only a body the guard read in full, +// with every title the line gives, and nothing when any text was opaque, +// since the part not read may be the declaration. +func (r Registry) forgeMentions(command string, seen *forgeSeen, add func(payloadSignal)) { + if !forgeHoldsPR(command) || seen.opaque || len(seen.bodies) == 0 || !r.forge.mentionsOn() { return } - if err != nil { - add(payloadSignal{ - id: forgeUnreadEntryID, verdict: VerdictWarn, family: familyForge, - reason: "The guard could not judge the " + what + " this `" + command + "` hands the forge: the repository's scanner " + - "configuration (.abcd/config/pii.json) does not load, and judging with fewer detectors than it declares would " + - "be a clearance nobody earned.", - successor: "Repair .abcd/config/pii.json, or remove any session URL and tool attribution footer from the text yourself " + - "before posting.", - }) + titles := seen.titles + if len(titles) == 0 { + titles = []string{""} + } + var ids []string + for _, t := range titles { + for _, b := range seen.bodies { + for _, id := range issuemention.Undeclared(t, b) { + if !containsString(ids, id) { + ids = append(ids, id) + } + } + } + } + if len(ids) > 0 { + add(forgeMentionSignal(command, ids)) + } +} + +// forgeConfigSignal is the warn for a text the guard could not judge because +// the scanner's configuration does not load. +func forgeConfigSignal(command, what string) payloadSignal { + return payloadSignal{ + id: forgeUnreadEntryID, verdict: VerdictWarn, family: familyForge, + reason: "The guard could not judge the " + what + " this `" + command + "` hands the forge: the repository's scanner " + + "configuration (.abcd/config/pii.json) does not load, and judging with fewer detectors than it declares would " + + "be a clearance nobody earned.", + successor: "Repair .abcd/config/pii.json, or remove any session URL and tool attribution footer from the text yourself " + + "before posting.", + } +} + +// forgeDisclosureSignal is the block for a pull request's body the +// attribution gate's body arm would refuse. It names the fault, never the text. +func forgeDisclosureSignal(command, what string, fault assistedby.BodyFault) payloadSignal { + fix := "Add a line of its own, outside any fenced block, in one of the three accepted forms: " + + "Assisted-by: : where a model assisted (Assisted-by: Claude:claude-opus-5-5), " + + "Assisted-by: None where none did, or the Assisted-by: abcd: label abcd itself writes; then run the same command again." + switch fault { + case assistedby.BodyCoAuthor: + fix = "Remove the Co-authored-by line and disclose assistance with Assisted-by: : on a line of its own instead, " + + "then run the same command again." + case assistedby.BodyGeneratedFooter: + fix = "Replace the footer with Assisted-by: : on a line of its own, then run the same command again." + case assistedby.BodyMalformedLabel: + fix = "Write the abcd label exactly as abcd writes it (Assisted-by: abcd:dev or Assisted-by: abcd:v1.2.3), or replace it with " + + "Assisted-by: : where a model wrote the text, then run the same command again." + } + return payloadSignal{ + id: forgeDisclosureEntryID, verdict: VerdictBlock, family: familyForge, + reason: "The " + what + " this `" + command + "` hands the forge carries " + fault.String() + ". This repository checks " + + "the text it posts (outbound.forge in .abcd/config.json), and a pull request's body discloses assistance as its " + + "commits do, judged as abcd's attribution gate judges a pull request's body.", + successor: fix, + } +} + +// forgeMentionSignal is the block for a pull request whose title and body +// name issue ids without declaring them. The ids are record ids, not secrets, +// so the refusal names them. +func forgeMentionSignal(command string, ids []string) payloadSignal { + return payloadSignal{ + id: forgeMentionEntryID, verdict: VerdictBlock, family: familyForge, + reason: "The title and body this `" + command + "` hands the forge name " + strings.Join(ids, ", ") + " without declaring " + + "their relation to it. This repository holds a pull request to the rule its commits follow (RS004; " + + "outbound.declare_mentions in .abcd/config.json), and the hosted check refuses it after the push.", + successor: "Add a line of its own to the body for each: Resolves: " + ids[0] + " if this change fixes it, or Refs: " + ids[0] + + " if it is touched but not fixed. One line may list several issue ids, comma-separated, and issue ids only: " + + "an intent or ADR id on the same line makes it no declaration. Then run the same command again.", } } diff --git a/internal/core/guard/forge_attribution_test.go b/internal/core/guard/forge_attribution_test.go new file mode 100644 index 0000000000..4b497908e4 --- /dev/null +++ b/internal/core/guard/forge_attribution_test.go @@ -0,0 +1,107 @@ +package guard + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// The disclosure half of the forge-text check (iss-2609061525437969). In a +// repository that opted in, a pull request's body discloses assistance as its +// commits do: a `gh pr create`, `gh pr new` or `gh pr edit` whose body or body +// file carries no Assisted-by line in one of the three accepted forms, judged as +// the attribution gate's body arm judges it, is blocked under +// forge-text-disclosure before the forge keeps it. + +func TestForgeDisclosureBlocksAPullRequestBodyWithoutTheLine(t *testing.T) { + _, reg := forgeRepo(t, forgeOptedIn) + cases := []string{ + `gh pr create --title "fix the walk" --body "Summary"`, + `gh pr create --title t --body ""`, + `gh pr new -b "Summary"`, + `gh pr edit 12 --body "Summary"`, + `gh pr create --body "Closes #3. Assisted-by: Claude:claude-opus-5-5"`, + "gh pr create --title t --body \"$(cat <<'EOF'\nSummary\n\n```\nAssisted-by: Claude:x\n```\nEOF\n)\"", + "gh pr create --body \"Summary\n\nCo-authored-by: A \nAssisted-by: Claude:x\"", + "gh pr create --body \"Summary\n\nAssisted-by: abcd:latest\"", + } + for _, cmd := range cases { + t.Run(cmd, func(t *testing.T) { + d := forgeCheck(t, reg, cmd) + if d.Verdict != VerdictBlock || d.EntryID != forgeDisclosureEntryID { + t.Fatalf("Check = %q via %q (%s), want block via %q", d.Verdict, d.EntryID, d.Message, forgeDisclosureEntryID) + } + if !strings.Contains(d.Message, "Assisted-by") { + t.Errorf("the refusal does not teach the accepted forms: %s", d.Message) + } + }) + } +} + +func TestForgeDisclosureAllowsEachAcceptedForm(t *testing.T) { + _, reg := forgeRepo(t, forgeOptedIn) + cases := []string{ + "gh pr create --title t --body \"Summary\n\nAssisted-by: Claude:claude-opus-5-5\"", + "gh pr create --title t --body \"Summary\n\nAssisted-by: None\"", + "gh pr create --title t --body \"Composed.\n\nAssisted-by: abcd:dev\"", + "gh pr create --title t --body \"Summary\r\n\r\nAssisted-by: Claude:claude-opus-5[1m]\r\n\"", + "gh pr edit 12 --body \"Summary\n\nAssisted-by: Claude:a\nAssisted-by: Vendor:b\"", + "gh pr create --title t --body \"$(cat <<'EOF'\nSummary\n\n```\nCo-authored-by: an example\n```\n\nAssisted-by: Claude:claude-opus-5-5\nEOF\n)\"", + // Only a pull request's body is held to it: a title, a comment and an + // issue are not. + `gh pr edit 12 --title "fix the walk"`, + `gh pr comment 12 --body "Thanks, merged."`, + `gh issue create --title t --body "Summary"`, + `gh issue edit 7 --body "Summary"`, + // A body the line does not fix is not seen. + `gh pr create --body "$BODY"`, + "gh pr create --body \"$(cat <8 ` as a redirect, so the later run warns +// script-written-then-run. That is the reading's defect, not the hook's +// (iss-2610261852072295), and its fix takes the ceiling back to zero. +const maxScriptCorpusWarns = 1 // scriptCorpusBlocks are the repository scripts that do run a blocker, and so // block when an agent runs them: each is named with what it runs. diff --git a/internal/core/guard/speculate.go b/internal/core/guard/speculate.go index 6c4ebb91bb..b331bce9b5 100644 --- a/internal/core/guard/speculate.go +++ b/internal/core/guard/speculate.go @@ -109,7 +109,7 @@ var reservedEntryIDs = []string{ syntheticEntryID, speculativeEntryID, braceEntryID, heredocEntryID, substitutionEntryID, gitConfigEntryID, stashEntryID, interpreterStreamEntryID, commandTooLongEntryID, unparsableEntryID, unknownProgramEntryID, ifsSplitEntryID, scriptHazardEntryID, scriptWrittenEntryID, scriptUnreadEntryID, - forgeTextEntryID, forgeUnreadEntryID, shellCDeleteEntryID, + forgeTextEntryID, forgeUnreadEntryID, forgeDisclosureEntryID, forgeMentionEntryID, shellCDeleteEntryID, unnamedProgramEntryID, inlineCodeEntryID, checkNoteEntryID, registryEntryID, promptToolEntryID, promptInputEntryID, protectedPathEntryID, criticalRemovalEntryID, sandboxEntryID, unfixedPathEntryID, gitConfigWriteEntryID, outsideRemovalEntryID, groupBrokenEntryID, nulByteEntryID, arithmeticEntryID, diff --git a/internal/core/guard/sweep_test.go b/internal/core/guard/sweep_test.go index be30c14504..b6f8e61bf9 100644 --- a/internal/core/guard/sweep_test.go +++ b/internal/core/guard/sweep_test.go @@ -150,7 +150,8 @@ func sweepPass(t *testing.T, r Registry, inputs []string) { if len(shown) > 160 { shown = shown[:160] + "…" } - if contains(d.Matches, forgeTextEntryID) || contains(d.Matches, forgeUnreadEntryID) { + if contains(d.Matches, forgeTextEntryID) || contains(d.Matches, forgeUnreadEntryID) || + contains(d.Matches, forgeDisclosureEntryID) || contains(d.Matches, forgeMentionEntryID) { forge++ t.Errorf("a forge-text verdict on ordinary work: %s %q", d.EntryID, shown) } diff --git a/internal/core/guard/testdata/corpus/forge-body.txt b/internal/core/guard/testdata/corpus/forge-body.txt new file mode 100644 index 0000000000..042f465623 --- /dev/null +++ b/internal/core/guard/testdata/corpus/forge-body.txt @@ -0,0 +1,4 @@ +A pull-request body read from a file: what the change does and the tests that +prove it, with the assistance disclosed on a line of its own. + +Assisted-by: Claude:claude-opus-5-5 diff --git a/internal/core/guard/testdata/corpus/forge-posting.txt b/internal/core/guard/testdata/corpus/forge-posting.txt index 0252a1c7d6..003b14ac3a 100644 --- a/internal/core/guard/testdata/corpus/forge-posting.txt +++ b/internal/core/guard/testdata/corpus/forge-posting.txt @@ -7,17 +7,23 @@ # session or a tool in prose, and a body file that exists and is clean. A # block or warn on any line here is a false positive the sweep counts, and a # forge-text verdict on any of them fails the sweep outright. +# +# A pull request's body carries its Assisted-by line on a line of its own, as +# the disclosure check asks (forge-text-disclosure), and a corpus line holds no +# newline: so a pull-request line here posts that line as its whole body, the +# summary in its title, and the body file is forge-body.txt. The bodies that +# check refuses are forge_attribution_test.go's. -gh pr create --title "fix: the walk skips a record family" --body "Closes #31. Assisted-by: Claude:claude-opus-5-5" -gh pr create -t "feat: an opt-in outbound gate" -b "Summary of the change, and the tests that prove it." -gh pr create --title="chore: capture two findings" --body="Records only; no code changes." -gh pr create -t"docs: say what the guard does not see" -b"One paragraph in commands/guard.md." -gh pr create --draft --base main --head feat/x --title "wip: the reader" --body "Draft while the review runs." +gh pr create --title "fix: the walk skips a record family" --body "Assisted-by: Claude:claude-opus-5-5" +gh pr create -t "feat: an opt-in outbound gate" -b "Assisted-by: Claude:claude-opus-5-5" +gh pr create --title="chore: capture two findings" --body="Assisted-by: None" +gh pr create -t"docs: say what the guard does not see" -b"Assisted-by: Claude:claude-opus-5-5" +gh pr create --draft --base main --head feat/x --title "wip: the reader" --body "Assisted-by: Claude:claude-opus-5-5" gh pr create --fill gh pr create --fill --label security --assignee @me -gh pr new --title "fix: a typo" --body "One word." +gh pr new --title "fix: a typo" --body "Assisted-by: None" gh pr edit 1012 --title "fix: count an exit 1 as a refusal only beside the verdict" -gh pr edit 1012 --body "Updated after review: the session store is now keyed on the root commit." +gh pr edit 1012 --body "Assisted-by: Claude:claude-opus-5-5" gh pr edit 1012 --add-label needs-review --remove-label wip gh pr comment 1012 --body "Re-ran the targeted tests; all pass. The session URL problem is fixed in the next commit." gh pr comment 1012 -b "LGTM once CI is green." @@ -29,7 +35,7 @@ gh issue edit 77 --title "renamed: the store is per machine" gh issue edit 77 --body "See https://github.com/intentdriven/abcd/pull/1012 for the fix." gh issue comment 77 --body "Fixed in #1012." gh issue comment 77 -b "Closing: duplicate of #75." -gh pr create --title t --body-file testdata/corpus/forge-posting.txt +gh pr create --title t --body-file testdata/corpus/forge-body.txt gh issue comment 77 -F testdata/corpus/forge-posting.txt gh pr view 1012 --json title,body gh pr list --state open --search "session" diff --git a/internal/core/implement/loop/drive_test.go b/internal/core/implement/loop/drive_test.go index 677ace8e3a..5661d4a0a7 100644 --- a/internal/core/implement/loop/drive_test.go +++ b/internal/core/implement/loop/drive_test.go @@ -41,6 +41,7 @@ import ( "github.com/intentdriven/abcd/internal/abcdhome" "github.com/intentdriven/abcd/internal/core/layered" "github.com/intentdriven/abcd/internal/core/runner" + "github.com/intentdriven/abcd/internal/gittest" "github.com/intentdriven/abcd/internal/testshard" ) @@ -59,7 +60,15 @@ func TestMain(m *testing.M) { if code, ok := testshard.Shard(); ok { os.Exit(code) } - os.Exit(m.Run()) + // Nor does a test reach the machine's own GitHub login, which CI lacks. + cleanup, err := gittest.HideGHLogin() + if err != nil { + fmt.Fprintln(os.Stderr, "loop tests: cannot hide the machine's gh login:", err) + os.Exit(1) + } + code := m.Run() + cleanup() + os.Exit(code) } // loopFakeHarness plays the harness its launch names: opencode's argv starts diff --git a/internal/core/issuemention/issuemention.go b/internal/core/issuemention/issuemention.go new file mode 100644 index 0000000000..e492108f83 --- /dev/null +++ b/internal/core/issuemention/issuemention.go @@ -0,0 +1,79 @@ +// Package issuemention is RS004's vocabulary in Go: the issue ids a text names, +// and the relation it declares to each with a `Resolves:` or `Refs:` line. +// +// scripts/check-issue-resolution.sh decides the rule (MENTION_RE, DECLARE_RE) +// and runs in CI without Go, so the two share a test rather than code +// (TestUndeclaredMatchesTheGate). This is its one Go copy, and a leaf: the +// shell guard judges a pull request's title and body with it before gh posts +// them (iss-2610261152125261), `abcd land` judges the body it composes with it, +// and capture's mentions listing reads history with it, and capture sits above +// the guard. +package issuemention + +import ( + "regexp" + "strings" +) + +// mentionRe finds an issue id anywhere in a text. The leading guard is the +// gate's hand-rolled word boundary: `xiss-1` inside a longer token is not a +// mention of iss-1, and the greedy digit run keeps `iss-12` from reading as a +// mention of iss-1. +var mentionRe = regexp.MustCompile(`(?:^|[^A-Za-z0-9])(iss-[0-9]+)`) + +// declareRe is the whole declaration vocabulary, a closed set of two +// spellings with nothing else on the line, its ids a comma-separated list of +// issue ids only. The gate's [[:space:]] is spelled out without the newline, +// since a grep line holds none and this pattern reads the whole text. +var declareRe = regexp.MustCompile(`(?m)^(Resolves|Refs):[ \t\v\f]+(iss-[0-9]+(?:[ \t\v\f]*,[ \t\v\f]*iss-[0-9]+)*)[ \t\v\f]*$`) + +// declaredIDRe takes the ids back out of a declaration line's list. +var declaredIDRe = regexp.MustCompile(`iss-[0-9]+`) + +// normalise drops every carriage return, as the gate does before it reads a +// line: a body typed into the forge's web form arrives CRLF-terminated. +func normalise(text string) string { return strings.ReplaceAll(text, "\r", "") } + +// Mentioned returns every issue id text names, declarations included, once +// each, in the order first named. +func Mentioned(text string) []string { + var out []string + seen := map[string]bool{} + for _, m := range mentionRe.FindAllStringSubmatch(normalise(text), -1) { + if !seen[m[1]] { + seen[m[1]] = true + out = append(out, m[1]) + } + } + return out +} + +// Declared returns the relation text declares to each id it declares one to: +// "Resolves" or "Refs". `Resolves:` outranks `Refs:` when a text carries both +// for one id, since the stronger claim is the one its author answers for. +func Declared(text string) map[string]string { + out := map[string]string{} + for _, m := range declareRe.FindAllStringSubmatch(normalise(text), -1) { + for _, id := range declaredIDRe.FindAllString(m[2], -1) { + if out[id] != "Resolves" { + out[id] = m[1] + } + } + } + return out +} + +// Undeclared returns the ids a pull request's title or body names that +// neither declares, once each, in the order first named: RS004 over a pull +// request, whose declaration set is read from both halves, since a title has +// no room for a trailer. +func Undeclared(title, body string) []string { + declared := Declared(title + "\n" + body) + var out []string + for _, id := range Mentioned(title + "\n" + body) { + if declared[id] == "" { + out = append(out, id) + } + } + return out +} diff --git a/internal/core/issuemention/issuemention_test.go b/internal/core/issuemention/issuemention_test.go new file mode 100644 index 0000000000..03aad32882 --- /dev/null +++ b/internal/core/issuemention/issuemention_test.go @@ -0,0 +1,109 @@ +package issuemention + +import ( + "os" + "os/exec" + "path/filepath" + "slices" + "testing" + + "github.com/intentdriven/abcd/internal/gittest" +) + +// prCorpus is every shape RS004 decides a pull request on: a mention declared +// in the body, in the title, or not at all; both spellings and the near misses +// the vocabulary closes; id lists, a mixed-family list, word boundaries, CRLF, +// and a fenced mention, which is a mention all the same. +var prCorpus = map[string]struct{ title, body string }{ + "no mention": {"fix the walk", "Summary\n"}, + "declared resolves": {"fix the walk", "Fixes it.\n\nResolves: iss-12\n"}, + "declared refs": {"touch iss-12", "Refs: iss-12\n"}, + "undeclared in body": {"fix", "See iss-12.\n"}, + "undeclared in title": {"fix iss-12", "Summary\n"}, + "declared by the title": {"Refs: iss-12", "See iss-12.\n"}, + "id list": {"fix", "iss-1 and iss-2\n\nRefs: iss-1, iss-2\n"}, + "id list missing one": {"fix", "iss-1 and iss-2 and iss-3\n\nRefs: iss-1, iss-2\n"}, + "mixed family": {"fix", "iss-9 and itd-146\n\nRefs: itd-146, iss-9\n"}, + "near miss ref": {"fix", "Ref: iss-12\n"}, + "near miss see": {"fix", "See: iss-12\n"}, + "lower-case keyword": {"fix", "refs: iss-12\n"}, + "keyword not alone": {"fix", "Refs: iss-12 and more\n"}, + "trailing space": {"fix", "Refs: iss-12 \n"}, + "tab after colon": {"fix", "Refs:\tiss-12\n"}, + "no space after colon": {"fix", "Refs:iss-12\n"}, + "indented declaration": {"fix", " Refs: iss-12\n"}, + "word boundary": {"fix", "xiss-12 and iss-12x\n"}, + "longer id is not shorter": {"fix", "iss-123\n\nRefs: iss-12\n"}, + "crlf": {"fix", "See iss-12.\r\n\r\nRefs: iss-12\r\n"}, + "fenced mention": {"fix", "```\niss-12\n```\n"}, + "fenced declaration": {"fix", "```\nRefs: iss-12\n```\n"}, + "resolves and refs": {"fix", "iss-1 iss-2\n\nResolves: iss-1\nRefs: iss-2\n"}, + "empty": {"", ""}, + "declaration on two lines": {"fix", "Refs:\niss-12\n"}, + "after punctuation": {"fix", "(iss-12), [iss-13]\n\nRefs: iss-12\n"}, +} + +// TestUndeclaredMatchesTheGate ties Undeclared to RS004's pull-request arm in +// scripts/check-issue-resolution.sh, the one place the rule is decided: the gate +// runs in CI without Go, so the two share a test rather than code. Every pull +// request in the corpus goes through the gate itself, and Undeclared must find +// an undeclared id in exactly the ones the gate refuses. +func TestUndeclaredMatchesTheGate(t *testing.T) { + gate, err := filepath.Abs(filepath.Join("..", "..", "..", "scripts", "check-issue-resolution.sh")) + if err != nil { + t.Fatal(err) + } + // The gate refuses to run outside a full git repository; the pull-request + // arm reads only the two files, so an empty one is enough. + repo := t.TempDir() + env := gittest.Env(t) + initRepo := exec.Command("git", "init", "-q", repo) + initRepo.Env = env + if out, err := initRepo.CombinedOutput(); err != nil { + t.Fatalf("git init: %v\n%s", err, out) + } + for name, pr := range prCorpus { + t.Run(name, func(t *testing.T) { + title, body := filepath.Join(repo, "title.txt"), filepath.Join(repo, "body.md") + if err := os.WriteFile(title, []byte(pr.title), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(body, []byte(pr.body), 0o644); err != nil { + t.Fatal(err) + } + cmd := exec.Command("bash", gate, "pr", title, body) + cmd.Dir = repo + cmd.Env = env + out, err := cmd.CombinedOutput() + gateAccepts := err == nil + if ee, ok := err.(*exec.ExitError); err != nil && (!ok || ee.ExitCode() != 1) { + t.Fatalf("the gate could not judge the pull request: %v\n%s", err, out) + } + undeclared := Undeclared(pr.title, pr.body) + if got := len(undeclared) == 0; got != gateAccepts { + t.Fatalf("Undeclared(%q, %q) = %v, but the gate %s it:\n%s", pr.title, pr.body, undeclared, + map[bool]string{true: "accepts", false: "refuses"}[gateAccepts], out) + } + }) + } +} + +func TestUndeclaredNamesEachIdOnceInOrder(t *testing.T) { + got := Undeclared("fix iss-3 and iss-1", "iss-1, iss-2, iss-3 again\n\nRefs: iss-2\n") + if want := []string{"iss-3", "iss-1"}; !slices.Equal(got, want) { + t.Fatalf("Undeclared = %v, want %v", got, want) + } +} + +func TestDeclaredResolvesOutranksRefs(t *testing.T) { + got := Declared("Refs: iss-1\nResolves: iss-1, iss-2\nRefs: iss-2\nRefs: iss-3\n") + want := map[string]string{"iss-1": "Resolves", "iss-2": "Resolves", "iss-3": "Refs"} + if len(got) != len(want) { + t.Fatalf("Declared = %v, want %v", got, want) + } + for id, kw := range want { + if got[id] != kw { + t.Errorf("Declared[%s] = %q, want %q", id, got[id], kw) + } + } +} diff --git a/internal/core/landing/land.go b/internal/core/landing/land.go index 0abcfbe547..fc8a7274b8 100644 --- a/internal/core/landing/land.go +++ b/internal/core/landing/land.go @@ -46,9 +46,11 @@ package landing // 7. enqueue (land.enqueue): the open pull request for the branch, armed by // the merge gate (mergeGate), pinned to the head, and confirmed in the // merge queue (PRStateQuery, asked of the repository the forge client -// named when origin was checked); with no pull request it prints the -// `gh pr create` command and stops. It never opens a pull request, never overrides an arming -// someone else made, and never edits the repository's settings. +// named when origin was checked); with no pull request it composes the +// body from the branch's commits into the local tier, prints the +// `gh pr create` command naming it, and stops (prbody.go). It never opens a +// pull request, never overrides an arming someone else made, and never +// edits the repository's settings. // // The trust boundaries: the remote is `origin` only, and its every fetch and // push URL must be the repository the forge client resolves here; a pull @@ -1160,8 +1162,9 @@ const ( ) // listOwnPR lists branch's one open pull request from this repository. ok is -// false with a nil error when there is none, and the stop is reported. -func listOwnPR(res *Result, o Options, dir string) (openPR, bool, error) { +// false with a nil error when there is none, and the stop is reported, with the +// body composed from the commits tracking..HEAD (createStop). +func listOwnPR(res *Result, o Options, dir, tracking string) (openPR, bool, error) { out, err := forge(o, dir, openPRArgs(res.Branch)...) if err != nil { return openPR{}, false, err @@ -1183,8 +1186,7 @@ func listOwnPR(res *Result, o Options, dir string) (openPR, bool, error) { return openPR{}, false, refuse("pull request #"+fmt.Sprint(listed[0].Number)+" for "+res.Branch+" comes from another repository (a fork), and land arms only a pull request from this repository's own branch", "merge it by hand on the forge, or open one from this repository's branch, then run abcd land again") case len(prs) == 0: - res.add("enqueue", StatusStopped, "no open pull request for "+res.Branch+"; land opens none: open it with the command, then run abcd land again to queue it", - "gh pr create --base "+shellQuote(res.Default)+" --head "+shellQuote(res.Branch)) + createStop(res, dir, tracking) return openPR{}, false, nil case len(prs) > 1: return openPR{}, false, refuse("the forge lists more than one open pull request for "+res.Branch+" from this repository", "close all but one, then run abcd land again") @@ -1200,7 +1202,7 @@ func enqueue(res *Result, o Options, dir, tracking, nameWithOwner string, moved res.add("enqueue", StatusOff, res.Enqueue.offText()+": nothing was armed or queued", "") return nil } - pr, ok, err := listOwnPR(res, o, dir) + pr, ok, err := listOwnPR(res, o, dir, tracking) if err != nil || !ok { return err } @@ -1212,7 +1214,7 @@ func enqueue(res *Result, o Options, dir, tracking, nameWithOwner string, moved for moved && pr.HeadRefOid != res.Head && waited < headWaitCap { o.Sleep(headWaitInterval) waited += headWaitInterval - if pr, ok, err = listOwnPR(res, o, dir); err != nil || !ok { + if pr, ok, err = listOwnPR(res, o, dir, tracking); err != nil || !ok { return err } } diff --git a/internal/core/landing/land_body_test.go b/internal/core/landing/land_body_test.go new file mode 100644 index 0000000000..4cdf59718c --- /dev/null +++ b/internal/core/landing/land_body_test.go @@ -0,0 +1,208 @@ +package landing + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/intentdriven/abcd/internal/core" + "github.com/intentdriven/abcd/internal/core/assistedby" + "github.com/intentdriven/abcd/internal/core/issuemention" +) + +// With no pull request open, land composes the body from the branch's own +// commits (iss-220): their subjects, their Refs:, Resolves: and Delivers: lines +// and their Assisted-by: lines, judged with the checks the shell guard applies +// to a posted body, written to the checkout's local tier, and named by the +// create command's --body-file. A body nobody composed is the one CI refused +// for a missing disclosure line, after the push. + +// bodyFileOf is the --body-file path the create command names, read back. +func bodyFileOf(t *testing.T, cmd string) (path, body string) { + t.Helper() + _, rest, ok := strings.Cut(cmd, " --body-file ") + if !ok { + t.Fatalf("the create command names no body file: %s", cmd) + } + path = strings.Trim(strings.Fields(rest)[0], "'") + b, err := os.ReadFile(path) + if err != nil { + t.Fatalf("the body file the command names cannot be read: %v", err) + } + return path, string(b) +} + +func TestNoPullRequestComposesTheBodyFromTheCommits(t *testing.T) { + f := newFixture(t, queueRuleset) + f.repo.Git("commit", "-q", "--amend", "-m", "fix: the walk skips a record family\n\nThe walk now reads every family (iss-1).\n\nRefs: iss-1\nResolves: iss-2\nAssisted-by: Claude:claude-opus-5-5") + f.repo.Write("second.txt", "more\n") + f.repo.Git("add", "second.txt") + f.repo.Git("commit", "-q", "-m", "docs: say which families the walk reads\n\nDelivers: itd-3\nAssisted-by: Claude:claude-opus-5-5\nAssisted-by: ExampleVendor:model-b") + // The trailers make the landing announce before it pushes. Git's identity + // names the sender, so no forge is asked whose account it is, and the + // embargo check, which is not what this test is about, lets it through. + f.asPat() + o := f.options() + o.Announce.Embargo = func(_, _, _ string, _ []string) (bool, error) { return false, nil } + res, err := f.land(o) + if err != nil { + t.Fatal(err) + } + s := step(t, res, "enqueue") + if s.Status != StatusStopped || !strings.HasPrefix(s.Command, "gh pr create --base main --head feat/x --title ") { + t.Fatalf("enqueue step %+v, want stopped with the create command", s) + } + if !strings.Contains(s.Command, "--title 'fix: the walk skips a record family'") { + t.Errorf("the title is not the branch's first commit's subject: %s", s.Command) + } + path, body := bodyFileOf(t, s.Command) + // Both resolved: a temporary directory may sit behind a symbolic link + // (/var on macOS), and land names the checkout git reports. + tier, err := filepath.EvalSymlinks(filepath.Join(f.repo.Root(), ".abcd", ".work.local")) + if err != nil { + t.Fatal(err) + } + if real, err := filepath.EvalSymlinks(path); err != nil || !strings.HasPrefix(real, tier+string(filepath.Separator)) { + t.Errorf("the body file %s is not in the checkout's local tier %s (%v)", path, tier, err) + } + for _, want := range []string{ + "- fix: the walk skips a record family\n", + "- docs: say which families the walk reads\n", + "\nRefs: iss-1\n", "\nResolves: iss-2\n", "\nDelivers: itd-3\n", + "\nAssisted-by: " + assistedby.ComposedValue(core.Version) + "\n", + "\nAssisted-by: Claude:claude-opus-5-5\n", "\nAssisted-by: ExampleVendor:model-b\n", + } { + if !strings.Contains(body, want) { + t.Errorf("the composed body lacks %q:\n%s", want, body) + } + } + if n := strings.Count(body, "Assisted-by: Claude:claude-opus-5-5"); n != 1 { + t.Errorf("a model two commits disclose is named %d times, want once:\n%s", n, body) + } + if strings.Contains(body, "The walk now reads") { + t.Errorf("the body carries a commit's prose, which it does not compose from:\n%s", body) + } + if fault := assistedby.JudgeBody(body); fault != assistedby.BodyClean { + t.Errorf("the composed body fails the disclosure check (%v):\n%s", fault, body) + } + if ids := issuemention.Undeclared("fix: the walk skips a record family", body); len(ids) != 0 { + t.Errorf("the composed body names %v without declaring them:\n%s", ids, body) + } + if strings.Contains(s.Note, "fails") { + t.Errorf("a body that passes its checks is reported as failing one: %s", s.Note) + } + if f.forge.called("pr", "create") != 0 { + t.Fatalf("forge calls %v: land opens no pull request", f.forge.calls) + } + if dirty, err := dirtyTree(f.repo.Root()); err != nil || dirty { + t.Fatalf("writing the body file left the checkout dirty (%v)", err) + } +} + +// TestAComposedBodyThatFailsACheckSaysSo: in a repository that opted into the +// mention check, a branch whose subject names an issue it never declares +// composes a body the guard would block, and the stop says which check it +// fails rather than handing over a body that will be refused. +func TestAComposedBodyThatFailsACheckSaysSo(t *testing.T) { + f := newFixture(t, queueRuleset) + f.repo.Write(".abcd/config.json", `{"outbound": {"forge": true, "declare_mentions": true}}`+"\n") + f.repo.Git("add", ".abcd/config.json") + f.repo.Git("commit", "-q", "--amend", "-m", "fix: the walk behind iss-7\n\nAssisted-by: None") + res, err := f.land(f.options()) + if err != nil { + t.Fatal(err) + } + s := step(t, res, "enqueue") + if s.Status != StatusStopped || !strings.Contains(s.Note, "iss-7") || !strings.Contains(s.Note, "fails") { + t.Fatalf("enqueue step %+v, want the stop to name the mention the body leaves undeclared", s) + } +} + +// TestADisclosureFaultIsNamedForWhatItIs: only a body with no disclosure is +// told to add one; a body carrying a line the guard refuses, a Co-authored-by +// a commit wrote, is told to correct that line. +func TestADisclosureFaultIsNamedForWhatItIs(t *testing.T) { + dir := t.TempDir() + note := judgeBody(dir, composedBody{title: "fix: x", body: "fix: x\n\nAssisted-by: None\nCo-authored-by: Alice Example \n"}) + if !strings.Contains(note, "Co-authored-by") || !strings.Contains(note, "correct or remove") { + t.Errorf("a body carrying a Co-authored-by line: %q, want the line named and corrected", note) + } + if strings.Contains(note, "disclose none") || strings.Contains(note, "add an Assisted-by") { + t.Errorf("a body that discloses is told it discloses none: %q", note) + } + if note := judgeBody(dir, composedBody{title: "fix: x", body: "fix: x\n"}); !strings.Contains(note, "add an Assisted-by line") { + t.Errorf("a body with no disclosure: %q, want it told to add one", note) + } +} + +// TestAnUnansweredIgnoreCheckIsNotCalledNotIgnored: check-ignore exits 128 +// when git cannot answer (here, no repository at all), which says nothing +// about whether the tier is ignored. +func TestAnUnansweredIgnoreCheckIsNotCalledNotIgnored(t *testing.T) { + path, note := writeBody(t.TempDir(), "feat/x", "fix: x\n") + if path != "" { + t.Fatalf("a body was written where git could not answer: %s", path) + } + if strings.Contains(note, "is not ignored") || !strings.Contains(note, "could not say") { + t.Errorf("note %q, want it to say git could not answer", note) + } +} + +// TestNoLocalTierWritesNoBody: land makes no local tier, so a worktree with +// none (its receipt minted in another worktree) gets the bare create command +// and a note saying why, and still has no tier afterwards. +func TestNoLocalTierWritesNoBody(t *testing.T) { + f := newFixture(t, queueRuleset) + f.forge.prs = "[]" + f.forge.head = f.head() + wt := filepath.Join(t.TempDir(), "wt") + f.repo.Git("worktree", "add", "-q", "-b", "feat/y", wt, "feat/x") + if err := f.mint(f.repo.Root()); err != nil { + t.Fatal(err) + } + res, err := Land(wt, f.options()) + if err != nil { + t.Fatal(err) + } + s := step(t, res, "enqueue") + if s.Status != StatusStopped || strings.Contains(s.Command, "--body-file") || !strings.Contains(s.Note, "no local tier") { + t.Fatalf("enqueue step %+v, want the bare create command and the missing tier named", s) + } + if _, err := os.Stat(filepath.Join(wt, ".abcd", ".work.local")); err == nil { + t.Fatal("land made the worktree's local tier") + } +} + +// TestTheComposedBodyDeclaresAbcdsLabel: abcd composed the body's text, so it +// carries abcd's own label whatever the commits declare (ruling R4), then the +// models they name; a commit's None is not carried, since a tool assembled the +// text the body is. +func TestTheComposedBodyDeclaresAbcdsLabel(t *testing.T) { + label := "\nAssisted-by: " + assistedby.ComposedValue(core.Version) + "\n" + for name, tc := range map[string]struct { + second string + want string + unwant string + }{ + "all none": {"Assisted-by: None", label, "Assisted-by: None"}, + "one helped": {"Assisted-by: Claude:claude-opus-5-5", label + "Assisted-by: Claude:claude-opus-5-5\n", "Assisted-by: None"}, + "one silent": {"", label, "Assisted-by: None"}, + } { + t.Run(name, func(t *testing.T) { + f := newFixture(t, queueRuleset) + f.repo.Git("commit", "-q", "--amend", "-m", "fix: the walk\n\nAssisted-by: None") + f.repo.Write("second.txt", "more\n") + f.repo.Git("add", "second.txt") + f.repo.Git("commit", "-q", "-m", "fix: the walk again\n\n"+tc.second) + res, err := f.land(f.options()) + if err != nil { + t.Fatal(err) + } + _, body := bodyFileOf(t, step(t, res, "enqueue").Command) + if !strings.Contains(body, tc.want) || strings.Contains(body, tc.unwant) { + t.Fatalf("composed body, want %q and no %q:\n%s", tc.want, tc.unwant, body) + } + }) + } +} diff --git a/internal/core/landing/land_test.go b/internal/core/landing/land_test.go index 2e10dfa1cb..7b38a0070c 100644 --- a/internal/core/landing/land_test.go +++ b/internal/core/landing/land_test.go @@ -490,7 +490,7 @@ func TestNoPullRequestPrintsTheCreateCommand(t *testing.T) { t.Fatal(err) } s := step(t, res, "enqueue") - if s.Status != StatusStopped || s.Command != "gh pr create --base main --head feat/x" { + if s.Status != StatusStopped || !strings.HasPrefix(s.Command, "gh pr create --base main --head feat/x --title ") { t.Fatalf("enqueue step %+v, want stopped with the create command", s) } if f.forge.called("pr", "create") != 0 || f.forge.called("pr", "merge") != 0 { diff --git a/internal/core/landing/main_test.go b/internal/core/landing/main_test.go new file mode 100644 index 0000000000..16c08b4f8f --- /dev/null +++ b/internal/core/landing/main_test.go @@ -0,0 +1,24 @@ +package landing + +import ( + "fmt" + "os" + "testing" + + "github.com/intentdriven/abcd/internal/gittest" +) + +// TestMain keeps the machine's own GitHub login out of reach: a landing +// announces before it pushes and asks the forge whose account it is when git's +// identity does not say, so a test that reached the real login would pass here +// and fail on CI, which has none. +func TestMain(m *testing.M) { + cleanup, err := gittest.HideGHLogin() + if err != nil { + fmt.Fprintln(os.Stderr, "landing tests: cannot hide the machine's gh login:", err) + os.Exit(1) + } + code := m.Run() + cleanup() + os.Exit(code) +} diff --git a/internal/core/landing/prbody.go b/internal/core/landing/prbody.go new file mode 100644 index 0000000000..e1861a50bf --- /dev/null +++ b/internal/core/landing/prbody.go @@ -0,0 +1,188 @@ +package landing + +import ( + "errors" + "fmt" + "io/fs" + "os" + "os/exec" + "path" + "path/filepath" + "regexp" + "strings" + + "github.com/intentdriven/abcd/internal/adapter/scanner" + "github.com/intentdriven/abcd/internal/core" + "github.com/intentdriven/abcd/internal/core/assistedby" + "github.com/intentdriven/abcd/internal/core/guard" + "github.com/intentdriven/abcd/internal/core/issuemention" + "github.com/intentdriven/abcd/internal/fsutil" + "github.com/intentdriven/abcd/internal/gitutil" +) + +// The pull-request body land composes when it finds no pull request to queue +// (iss-220). land still opens none: it writes the body to the checkout's local +// tier and names it in the create command it prints, so the body the person +// posts carries what the hosted checks ask of it. A body typed by hand was the +// one CI refused for a missing Assisted-by line, after the push. +// +// The body is the branch's own record: the subject of each commit, then each +// commit's Refs:, Resolves: and Delivers: lines, then abcd's own Assisted-by +// label and the models the commits' Assisted-by: lines name, once each, in the +// order the commits give them. It carries no commit's prose. It is +// judged with the checks the shell guard applies to a posted body: the +// outbound policy, the disclosure (assistedby.JudgeBody) and, where the +// repository opted in, the mentions (issuemention); a body failing one is still +// written, and the stop says which check it fails. + +// bodyRelDir is where the body is written, in the checkout's local tier. +const bodyRelDir = ".abcd/.work.local/scratch/land" + +// maxComposedCommits bounds the commits a body is composed from: a branch that +// far ahead is not one pull request's worth, and its body is not composed. +const maxComposedCommits = 200 + +var ( + declareLineRe = regexp.MustCompile(`^(Resolves|Refs):[ \t]+iss-[0-9]+([ \t]*,[ \t]*iss-[0-9]+)*[ \t]*$`) + deliversLineRe = regexp.MustCompile(`^Delivers:[ \t]+itd-[0-9]+([ \t]*,[ \t]*itd-[0-9]+)*[ \t]*$`) + unsafeNameRe = regexp.MustCompile(`[^A-Za-z0-9._-]+`) +) + +// composedBody is a body land composed: the title and body text, and the +// commits it was read from. +type composedBody struct { + title string + body string + commits int +} + +// composeBody reads the commits tracking..head in the checkout at dir, oldest +// first, merges left out. +func composeBody(dir, tracking, head string) (composedBody, error) { + out, err := gitutil.Run(dir, "log", "--reverse", "--no-merges", fmt.Sprintf("--max-count=%d", maxComposedCommits+1), + "--format=%x1e%s%x1f%B", tracking+".."+head, "--") + if err != nil { + return composedBody{}, err + } + // abcd composed the body's text, so it declares abcd's label, and the + // change it describes carries the models' work, so their lines follow + // (ruling R4, internal/core/implement/loop/composed_attribution.go). A + // commit's None is not carried: the body is text a tool assembled. + var subjects, declared []string + assisted := []string{"Assisted-by: " + assistedby.ComposedValue(core.Version)} + for _, rec := range strings.Split(out, "\x1e") { + subject, msg, ok := strings.Cut(rec, "\x1f") + subject = strings.TrimSpace(subject) + if !ok || subject == "" { + continue + } + subjects = append(subjects, subject) + for _, l := range strings.Split(strings.ReplaceAll(msg, "\r", ""), "\n") { + switch v, isTrailer := strings.CutPrefix(l, "Assisted-by: "); { + case declareLineRe.MatchString(l), deliversLineRe.MatchString(l): + declared = appendOnce(declared, l) + case isTrailer && (assistedby.IsModelValue(v) || assistedby.IsComposed(v)): + assisted = appendOnce(assisted, l) + } + } + } + if len(subjects) == 0 { + return composedBody{}, fmt.Errorf("no commit between %s and %s to compose a body from", tracking, short(head)) + } + if len(subjects) > maxComposedCommits { + return composedBody{}, fmt.Errorf("the branch is more than %d commits ahead of %s", maxComposedCommits, tracking) + } + var b strings.Builder + for _, s := range subjects { + b.WriteString("- " + s + "\n") + } + b.WriteString("\n" + strings.Join(append(declared, assisted...), "\n") + "\n") + return composedBody{title: subjects[0], body: b.String(), commits: len(subjects)}, nil +} + +func appendOnce(list []string, s string) []string { + for _, x := range list { + if x == s { + return list + } + } + return append(list, s) +} + +// judgeBody names the first check of the shell guard's that body fails, or "" +// when it passes them all. dir's own configuration decides the mention check. +func judgeBody(dir string, c composedBody) string { + if findings, err := scanner.CheckOutbound(dir, c.body, "the composed pull-request body"); len(findings) > 0 { + return "the outbound check (it carries a live agent-session URL or a tool attribution footer)" + } else if err != nil { + return "the outbound check, which could not run (" + err.Error() + ")" + } + if fault := assistedby.JudgeBody(c.body); fault == assistedby.BodyNoTrailer { + return "the disclosure check: it carries " + fault.String() + ", since the branch's commits disclose none; " + + "add an Assisted-by line before posting" + } else if fault != assistedby.BodyClean { + return "the disclosure check: it carries " + fault.String() + ", taken from the branch's commits; " + + "correct or remove that line before posting" + } + if guard.ForgeMentionsOptedIn(dir) { + if ids := issuemention.Undeclared(c.title, c.body); len(ids) > 0 { + return "the mention check: it names " + strings.Join(ids, ", ") + " without a Resolves: or Refs: line" + } + } + return "" +} + +// writeBody writes the composed body to the checkout's local tier and returns +// its path, or "" with the reason it wrote none: a tier git does not ignore +// would leave the checkout dirty, and the next landing would refuse it. land +// makes no tier: a checkout without one (a new worktree whose receipt another +// worktree minted) keeps nothing of land's, the release hold's sightings +// included, and gets the bare create command. +func writeBody(dir, branch, body string) (string, string) { + rel := path.Join(bodyRelDir, "pr-body-"+strings.Trim(unsafeNameRe.ReplaceAllString(branch, "-"), "-")+".md") + // check-ignore exits 1 for a path git does not ignore, and 128 when it + // cannot answer at all. + if err := gitutil.WorkingTreeCommand(dir, "check-ignore", "-q", "--", rel).Run(); err != nil { + var ee *exec.ExitError + if errors.As(err, &ee) && ee.ExitCode() == 1 { + return "", "the local tier (" + bodyRelDir + ") is not ignored by git in this checkout, so land wrote no body file" + } + return "", "git could not say whether it ignores the local tier (" + err.Error() + "), so land wrote no body file" + } + root, err := os.OpenRoot(dir) + if err != nil { + return "", "the body file could not be written (" + err.Error() + ")" + } + defer root.Close() + if fi, err := root.Lstat(TierRelDir); errors.Is(err, fs.ErrNotExist) || (err == nil && !fi.IsDir()) { + return "", "this checkout has no local tier (" + TierRelDir + "), and land makes none, so it wrote no body file" + } else if err != nil { + return "", "the local tier could not be read (" + err.Error() + "), so land wrote no body file" + } + if err := fsutil.WriteFileAtomicInRoot(root, rel, []byte(body), 0o644); err != nil { + return "", "the body file could not be written (" + err.Error() + ")" + } + return filepath.Join(dir, filepath.FromSlash(rel)), "" +} + +// createStop is the stop when the branch has no pull request: the create +// command, with the composed body named where land could write one. +func createStop(res *Result, dir, tracking string) { + base := "gh pr create --base " + shellQuote(res.Default) + " --head " + shellQuote(res.Branch) + note := "no open pull request for " + res.Branch + "; land opens none: open it with the command, then run abcd land again to queue it" + c, err := composeBody(dir, tracking, res.Head) + if err != nil { + res.add("enqueue", StatusStopped, note+"; no body was composed ("+err.Error()+")", base) + return + } + file, why := writeBody(dir, res.Branch, c.body) + if file == "" { + res.add("enqueue", StatusStopped, note+"; "+why, base) + return + } + note += fmt.Sprintf("; the body was composed from the branch's %d commit(s) into %s", c.commits, file) + if fails := judgeBody(dir, c); fails != "" { + note += "; it fails " + fails + } + res.add("enqueue", StatusStopped, note, base+" --title "+shellQuote(c.title)+" --body-file "+shellQuote(file)) +} diff --git a/internal/core/lint/preflightscans_test.go b/internal/core/lint/preflightscans_test.go new file mode 100644 index 0000000000..85603924cd --- /dev/null +++ b/internal/core/lint/preflightscans_test.go @@ -0,0 +1,291 @@ +package lint_test + +import ( + "os" + "os/exec" + "path/filepath" + "regexp" + "slices" + "strings" + "testing" + + "github.com/intentdriven/abcd/internal/gittest" +) + +// The two hosted scans preflight runs before the push (iss-2610261151381604, +// iss-2610261151481013): govulncheck over the module and gitleaks over the +// commits the branch would land, the scans that failed 12 of 32 failed CI runs +// between 2026-10-18 and 2026-10-26. Both are preflight prerequisites, so a +// finding stops the run before its test stages wait at the load gate, and both +// take their versions from .github/workflows/ci.yml, so the local scan is CI's. + +func TestPreflightRunsTheHostedScans(t *testing.T) { + root := filepath.Join("..", "..", "..") + declared := preflightPrereqs(t, root) + for _, gate := range []string{"vulncheck", "secrets-check"} { + if !slices.Contains(declared, gate) { + t.Errorf("preflight does not run %s (it declares: %s), so a finding CI's scan makes is found only after the push", + gate, strings.Join(declared, " ")) + } + } +} + +// TestPreflightScansTakeTheirVersionsFromCI: the script names the version of +// each tool that ci.yml pins, read from ci.yml itself, so there is no second +// pin to fall behind. +func TestPreflightScansTakeTheirVersionsFromCI(t *testing.T) { + root, err := filepath.Abs(filepath.Join("..", "..", "..")) + if err != nil { + t.Fatal(err) + } + ci := readRepoFile(t, root, ".github/workflows/ci.yml") + vuln := regexp.MustCompile(`golang\.org/x/vuln/cmd/govulncheck@(v[0-9]+\.[0-9]+\.[0-9]+)`).FindStringSubmatch(ci) + leaks := regexp.MustCompile(`(?m)^\s*GITLEAKS_VERSION:\s*([0-9][0-9.]*)\s*$`).FindStringSubmatch(ci) + if vuln == nil || leaks == nil { + t.Fatal("ci.yml pins no govulncheck or no gitleaks version; the scans have nothing to match") + } + cmd := exec.Command("bash", filepath.Join(root, "scripts", "preflight-scans.sh"), "pins") + cmd.Dir = root + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("preflight-scans.sh pins: %v\n%s", err, out) + } + if want := "govulncheck " + vuln[1] + "\ngitleaks " + leaks[1] + "\n"; string(out) != want { + t.Fatalf("preflight-scans.sh pins printed\n%s\nwant\n%s", out, want) + } +} + +// scanRepo is a repository with a ci.yml pinning both tools, a default branch +// on origin, and a branch one commit ahead of it; stubs is a directory put +// first on the script's PATH. +type scanRepo struct { + t *testing.T + dir string + stubs string +} + +func newScanRepo(t *testing.T, changeGoMod bool) *scanRepo { + t.Helper() + r := &scanRepo{t: t, dir: t.TempDir(), stubs: t.TempDir()} + r.git("init", "-q", "-b", "main") + r.git("config", "user.name", "Alice Example") + r.git("config", "user.email", "alice@example.com") + r.git("config", "commit.gpgsign", "false") + r.write(".github/workflows/ci.yml", "jobs:\n gitleaks:\n env:\n GITLEAKS_VERSION: 8.24.3\n"+ + " govulncheck:\n steps:\n - run: go run golang.org/x/vuln/cmd/govulncheck@v1.7.0 ./...\n") + r.write("go.mod", "module example.com/m\n\ngo 1.26\n") + r.git("add", ".") + r.git("commit", "-q", "-m", "the base") + r.git("update-ref", "refs/remotes/origin/main", "HEAD") + r.git("checkout", "-q", "-b", "feat/x") + if changeGoMod { + r.write("go.mod", "module example.com/m\n\ngo 1.26\n\nrequire example.com/dep v1.2.3\n") + } else { + r.write("feature.txt", "the change\n") + } + r.git("add", ".") + r.git("commit", "-q", "-m", "the change") + return r +} + +func (r *scanRepo) git(args ...string) string { + r.t.Helper() + cmd := exec.Command("git", append([]string{"-C", r.dir}, args...)...) + cmd.Env = gittest.Env(r.t) + out, err := cmd.CombinedOutput() + if err != nil { + r.t.Fatalf("git %v: %v\n%s", args, err, out) + } + return strings.TrimSpace(string(out)) +} + +func (r *scanRepo) write(rel, body string) { + r.t.Helper() + p := filepath.Join(r.dir, filepath.FromSlash(rel)) + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + r.t.Fatal(err) + } + if err := os.WriteFile(p, []byte(body), 0o644); err != nil { + r.t.Fatal(err) + } +} + +// stub writes an executable named name that records its arguments and runs +// body. +func (r *scanRepo) stub(name, body string) { + r.t.Helper() + script := "#!/bin/sh\nprintf '%s\\n' \"$*\" >>\"" + filepath.Join(r.stubs, name+".args") + "\"\n" + body + "\n" + if err := os.WriteFile(filepath.Join(r.stubs, name), []byte(script), 0o755); err != nil { + r.t.Fatal(err) + } +} + +func (r *scanRepo) args(name string) string { + b, _ := os.ReadFile(filepath.Join(r.stubs, name+".args")) + return string(b) +} + +// run runs one mode of the script in the repository, with the stubs first on +// PATH and nothing else of the caller's but the system tools. +func (r *scanRepo) run(mode string) (int, string) { + r.t.Helper() + root, err := filepath.Abs(filepath.Join("..", "..", "..")) + if err != nil { + r.t.Fatal(err) + } + gitPath, err := exec.LookPath("git") + if err != nil { + r.t.Skip("git unavailable") + } + // git gets a directory of its own rather than its install directory on + // PATH: a package manager's bin (/opt/homebrew/bin) also holds gitleaks, + // which would turn the no-gitleaks case into a real scan. + gitDir := r.t.TempDir() + if err := os.WriteFile(filepath.Join(gitDir, "git"), []byte("#!/bin/sh\nexec '"+gitPath+"' \"$@\"\n"), 0o755); err != nil { + r.t.Fatal(err) + } + cmd := exec.Command("bash", filepath.Join(root, "scripts", "preflight-scans.sh"), mode) + cmd.Dir = r.dir + cmd.Env = []string{ + "PATH=" + r.stubs + ":" + gitDir + ":/usr/bin:/bin", + "HOME=" + r.t.TempDir(), "GIT_CONFIG_GLOBAL=/dev/null", "GIT_CONFIG_NOSYSTEM=1", + } + out, err := cmd.CombinedOutput() + if ee, ok := err.(*exec.ExitError); ok { + return ee.ExitCode(), string(out) + } else if err != nil { + r.t.Fatalf("preflight-scans.sh %s: %v\n%s", mode, err, out) + } + return 0, string(out) +} + +// govulncheckFinding is a finding as govulncheck prints one: exit 3, and a +// "Fixed in:" line per fix. +const govulncheckFinding = `echo 'Vulnerability #1: GO-2026-0001' +echo ' Standard library' +echo ' Found in: net/http@go1.26.9' +echo ' Fixed in: net/http@go1.26.10' +exit 3` + +func TestVulncheckRunsCIsVersionAndPassesAClearModule(t *testing.T) { + r := newScanRepo(t, false) + r.stub("go", "echo 'No vulnerabilities found.'; exit 0") + code, out := r.run("vulncheck") + if code != 0 { + t.Fatalf("a clear module: exit %d\n%s", code, out) + } + if got := r.args("go"); got != "run golang.org/x/vuln/cmd/govulncheck@v1.7.0 ./...\n" { + t.Fatalf("the scan ran %q, want CI's command", got) + } +} + +// TestVulncheckSaysAFindingLikelyStandsOnTheDefaultBranch: a branch that +// changes neither go.mod nor go.sum most likely did not bring the finding in, +// and govulncheck is not a required check, so the scan warns that the finding +// most likely stands on the default branch, names each fix, and does not stop +// a preflight no branch cut from there could pass. +func TestVulncheckSaysAFindingLikelyStandsOnTheDefaultBranch(t *testing.T) { + r := newScanRepo(t, false) + r.stub("go", govulncheckFinding) + code, out := r.run("vulncheck") + if code != 0 { + t.Fatalf("a finding the branch did not bring in: exit %d, want 0\n%s", code, out) + } + for _, want := range []string{"WARNING", "origin/main", "neither go.mod nor go.sum", "net/http@go1.26.10"} { + if !strings.Contains(out, want) { + t.Errorf("the refusal does not say %q:\n%s", want, out) + } + } +} + +func TestVulncheckOnABranchThatMovesADependencyBlamesNoOne(t *testing.T) { + r := newScanRepo(t, true) + r.stub("go", govulncheckFinding) + code, out := r.run("vulncheck") + if code != 1 { + t.Fatalf("a finding: exit %d, want 1\n%s", code, out) + } + if strings.Contains(out, "neither go.mod nor go.sum") { + t.Errorf("a branch that changes go.mod is told the finding is the default branch's:\n%s", out) + } + if !strings.Contains(out, "net/http@go1.26.10") { + t.Errorf("the refusal does not name the fix:\n%s", out) + } +} + +// TestVulncheckThatCannotRunSaysWhatWasNotChecked: offline, the scan cannot +// fetch the vulnerability database; that is said, loudly, and does not stop +// the preflight, because CI's scan still runs. +func TestVulncheckThatCannotRunSaysWhatWasNotChecked(t *testing.T) { + r := newScanRepo(t, false) + r.stub("go", "echo 'govulncheck: fetching vulnerabilities: dial tcp: lookup vuln.go.dev: no such host' >&2; exit 1") + code, out := r.run("vulncheck") + if code != 0 { + t.Fatalf("a scan that could not run: exit %d, want 0\n%s", code, out) + } + if !strings.Contains(out, "SKIPPED") || !strings.Contains(out, "not checked") { + t.Fatalf("the skip is not said loudly:\n%s", out) + } +} + +func TestSecretsCheckScansTheBranchsOwnCommits(t *testing.T) { + r := newScanRepo(t, false) + r.stub("gitleaks", `case "$1" in version) echo 8.24.3 ;; esac; exit 0`) + code, out := r.run("secrets") + if code != 0 { + t.Fatalf("a clean scan: exit %d\n%s", code, out) + } + base := r.git("merge-base", "HEAD", "refs/remotes/origin/main") + if !strings.Contains(r.args("gitleaks"), "git --redact --no-banner --exit-code 77 --log-opts="+base+"..HEAD .") { + t.Fatalf("gitleaks ran %q, want CI's scan over %s..HEAD", r.args("gitleaks"), base) + } + if strings.Contains(out, "not the") { + t.Errorf("CI's own version is warned about:\n%s", out) + } +} + +func TestSecretsCheckRefusesAFinding(t *testing.T) { + r := newScanRepo(t, false) + r.stub("gitleaks", `case "$1" in version) echo 8.24.3; exit 0 ;; esac; echo 'leaks found: 1'; exit 77`) + if code, out := r.run("secrets"); code != 1 { + t.Fatalf("a finding: exit %d, want 1\n%s", code, out) + } +} + +// TestSecretsCheckThatCannotScanIsNotCalledAFinding: gitleaks exits 1 on its +// own errors as on a leak, so an error must not read as a secret in the +// history; it stops the preflight as the fault CI's required job would hit. +func TestSecretsCheckThatCannotScanIsNotCalledAFinding(t *testing.T) { + r := newScanRepo(t, false) + r.stub("gitleaks", `case "$1" in version) echo 8.24.3; exit 0 ;; esac; echo 'error: unable to read config' >&2; exit 1`) + code, out := r.run("secrets") + if code != 2 { + t.Fatalf("a scan that could not run: exit %d, want 2\n%s", code, out) + } + if strings.Contains(out, "found a secret") || !strings.Contains(out, "could not scan") { + t.Errorf("an error is reported as a finding, or not said:\n%s", out) + } +} + +func TestSecretsCheckWarnsOnAnotherVersion(t *testing.T) { + r := newScanRepo(t, false) + r.stub("gitleaks", `case "$1" in version) echo 'version is set by build process' ;; esac; exit 0`) + code, out := r.run("secrets") + if code != 0 { + t.Fatalf("a clean scan: exit %d\n%s", code, out) + } + if !strings.Contains(out, "8.24.3") || !strings.Contains(out, "not the") { + t.Fatalf("another version is not warned about:\n%s", out) + } +} + +func TestSecretsCheckWithoutGitleaksSaysWhatWasNotChecked(t *testing.T) { + r := newScanRepo(t, false) + code, out := r.run("secrets") + if code != 0 { + t.Fatalf("no gitleaks: exit %d, want 0\n%s", code, out) + } + if !strings.Contains(out, "SKIPPED") || !strings.Contains(out, "not scanned") { + t.Fatalf("the skip is not said loudly:\n%s", out) + } +} diff --git a/internal/core/mdrecord/fence_shape_canonical_test.go b/internal/core/mdrecord/fence_shape_canonical_test.go index fae8b99779..11e6b95792 100644 --- a/internal/core/mdrecord/fence_shape_canonical_test.go +++ b/internal/core/mdrecord/fence_shape_canonical_test.go @@ -26,6 +26,7 @@ var fenceShapeReaders = map[string]fenceWriter{ "internal/core/positioning/check.go": {1, "emphasisRe strips inline emphasis and code markers from one tagline candidate, so a bolded tagline still reads as the tagline; it judges one string and tracks no lines"}, "internal/termsafe/prose.go": {2, "BlockText escapes a line-leading value's first byte when it would open a block (a backtick run, a tilde, a heading or list marker), so the rendered record keeps its text as prose; it judges one string and tracks no lines"}, "internal/core/guard/varpath.go": {1, "emptiedInside reads whether a default's text opens with a backquote, which opens a command substitution in the shell string that re-reads it: a shell grammar, not markdown; it judges one string and tracks no lines"}, + "internal/core/assistedby/body.go": {2, "fenceRun is the Go copy of scripts/check-attribution.sh's strip_fenced_blocks, the rule CI's attribution gate strips a pull-request body by, held to it by TestJudgeBodyMatchesTheGate: it must strip what that gate strips, which is not what Read reads (the gate keeps a body that opens an HTML block whole), so routing it through Read would pass bodies the gate refuses"}, "internal/core/lifeboat/mdrender.go": {2, "escapeLeadingMarker escapes a value's first byte when it opens a block, a backtick run termsafe judges unbalanced or a tilde among the other markers, so a rendered field stays prose; it judges one string and tracks no lines"}, } diff --git a/internal/core/rules/defaults/rules.json b/internal/core/rules/defaults/rules.json index b3a952a866..5497a89269 100644 --- a/internal/core/rules/defaults/rules.json +++ b/internal/core/rules/defaults/rules.json @@ -8,7 +8,7 @@ "aliases": ["pull request", "pr", "git add", "conventional commit"], "rules": [ "Conventional prefix (feat/fix/chore/refactor/docs/test, no scopes); small atomic commits, one purpose each.", - "AI-assisted commits carry an Assisted-by trailer (kernel format); never Co-Authored-By for AI.", + "AI-assisted commits carry an Assisted-by trailer (kernel format), and so does the body of a pull request with AI-assisted work, on a line of its own; never Co-Authored-By for AI.", "Commit as the human: an agent or autonomous run sets its git identity to the person responsible, never the tool's name or a bot address.", "Outward text (a pull-request body, an issue, a comment) carries no agent-session URL and no tool 'generated with' footer: after creating one, re-read what was posted and strip either shape the harness appended.", "Substantive work goes on a branch + PR; never force-push, never --no-verify.", diff --git a/internal/gittest/gh.go b/internal/gittest/gh.go new file mode 100644 index 0000000000..17789c5e3d --- /dev/null +++ b/internal/gittest/gh.go @@ -0,0 +1,33 @@ +package gittest + +import "os" + +// ghTokenVars are the variables gh takes a token from ahead of its own +// configuration. +var ghTokenVars = []string{"GH_TOKEN", "GITHUB_TOKEN", "GH_ENTERPRISE_TOKEN", "GITHUB_ENTERPRISE_TOKEN"} + +// HideGHLogin points gh, for the rest of the process, at an empty +// configuration directory and clears the token variables, so no test reaches +// the machine's own GitHub login. A test that did would pass on a developer's +// machine and fail on CI, which has no login: a landing test read the +// account behind an announcement that way and passed preflight, then failed +// in the merge queue. A package whose tests can reach gh calls it from +// TestMain before m.Run and runs the returned cleanup after; a test that +// wants a login of its own still sets one with t.Setenv. +func HideGHLogin() (cleanup func(), err error) { + dir, err := os.MkdirTemp("", "abcd-test-gh-config-") + if err != nil { + return nil, err + } + for _, k := range ghTokenVars { + if err := os.Unsetenv(k); err != nil { + _ = os.RemoveAll(dir) + return nil, err + } + } + if err := os.Setenv("GH_CONFIG_DIR", dir); err != nil { + _ = os.RemoveAll(dir) + return nil, err + } + return func() { _ = os.RemoveAll(dir) }, nil +} diff --git a/internal/gittest/gh_test.go b/internal/gittest/gh_test.go new file mode 100644 index 0000000000..1fe253687b --- /dev/null +++ b/internal/gittest/gh_test.go @@ -0,0 +1,41 @@ +package gittest + +import ( + "os" + "os/exec" + "testing" +) + +// After HideGHLogin no token variable is set, gh's configuration directory is +// an empty one of its own, and gh itself, where it is installed, reports no +// login: a machine's own login stays out of reach of the tests. +func TestHideGHLoginLeavesGHWithNoLogin(t *testing.T) { + t.Setenv("GH_CONFIG_DIR", t.TempDir()) + for _, k := range ghTokenVars { + t.Setenv(k, "a-machine-credential") + } + cleanup, err := HideGHLogin() + if err != nil { + t.Fatal(err) + } + dir := os.Getenv("GH_CONFIG_DIR") + for _, k := range ghTokenVars { + if v, ok := os.LookupEnv(k); ok { + t.Errorf("%s is still set (%q) after HideGHLogin", k, v) + } + } + entries, err := os.ReadDir(dir) + if err != nil || len(entries) != 0 { + t.Errorf("GH_CONFIG_DIR %s is not an empty directory: %v entries, %v", dir, len(entries), err) + } + if gh, err := exec.LookPath("gh"); err == nil { + out, err := exec.Command(gh, "auth", "status", "--hostname", "github.com").CombinedOutput() + if err == nil { + t.Errorf("gh reports a login after HideGHLogin:\n%s", out) + } + } + cleanup() + if _, err := os.Stat(dir); !os.IsNotExist(err) { + t.Errorf("cleanup left %s behind (%v)", dir, err) + } +} diff --git a/internal/surface/cli/cli.go b/internal/surface/cli/cli.go index bb7fa0f202..ad00213112 100644 --- a/internal/surface/cli/cli.go +++ b/internal/surface/cli/cli.go @@ -4340,7 +4340,7 @@ func newAhoyCommand(asJSON *bool) *cobra.Command { installCmd.Flags().BoolVar(&refuseAdopt, "refuse-adopt", false, "decline to adopt an unmanaged repo") installCmd.Flags().BoolVar(&dev, "dev", false, "track-latest dogfood mode: the PATH entry rebuilds from the source tip on every call instead of pinning the built binary") installCmd.Flags().BoolVar(&attribution, "attribution", false, "opt this repo into the committed prepare-commit-msg prompt asking every commit to declare whether a tool assisted it; the choice is recorded, so a later install without the flag keeps the hook") - installCmd.Flags().BoolVar(&outbound, "outbound", false, "opt this repo into both outbound checks, which refuse a live agent-session URL or a tool attribution footer: the committed commit-msg hook over every commit message, and the shell guard's check of the title, body and body file a posting gh pr or gh issue command hands the forge; it also sets this clone's git config --local abcd.binary to the abcd copy whose build provenance it verified and, where no core.hooksPath is set and no commit-msg hook is there, writes abcd's copy of that hook into the clone's git hooks directory, and says so when it cannot do either; the choice is recorded in .abcd/config.json as outbound.hook and outbound.forge, so a later install without the flag keeps both") + installCmd.Flags().BoolVar(&outbound, "outbound", false, "opt this repo into both outbound checks, which refuse a live agent-session URL or a tool attribution footer: the committed commit-msg hook over every commit message, and the shell guard's check of the title, body and body file a posting gh pr or gh issue command hands the forge, which also refuses a pull request body (gh pr create, new or edit) with no Assisted-by line in an accepted form, or one carrying a Co-authored-by line; it also sets this clone's git config --local abcd.binary to the abcd copy whose build provenance it verified and, where no core.hooksPath is set and no commit-msg hook is there, writes abcd's copy of that hook into the clone's git hooks directory, and says so when it cannot do either; the choice is recorded in .abcd/config.json as outbound.hook and outbound.forge, so a later install without the flag keeps both") installCmd.Flags().BoolVar(&allowStale, "allow-stale-binary", false, "proceed even when the running binary is stale against its source tip or its vintage cannot be determined; the default is to refuse before any write and name the rebuild fix") installCmd.Flags().StringVar(&binDir, "bin-dir", "", "directory for the PATH entry (default ~/.local/bin, or an existing abcd install adopted in place); fails when it is not writable — abcd never escalates privileges") installCmd.Flags().StringVar(&visibility, "visibility", "", "repo visibility: private | public") diff --git a/internal/surface/cli/githook_commitmsg_test.go b/internal/surface/cli/githook_commitmsg_test.go index f67d14bb09..40ff16e9d0 100644 --- a/internal/surface/cli/githook_commitmsg_test.go +++ b/internal/surface/cli/githook_commitmsg_test.go @@ -203,6 +203,105 @@ func TestCommitMsgHookJudgesTextBelowAScissorsLineGitKeeps(t *testing.T) { // git runs commit-msg for a merge that creates a commit, and git does NOT run // pre-commit for one: the merge message is one of the places a leaked URL lands. +// RS004's message half runs in the hook too (iss-2610241424265464): a message +// naming an issue id with no Refs: or Resolves: line declaring it is refused at +// the commit, not by lint-issues inside a preflight tens of minutes later. +func TestCommitMsgHookRefusesAnUndeclaredIssueMention(t *testing.T) { + c := newOwnRepoHookCase(t) + refused, out := c.commitWith("a.txt", "fix: the walk behind iss-12\n\nAssisted-by: None\n") + if !refused { + t.Fatalf("a commit message naming an undeclared issue id was committed\n%s", out) + } + if !strings.Contains(out, "commit-msg: BLOCKED") || !strings.Contains(out, "RS004") || !strings.Contains(out, "Refs: iss-12") { + t.Errorf("the refusal does not name the rule and the declaration that clears it\n%s", out) + } + if _, err := c.tryGit("rev-parse", "--verify", "HEAD"); err == nil { + t.Errorf("a commit exists after the refusal; the hook must fail before the commit is made") + } +} + +func TestCommitMsgHookPassesADeclaredIssueMention(t *testing.T) { + c := newOwnRepoHookCase(t) + if refused, out := c.commitWith("a.txt", "fix: the walk behind iss-12\n\nRefs: iss-12\nAssisted-by: None\n"); refused { + t.Fatalf("a commit message declaring its mention was refused\n%s", out) + } +} + +// TestCommitMsgHookLeavesCommentLinesOutOfTheMentionCheck: git's own status +// comments name the branch, and a branch is often named after the issue it +// works (`capture/iss-12`). An edited message carries those comments to the +// hook and git strips them after it, so judging them would refuse every +// edited commit on such a branch for a line no commit records. A comment line +// a -F message keeps is left to the range check in preflight and CI. +func TestCommitMsgHookLeavesCommentLinesOutOfTheMentionCheck(t *testing.T) { + c := newOwnRepoHookCase(t) + c.git("checkout", "-q", "-b", "capture/iss-12") + if refused, out := c.commitWith("a.txt", "fix: the walk\n\nAssisted-by: None\n", "-e"); refused { + t.Fatalf("an edited commit on a branch named after an issue was refused for git's own comment\n%s", out) + } +} + +// git's own message for a merge names the branch it merges, and a branch is +// often named after its issue (`Merge branch 'capture/iss-12'`). The range +// check skips merge commits (--no-merges), so the hook leaves a merge to it. +func TestCommitMsgHookLeavesAMergeMessageToTheRangeCheck(t *testing.T) { + c := newOwnRepoHookCase(t) + if refused, out := c.commitWith("a.txt", "chore: seed\n\nAssisted-by: None\n"); refused { + t.Fatalf("seed refused\n%s", out) + } + c.git("checkout", "-q", "-b", "capture/iss-12") + if refused, out := c.commitWith("b.txt", "fix: the walk behind iss-12\n\nRefs: iss-12\nAssisted-by: None\n"); refused { + t.Fatalf("side commit refused\n%s", out) + } + c.git("checkout", "-q", "main") + if out, err := c.tryGit("merge", "--no-ff", "--no-edit", "capture/iss-12"); err != nil { + t.Fatalf("git's own message for merging a branch named after its issue was refused\n%s", out) + } +} + +// RS004 is a rule abcd's own record adopted. Run by a global dispatcher for +// every repository on the machine, the hook holds another repository's +// message to the outbound policy alone. +func TestCommitMsgHookLeavesAnotherRepositorysIssueIdsAlone(t *testing.T) { + c := newCommitMsgHookCase(t) + if refused, out := c.commitWith("a.txt", "fix: the walk behind iss-12\n\nAssisted-by: None\n"); refused { + t.Fatalf("a commit in another repository was refused under abcd's issue rule\n%s", out) + } +} + +// newOwnRepoHookCase is a case whose repository is the hook's own, the shape +// of a checkout of abcd: it carries the hook, the issue-resolution gate and an +// abcd source, so the hook judges RS004 for its commits. The source is a stub +// whose abcd exits 0, so a case here is judged by RS004 alone. +func newOwnRepoHookCase(t *testing.T) *commitMsgHookCase { + t.Helper() + c := newCommitMsgHookCase(t) + hook, err := os.ReadFile(filepath.Join(c.hooks, "commit-msg")) + if err != nil { + t.Fatal(err) + } + gate, err := os.ReadFile(filepath.Join(c.root, "scripts", "check-issue-resolution.sh")) + if err != nil { + t.Fatal(err) + } + for rel, body := range map[string]string{ + "go.mod": "module example.com/stub\n\ngo 1.21\n", + "cmd/abcd/main.go": "package main\n\nfunc main() {}\n", + ".githooks/commit-msg": string(hook), + "scripts/check-issue-resolution.sh": string(gate), + } { + p := filepath.Join(c.dir, filepath.FromSlash(rel)) + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte(body), 0o755); err != nil { + t.Fatal(err) + } + } + c.git("config", "core.hooksPath", ".githooks") + return c +} + func TestCommitMsgHookRefusesASessionURLInAMergeMessage(t *testing.T) { c := newCommitMsgHookCase(t) if refused, out := c.commitWith("a.txt", "chore: seed\n\nAssisted-by: None\n"); refused { diff --git a/internal/surface/cli/land.go b/internal/surface/cli/land.go index 8ad71e3e02..d0191b6f14 100644 --- a/internal/surface/cli/land.go +++ b/internal/surface/cli/land.go @@ -68,8 +68,12 @@ func newLandCommand(asJSON *bool) *cobra.Command { " queue where the merge gate allows it (today: only where a merge queue gates the\n" + " default branch and a ruleset requires a person's approval), pinned to the\n" + " head, then confirmed in the queue. An arming someone else made is left as it\n" + - " is. With no pull request it prints the `gh pr create` command and stops; it\n" + - " never opens one.\n\n" + + " is. With no pull request it composes the body from the branch's commits (each\n" + + " subject, their Refs:, Resolves: and Delivers: lines, abcd's own Assisted-by label\n" + + " and the models their Assisted-by: lines name) into the\n" + + " checkout's .abcd/.work.local/ where it has one, judges it as the shell guard\n" + + " judges a posted body, prints the `gh pr create` command naming it with\n" + + " --body-file, and stops; it never opens one.\n\n" + "land.push and land.enqueue are read from .abcd/config.json (in the working\n" + "directory's checkout and the branch's own) and " + abcdhome.Display("config.json") + ",\n" + "default true, and off wins from any layer: any file can turn an act off and none\n" + diff --git a/internal/surface/cli/main_test.go b/internal/surface/cli/main_test.go index 9204995c52..25962a234e 100644 --- a/internal/surface/cli/main_test.go +++ b/internal/surface/cli/main_test.go @@ -80,6 +80,12 @@ func TestMain(m *testing.M) { os.Exit(code) } pageRunnerExtraEnv = []string{cliTestAsBinaryEnv + "=1"} + // No test here reaches the machine's own GitHub login: one that did would + // pass on a developer's machine and fail on CI, which has none. + showGHLogin, err := gittest.HideGHLogin() + if err != nil { + panic(err) + } // The machine's login shell never shapes a setup run here: with no shell // named, the shell-settings offer (itd-2610240919030303) is not raised, // and a test that wants it names one with t.Setenv. @@ -126,6 +132,7 @@ func TestMain(m *testing.M) { }) code := m.Run() _ = os.RemoveAll(emptyHome) + showGHLogin() builtAbcd.Remove() os.Exit(code) } diff --git a/internal/termsafe/codespan_canonical_test.go b/internal/termsafe/codespan_canonical_test.go index faa0405a9f..172dd7e890 100644 --- a/internal/termsafe/codespan_canonical_test.go +++ b/internal/termsafe/codespan_canonical_test.go @@ -30,6 +30,7 @@ type backtickScanner struct { // default for a file this test names is to pair through PairCodeSpan. var backtickScanners = map[string]backtickScanner{ "internal/adapter/scanner/identity.go": {2, "two delimiter sets: a backtick is one of the characters that may end an identity token, and one of those that bounds the path token an owner slug is judged in; nothing is paired"}, + "internal/core/assistedby/body.go": {2, "stripFencedBlocks is the Go copy of scripts/check-attribution.sh's strip_fenced_blocks, held to that gate by TestJudgeBodyMatchesTheGate: a backtick there is a fence opener's character, and an opener whose info string carries one is no fence, as the gate reads it; nothing is paired"}, "internal/core/capture/promote.go": {1, "a WRITER: codeSpan measures the longest backtick run to choose a fence the value cannot close; nothing is paired"}, "internal/core/guard/tokenize.go": {25, "the shell tokenizer: a backtick there is command substitution, a shell grammar, not markdown"}, "internal/core/guard/payload.go": {3, "targetsAnExpansion reads a shell line's raw text for an assignment target that holds an expansion: a backtick there opens or closes a command substitution, a shell grammar, not markdown; nothing is paired"}, diff --git a/scripts/check-issue-resolution-cases.sh b/scripts/check-issue-resolution-cases.sh index 470377e016..7fbf4300bb 100755 --- a/scripts/check-issue-resolution-cases.sh +++ b/scripts/check-issue-resolution-cases.sh @@ -1001,6 +1001,67 @@ printf '%s' "chore: tidy the parser" >"$d/title.txt" printf '%s\n' "No record is named here." >"$d/body.md" expect pass "$d" "RS004 a pull-request form naming no record" -- pr title.txt body.md +# RS004 over one commit message, the form the committed commit-msg hook runs so +# a bare mention is refused at the commit rather than at the preflight that +# lint-issues runs it in (iss-2610241424265464). It is the message half alone: +# no range and no ledger, so it needs nothing but the file. +d="$(newrepo rs004-message-bare)" +printf '%s\n' "fix: the parser hole behind iss-999" "" "Assisted-by: None" >"$d/msg.txt" +expect_refusal_naming "$d" "RS004 bare mention in a commit message" \ + "RS004.*iss-999.*Refs: iss-999" -- message msg.txt + +d="$(newrepo rs004-message-declared)" +printf '%s\n' "fix: the parser hole behind iss-999" "" "Refs: iss-999" >"$d/msg.txt" +expect pass "$d" "RS004 a commit message declaring its mention" -- message msg.txt + +d="$(newrepo rs004-message-list)" +printf '%s\n' "fix: iss-998 and iss-999" "" "Resolves: iss-998, iss-999" >"$d/msg.txt" +expect pass "$d" "RS004 a commit message declaring a list" -- message msg.txt + +d="$(newrepo rs004-message-mixed)" +printf '%s\n' "fix: iss-999 behind itd-146" "" "Refs: itd-146, iss-999" >"$d/msg.txt" +expect_refusal_naming "$d" "RS004 a mixed-family line in a commit message" \ + "RS004.*iss-999.*a Refs: line naming iss-999 beside an id of another family" -- message msg.txt + +d="$(newrepo rs004-message-crlf)" +printf 'fix: iss-999\r\n\r\nRefs: iss-999\r\n' >"$d/msg.txt" +expect pass "$d" "RS004 a CRLF commit message declaring its mention" -- message msg.txt + +d="$(newrepo rs004-message-missing)" +out="$(cd "$d" && bash "$GATE" message no-such.txt 2>&1)" && rc=0 || rc=$? +if [ "$rc" -ne 2 ]; then + printf 'cases: FAIL RS004 a missing message file — expected exit 2, got %d:\n%s\n' "$rc" "$out" >&2 + failures=$((failures + 1)) +else + printf 'cases: ok RS004 a missing message file (exit 2, as expected)\n' +fi + +# `-` reads the message on the standard input, the form the hook runs so that +# it writes no file before a shell runs the gate: refused and passed alike. +d="$(newrepo rs004-message-stdin)" +for c in "1|fix: the parser hole behind iss-999" "0|fix: the parser hole behind iss-999\n\nRefs: iss-999"; do + want="${c%%|*}" + out="$(cd "$d" && printf '%b\n' "${c#*|}" | bash "$GATE" message - 2>&1)" && rc=0 || rc=$? + if [ "$rc" -ne "$want" ]; then + printf 'cases: FAIL RS004 a commit message on standard input — expected exit %d, got %d:\n%s\n' "$want" "$rc" "$out" >&2 + failures=$((failures + 1)) + else + printf 'cases: ok RS004 a commit message on standard input (exit %d, as expected)\n' "$want" + fi +done + +# The message form never asks git about history, so a shallow clone, where the +# range forms refuse to run, judges a message all the same: the hook runs in +# whatever clone the commit is made in. +d="$(newrepo rs004-message-shallow-src)" +git -C "$d" commit -q --allow-empty -m "one" +git -C "$d" commit -q --allow-empty -m "two" +s="$tmproot/rs004-message-shallow" +git clone -q --depth 1 "file://$d" "$s" +printf '%s\n' "fix: the parser hole behind iss-999" >"$s/msg.txt" +expect_refusal_naming "$s" "RS004 a commit message in a shallow clone" \ + "RS004.*iss-999" -- message msg.txt + # --- RS005: a declared delivery must ship the intent -------------------------- # # The intent-store twin of RS001 (itd-2609111003026787). A change that delivers diff --git a/scripts/check-issue-resolution.sh b/scripts/check-issue-resolution.sh index 6ed7db1a1a..62beb23327 100755 --- a/scripts/check-issue-resolution.sh +++ b/scripts/check-issue-resolution.sh @@ -51,6 +51,14 @@ # honest, so a commit that merely touches an issue's ground is not # pushed into claiming a fix it did not make. # +# Its message half also runs alone, `message ` (`-` for the +# standard input), which the committed commit-msg hook calls so a +# bare mention is refused at the commit rather than by the preflight +# that runs this gate over the range tens of minutes later +# (iss-2610241424265464); and the shell guard holds a pull +# request's title and body to it before gh posts them +# (internal/core/issuemention, tied to this file by a test). +# # SCOPE. RS004 judges COMMIT MESSAGES and PULL-REQUEST TITLES/BODIES, # and nothing else. An id written in the BODY OF A RECORD is a # different surface with its own rule — `prose_citation_resolves`, in @@ -246,7 +254,7 @@ fail() { } usage() { - echo "usage: check-issue-resolution.sh commits | ledger [] | pr " >&2 + echo "usage: check-issue-resolution.sh commits | ledger [] | pr | message |-" >&2 exit 2 } @@ -844,6 +852,26 @@ $body")" echo "check-issue-resolution: RS004 checked the pull-request title and body" } +# check_message applies RS004 to one commit message, the form the commit-msg +# hook runs before the commit exists. It reads the file only, never history, so +# it runs in any clone, a shallow one included. A file of `-` is the standard +# input, which the hook hands it so that it writes no file before a shell runs +# this one. +check_message() { + local file="$1" msg + if [ "$file" = - ]; then + msg="$(tr -d '\r')" + else + [ -f "$file" ] || { + echo "check-issue-resolution: no such file: $file" >&2 + exit 2 + } + msg="$(tr -d '\r' <"$file")" + fi + check_mentions "the commit message" "$msg" "$(declared_ids "$msg")" + echo "check-issue-resolution: RS004 checked the commit message" +} + check_commits() { local base="$1" head="$2" local range @@ -1135,24 +1163,29 @@ check_ledger() { # code the contract reserves for it, never a violation. # The probe itself is rc-checked: comparing a failed substitution against # "true" would let the very fault this arm exists to report disarm it. -rc=0 -shallow="$(git rev-parse --is-shallow-repository 2>&1)" || rc=$? -if [ "$rc" -ne 0 ]; then - echo "check-issue-resolution: git rev-parse --is-shallow-repository failed (exit $rc) — refusing rather than reporting a vacuous pass:" >&2 - echo "$shallow" >&2 - exit 2 +# +# The message form reads one file and no history, so it is exempt: the +# commit-msg hook runs it in whatever clone the commit is made in. +if [ "${1:-}" != message ]; then + rc=0 + shallow="$(git rev-parse --is-shallow-repository 2>&1)" || rc=$? + if [ "$rc" -ne 0 ]; then + echo "check-issue-resolution: git rev-parse --is-shallow-repository failed (exit $rc) — refusing rather than reporting a vacuous pass:" >&2 + echo "$shallow" >&2 + exit 2 + fi + case "$shallow" in + true) + echo "check-issue-resolution: shallow checkout — RS002/RS003 cannot tell an absent commit from an unfetched one; run 'git fetch --unshallow' first (CI checks out with fetch-depth: 0)." >&2 + exit 2 + ;; + false) ;; + *) + echo "check-issue-resolution: unexpected git rev-parse --is-shallow-repository output \"$shallow\" — refusing rather than guessing." >&2 + exit 2 + ;; + esac fi -case "$shallow" in -true) - echo "check-issue-resolution: shallow checkout — RS002/RS003 cannot tell an absent commit from an unfetched one; run 'git fetch --unshallow' first (CI checks out with fetch-depth: 0)." >&2 - exit 2 - ;; -false) ;; -*) - echo "check-issue-resolution: unexpected git rev-parse --is-shallow-repository output \"$shallow\" — refusing rather than guessing." >&2 - exit 2 - ;; -esac case "${1:-}" in commits) @@ -1163,6 +1196,10 @@ ledger) [ $# -le 2 ] || usage check_ledger "${2:-HEAD}" ;; +message) + [ $# -eq 2 ] || usage + check_message "$2" + ;; pr) # RS004 over the artefact no commit message reaches: the title and body a # human types into the forge. Two FILES rather than two arguments, because diff --git a/scripts/gate-tools.tsv b/scripts/gate-tools.tsv index 9628f24c60..dd3fbbaf0a 100644 --- a/scripts/gate-tools.tsv +++ b/scripts/gate-tools.tsv @@ -17,10 +17,11 @@ # hand; that is the cost decision 5 accepted. # # go is also checked against the toolchain go.mod declares, which the setup -# resolves through scripts/pinned-toolchain.sh. -go preflight,fmt-check,lint-issues,record-lint,issue-drift,docs-lint,check-attribution,site-render,smoke,evals-cold-reading the setup's toolchain step (the toolchain go.mod declares) -git preflight,lint-reviews,lint-issues,record-lint,check-attribution,smoke the image -bash lint-reviews,lint-issues,check-attribution the image +# resolves through scripts/pinned-toolchain.sh. gitleaks is needed by no gate: +# secrets-check skips loudly without it, and CI's scan still runs. +go preflight,fmt-check,lint-issues,record-lint,issue-drift,docs-lint,check-attribution,vulncheck,site-render,smoke,evals-cold-reading the setup's toolchain step (the toolchain go.mod declares) +git preflight,lint-reviews,lint-issues,record-lint,check-attribution,secrets-check,vulncheck,smoke the image +bash lint-reviews,lint-issues,check-attribution,secrets-check,vulncheck the image make preflight the image python3 lint-issues the image gitleaks - the setup's gitleaks step (the version ci.yml pins) diff --git a/scripts/preflight-scans.sh b/scripts/preflight-scans.sh new file mode 100755 index 0000000000..2da263442b --- /dev/null +++ b/scripts/preflight-scans.sh @@ -0,0 +1,175 @@ +#!/usr/bin/env bash +# The two hosted scans CI runs on every pull request, run by `make preflight` +# before the push (iss-2610261151381604): govulncheck over the module and +# gitleaks over the commits the branch would land. Between 2026-10-18 and +# 2026-10-26 they failed 12 of 32 failed CI runs, each found only after a push +# and a hosted run; here a finding stops the preflight as a prerequisite, before +# its test stages wait at the machine's load gate. The one exception is a +# govulncheck finding on a branch that moves no dependency: govulncheck is not a +# required check, and such a finding most likely stands on the default branch, +# so stopping would hold every branch for a fix none of them can make. It warns. +# +# Each takes its version from .github/workflows/ci.yml, the one pin, so the scan +# run here is CI's: govulncheck from the `go run …/govulncheck@` line of +# the govulncheck job, gitleaks from GITLEAKS_VERSION, which +# scripts/cloud/connectors/claude/setup.sh installs from as well. +# +# A scan that cannot run here is SKIPPED LOUDLY, never passed in silence: no +# gitleaks on PATH, or a govulncheck that cannot reach its vulnerability +# database offline. Neither stops the preflight, since CI's scan still runs; the +# line says what was not checked. +# +# Usage: +# preflight-scans.sh vulncheck # govulncheck over ./... (exit 1 on a finding) +# preflight-scans.sh secrets # gitleaks over ..HEAD (exit 1 on a finding) +# preflight-scans.sh pins # print the two versions ci.yml pins +# Exit 0 clean, skipped or warned, 1 a finding, 2 a usage or environment fault +# (a gitleaks that ran and could not scan among them: CI's would fail too). +set -euo pipefail +export LC_ALL=C + +CI_FILE=".github/workflows/ci.yml" + +say() { printf 'preflight-scans: %s\n' "$*"; } + +usage() { + echo "usage: preflight-scans.sh vulncheck | secrets | pins" >&2 + exit 2 +} + +# one_pin prints the one value its sed program takes out of ci.yml, or refuses: +# none, or two that differ, leaves no version to match CI with. +one_pin() { + local what="$1" prog="$2" pins + [ -f "$CI_FILE" ] || { + echo "preflight-scans: no $CI_FILE to read the $what version from" >&2 + exit 2 + } + pins="$(sed -n "$prog" "$CI_FILE" | sort -u)" + if [ -z "$pins" ] || [ "$(printf '%s\n' "$pins" | grep -c .)" -ne 1 ]; then + echo "preflight-scans: $CI_FILE does not pin exactly one $what version, so there is no version to match CI's scan with" >&2 + exit 2 + fi + printf '%s' "$pins" +} + +vuln_pin() { + one_pin govulncheck 's/.*golang\.org\/x\/vuln\/cmd\/govulncheck@\(v[0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*\).*/\1/p' +} + +leaks_pin() { + one_pin gitleaks 's/^[[:space:]]*GITLEAKS_VERSION:[[:space:]]*\([0-9][0-9.]*\)[[:space:]]*$/\1/p' +} + +# default_ref is origin's default branch as this clone last fetched it: +# origin/HEAD when it is set, origin/main otherwise, or nothing. +default_ref() { + local ref + ref="$(git symbolic-ref -q --short refs/remotes/origin/HEAD 2>/dev/null || true)" + [ -n "$ref" ] || ref="origin/main" + if git rev-parse -q --verify "$ref^{commit}" >/dev/null 2>&1; then + printf '%s' "$ref" + fi +} + +vulncheck() { + local version rc=0 out def base fixes + version="$(vuln_pin)" + out="$(go run "golang.org/x/vuln/cmd/govulncheck@$version" ./... 2>&1)" || rc=$? + case "$rc" in + 0) + say "govulncheck $version: no known vulnerability the module's code reaches" + return 0 + ;; + 3) ;; + *) + printf '%s\n' "$out" | tail -n 5 | sed 's/^/ /' + say "SKIPPED — govulncheck $version could not run (exit $rc), so the module was not checked for known vulnerabilities." + say " The lines above say why: offline it cannot fetch its vulnerability database, and a module it cannot load stops it too." + say " CI's govulncheck job still runs." + return 0 + ;; + esac + printf '%s\n' "$out" + fixes="$(printf '%s\n' "$out" | sed -n 's/^[[:space:]]*Fixed in:[[:space:]]*//p' | sort -u)" + # A branch that moves no dependency most likely did not bring the finding + # in: a newly published vulnerability, or the toolchain go.mod declares, + # fails the default branch too, and every branch cut from it + # (iss-2610261151481013). The module was already required there, though + # this branch's own code may be what newly reaches it. + def="$(default_ref)" + if [ -n "$def" ] && base="$(git merge-base HEAD "$def" 2>/dev/null)" && + git diff --quiet "$base" HEAD -- go.mod go.sum; then + say "WARNING — govulncheck $version found a known vulnerability the module's code reaches, and this branch changes neither go.mod nor go.sum against $def," + say " so the vulnerable module was already required there and the finding most likely stands on $def itself. It does not stop the preflight:" + say " govulncheck is not a required check, and no branch cut from $def can fix it alone. Fix it on a branch of its own (the toolchain" + say " line in go.mod, or the module go.sum names); if this branch's own code is what reaches it, CI's govulncheck job shows it on the pull request." + if [ -n "$fixes" ]; then + say "The fixes govulncheck names:" + printf '%s\n' "$fixes" | sed 's/^/ /' + fi + return 0 + fi + say "govulncheck $version found a known vulnerability the module's code reaches; the preflight stops before its test stages." + if [ -n "$fixes" ]; then + say "The fixes govulncheck names:" + printf '%s\n' "$fixes" | sed 's/^/ /' + fi + return 1 +} + +secrets() { + local version raw have def base rc=0 out + version="$(leaks_pin)" + if ! command -v gitleaks >/dev/null 2>&1; then + say "SKIPPED — gitleaks is not on PATH, so the commits this branch would land were not scanned for secrets." + say " CI's gitleaks job (gitleaks $version) still scans them. Install gitleaks $version to scan here." + return 0 + fi + raw="$(gitleaks version 2>/dev/null | head -n 1 || true)" + have="$(printf '%s' "$raw" | tr -d '[:space:]')" + have="${have#v}" + if [ "$have" != "$version" ]; then + say "WARNING — gitleaks reports version '${raw:-unknown}', not the $version $CI_FILE pins; its rules may differ from CI's, so a clean scan here is not CI's verdict." + fi + def="$(default_ref)" + if [ -z "$def" ] || ! base="$(git merge-base HEAD "$def" 2>/dev/null)"; then + say "WARNING — no origin default branch to scope the scan to; scanning the full history reachable from HEAD instead." + base="" + fi + if [ -n "$base" ] && [ "$base" = "$(git rev-parse HEAD)" ]; then + say "gitleaks: this branch adds no commit to $def; nothing to scan" + return 0 + fi + local scope="${base:+$base..}HEAD" + say "gitleaks: scanning $scope" + # gitleaks exits 1 on a leak and on its own errors alike, so a leak is given + # a code of its own. + out="$(gitleaks git --redact --no-banner --exit-code 77 --log-opts="$scope" . 2>&1)" || rc=$? + printf '%s\n' "$out" + case "$rc" in + 0) return 0 ;; + 77) + say "gitleaks found a secret in $scope; the preflight stops. Remove it from the history (rewrite the branch), and rotate it if it was ever pushed." + return 1 + ;; + *) + say "gitleaks could not scan $scope (exit $rc); the lines above say why. CI's gitleaks job, a required check, would most likely fail the same way, so the preflight stops." + exit 2 + ;; + esac +} + +[ $# -eq 1 ] || usage +case "$1" in +vulncheck) vulncheck ;; +secrets) secrets ;; +pins) + # Assigned first: a failed substitution inside printf's arguments would + # not stop the script, and an empty pin would print as a version. + vuln="$(vuln_pin)" + leaks="$(leaks_pin)" + printf 'govulncheck %s\ngitleaks %s\n' "$vuln" "$leaks" + ;; +*) usage ;; +esac