Skip to content

US-26.4.6: story verification reads CI from the intake source, per-tenant credential (fixes #913) - #933

Merged
mkreyman merged 6 commits into
masterfrom
feature/us-26.4.6-verification-ci
Sep 28, 2026
Merged

mkreyman merged 6 commits into
masterfrom
feature/us-26.4.6-verification-ci

Conversation

@mkreyman

@mkreyman mkreyman commented Sep 28, 2026 •

Copy link
Copy Markdown
Owner

Fixes #913. Implements story US-26.4.6 (docs/user_stories/epic_26_chain_of_custody_v2/us_26.4.6.json, #916). Supersedes #914, #915 and #931.

Approved by Mark on 2026-09-28 after round 3, together with one extra review round. That round ran, and its findings are fixed in the last commit.

Problem:

  • Story verification read CI through GET /commits/:sha/check-runs. That API needs checks: read, which a fine-grained token can't have.
  • It also read the tenant-controlled projects.repo_url with the operator token, which is a cross-tenant oracle.
  • On master, an uncast schemaless query crashes every started run before it reaches the CI read. That crash is the only reason neither problem is live today.

What verification does now:

  • Repository and branch: the repository comes only from the project's intake source. The branch is the story's own: the thread branch, or the placed pr branch.

  • Credential: the operator's GITHUB_TOKEN is used only for (tenant, repository) pairs listed in the new VERIFICATION_OPERATOR_TOKEN_TENANTS, which is empty by default. The checks run in this order:

    • a tenant with no entry records credential_unavailable before anything else is read;
    • a pr source that names no required checks records no_required_checks;
    • the pair is checked before any GitHub read.

    Per-tenant GitHub credentials are Per-tenant GitHub credentials for forge reads (merge gate + verification) #936.

  • Change check, once per run:

    • a branch that changes its own CI definition is refused ci_definition_changed;
    • an empty change is refused empty_change: an empty three-dot diff, or a head tree equal to the base tree.

    Both are the merge gate's own rules, shared through CiDefinition and the new EmptyChange. The check's pass is stamped on the run (change_checked_at), so a merge during the CI wait can't later refuse a checked commit.

  • Verdict: GitHubPullRequestSource.check_evidence/3, judged by CiEvidence.judge/2 over the source's required_checks.

  • Bounded waits:

    • Pending CI waits until the run's age limit, then records ci_wait_exhausted.
    • Every poll that ends in a transient forge fault counts toward the merge gate's consecutive bound, then records forge_unavailable. Only a CI answer resets the count.
    • Snoozes back off 60/120/240/480/900 s.
    • Database contention on any read or write snoozes as database_busy. It never touches the fault count and leaves the run unchanged.
  • Abbreviated SHA: resolved once, then stored on the run. GitHub's 422 is recorded as unresolved_sha; its 404 as repository_unreadable.

  • Recorded results: ac_results hold fixed codes only, with no GitHub-echoed URLs and no exception text.

  • No local fallback: TestRunner and :enable_local_test_runner are deleted. The runner had no environment it could safely run in: it would execute tenant code with loopctl's environment and filesystem. AC-26.4.6.6 says what brings it back.

  • Crash fixed: the uncast story query that crashed every started run is gone.

Shared with the merge gate: CiDefinition, EmptyChange, and in CiEvidence the rule that only a jobless run that concluded as a failure counts against a check.

Migrations: 20260928120000 adds resolved_commit_sha and ci_forge_faults, and 20260928150000 adds change_checked_at, both to verification_runs. Both are additive and nullable.

Review history: four rounds, the fourth approved by Mark.

  • Round 1:
    • scrubbed the fallback's environment (the fallback was later removed);
    • moved the allowlist to pairs;
    • corrected the resolve codes and the jobless-run rule.
  • Round 2:
    • removed the local fallback;
    • fixed the fault streak;
    • refused on-base commits.
  • Round 3: tried binding verification to the commit the merge gate allowed. That rested on the wrong order. Verification runs before the merge, because the gate allows nothing until the story is verified.
  • Round 4:
    • removed that binding;
    • moved to the shared empty-change rule;
    • made every worker write a bounded wait.

Evidence:

  • Mutations (mutate.sh), each exit 0, across the rounds. The final set:
    • change check: stamp, once-skip, shared tree rule and empty diff through all three callers, tree wiring, unreadable arm, gate wiring;
    • credentials: pair order, allowlist, pair wiring;
    • verdict and waits: CI definition, no-verdict arm, fault streak, resolve once, MCP doc order;
    • bounded writes: one per wrapped write, plus busy class, lock bound and rollback.
  • Gate: 12089 tests, 0 failures. MCP: 812 pass.

…nant credential (fixes #913)

Verification's CI read used the check-runs API, which needs checks: read, a permission a
fine-grained token cannot have, and read the tenant-controlled projects.repo_url with the
operator token. It now reads the way the merge gate does.

- The repository comes only from the project's intake source, never repo_url; the branch is
  the story's own (thread branch, or the placed pr branch).
- Evidence is GitHubPullRequestSource.check_evidence/3 judged by CiEvidence.judge/2 over the
  source's required_checks; none configured records no_required_checks.
- A change to the CI definition on the branch refuses (CiDefinition, now shared with the
  merge gate), and a refusal never falls back to the local runner.
- Waits are declared outcomes of the CI behaviour: pending waits to the run's age limit
  (ci_wait_exhausted), transient faults are counted on the run and bounded
  (forge_unavailable). An abbreviated sha is resolved once and persisted.
- Credential seam: the operator token is lent only to tenants listed in
  VERIFICATION_OPERATOR_TOKEN_TENANTS (default empty); others record
  credential_unavailable. The clone's token travels in the git environment, scoped to
  github.com, redirects off.
- ac_results carry fixed codes only: no GitHub-echoed URLs and no exception text.
- The uncast schemaless story query that crashed every started run is gone.
- The merge_precondition MCP README row no longer says CI is read from check-runs.

Mutations (bin/mutate.sh): 45 in the PR body, all exit 0, plus three of mine on the
credential: allowlist bypassed, UUID filter removed, token Inspect exclusion removed, all
exit 0.
The intake source tools describe required_checks as what story verification judges by,
and the merge_precondition README row no longer names check-runs. Published files
changed, so the package version moves.
…enant+repository pairs

- Local fallback: every command runs with an env that unsets everything outside a short
  allowlist (PATH, HOME, locale, TMPDIR, MIX_HOME, HEX_HOME, asdf), so GITHUB_TOKEN,
  DATABASE_URL and the Cloak keys never reach tenant code; the credential's git config goes
  to git clone and fetch only. Each step runs under coreutils timeout with its own budget,
  exit 124/137 records local_timeout, the worker bounds the whole fallback in a task, and
  Oban timeout/1 sits above both.
- VERIFICATION_OPERATOR_TOKEN_TENANTS entries are tenant_uuid:owner/repo pairs;
  Credential.for_read/2 is asked for the intake source's repository, after CiTarget.
- Transient forge faults back off 60/120/240/480/900s (a forge retry-after still wins, to
  an hour); any answered read resets the count; database contention is its own wait and
  never counts as a forge fault.
- Resolve codes follow GitHub as measured: 422 is unresolved_sha, 404 is
  repository_unreadable.
- CiEvidence: a jobless run that concluded success, skipped or neutral is not a failure,
  and the failing URL comes from the run that failed the check. Shared with the merge gate.
- A commit already on the base is judged, not refused: its CI definition is the base's own.
- TestRunner.run_tests/2, which built a credential outside the seam, is deleted.
- The integration test deletes only the tenants it created.

Mutations (bin/mutate.sh): the fixer's 34, all exit 0, plus mine: repo half of the
allowlist ignored, exit 0.
…ng poll, refuse on-base commits

- Verification no longer falls back to running a tenant's tests. The runner is disabled
  everywhere and the production image has no git or mix to run it, yet the path carried
  the environment leak, the untidied clones, the Lifeline re-run and the timeout surface.
  TestRunner is back to master byte for byte, LocalRunner and Credential.git_env/1 are
  deleted, and a final no-verdict records its code and ends. AC-26.4.6.6 says why and what
  brings it back: an isolated, egress-restricted runner.
- Every poll that ends in a transient fault counts, whichever read faulted; only a CI
  answer resets the streak, so a persistent evidence-read fault reaches forge_unavailable.
- A commit already on the base is refused as commit_on_base: its three-dot diff is empty
  whatever it holds, so an implementer could point the branch at an old green master
  commit. Verify before merge.
- A tenant with no allowlist entry records credential_unavailable before any CiTarget
  database read; the (tenant, repository) pair is still checked once the repository is
  known.
- FLY_SECRETS no longer describes an upgrade from a format that never shipped.

Mutations (bin/mutate.sh, all exit 0): the fixer's M1-M10 and the re-run R1-R7
(allowlist, repository half, UUID filter, pair wiring, ci_definition, bound, no-verdict arm).
… refuse empty changes

- A story whose stage row carries merge_gate_allowed_sha is verified on that commit only;
  any other records commit_not_merge_gated before CI is read. In thread mode the gate
  already refused CI-definition changes and empty changes, so the commit is judged whether
  or not it has merged. In pr mode the gate checks neither, so verification still runs its
  own change check.
- The change check (CI-definition refusal plus empty_change, the merge gate's code for an
  empty three-dot diff) runs once per run and is stamped as ci_definition_checked_at, so a
  merge during the CI wait cannot turn a checked commit into a refused one. commit_on_base
  is gone: an empty diff covers the old-base commit and the empty commit alike.
- The stage row is read through a bounded Stages.fetch/2; contention is a wait.
- Docs say the allowlist comes before no_required_checks (controller, source, ACs, both MCP
  tool descriptions and README rows; MCP server 2.111.2).
- TestRunner and :enable_local_test_runner are deleted; the SHA validator stays, guarding
  a caller-supplied value into GitHub URLs.
- The Credential moduledoc says the seam licenses the read and carries no token; a
  per-tenant token is #936.

Mutations (bin/mutate.sh, all exit 0): the fixer's set (binding, thread skip, pr compare,
checked-once skip, stamp, stage wiring, busy wait, empty-diff refusal, re-runs) plus mine:
the commit_not_merge_gated refusal replaced by a pass.
…ange rule, bound every write

- The binding to merge_gate_allowed_sha is removed. It assumed story verification runs
  after the merge; it runs before it (custody verify enqueues the run, and the merge gate
  allows nothing until the story is verified), so the binding only added a race: an allow
  landing mid-wait for another head ended a checked run. The worker moduledoc and the story
  say verification runs before the merge.
- The change check runs once per run and is stamped as change_checked_at (renamed from
  ci_definition_checked_at), added by its own migration 20260928150000 instead of an edit to
  20260928120000.
- Loopctl.Delivery.EmptyChange is the merge gate's empty-change rule, moved out and called
  by both: an empty three-dot diff, or a head tree equal to the base tree. The adapter reads
  the commit's tree for it; an unreadable one is forge_unreadable, never a pass.
- Every write the worker makes to its own run goes through Verification's bounded write: a
  lock timeout, rollback on refusal, and contention answered as {:error, :busy}, which the
  worker turns into a database_busy snooze that leaves the run unchanged and the fault
  streak alone. complete_run writes its disposition in one statement.
- The allowlist docs state the order the code checks: an entry for the tenant first, then
  no_required_checks for a pr source that names none, then the tenant+repository pair before
  any GitHub read (controller, source, story, both MCP descriptions and README rows).

Mutations (bin/mutate.sh, all exit 0): the change-check set (stamp wait, stamp, once-skip,
shared tree rule and empty diff through all three callers, tree wiring, commit read,
unreadable arm, gate wiring, pair order, allowlist, pair wiring, CI definition, no-verdict
arm, fault streak, resolve once, MCP doc order) and the bounded-write set (one per wrapped
write, plus busy class, settled snooze, lock bound, busy wiring, rollback arm).
@mkreyman
mkreyman enabled auto-merge (squash) September 28, 2026 21:53
@mkreyman
mkreyman merged commit 42185b2 into master Sep 28, 2026
17 checks passed
@mkreyman
mkreyman deleted the feature/us-26.4.6-verification-ci branch September 28, 2026 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Story verification CI lookup 403s on private repos: check-runs needs checks:read, which fine-grained PATs cannot have

1 participant