Skip to content

docs: plan signed branch updates for signature-enforcing repos - #19

Closed
robinbowes wants to merge 8 commits into
mainfrom
docs/signed-branch-updates
Closed

robinbowes wants to merge 8 commits into
mainfrom
docs/signed-branch-updates

Conversation

@robinbowes

Copy link
Copy Markdown

Adds docs/plans/signed-branch-updates/README.md.

Every commit Last Light creates with git in a sandbox is unsigned, and 22 of the yo61 repos enforce required_signatures on main. On those repos a fix run that merges the base branch does not unblock the PR — it blocks it permanently, because the rule is evaluated over every commit reachable in the PR, not just the head.

The case

yo61/go-udap#185 sat at mergeStateStatus: BLOCKED with all eight required checks green, mergeable: MERGEABLE and zero required approvals. The sole blocker was merge commit 747b1c08:

verification: { verified: false, reason: "unsigned" }
author/committer: yo61-lastlight[bot] <yo61-lastlight[bot]@users.noreply.github.com>

Dependabot's own commit on the same branch was verified: true — anything pushed through GitHub's API is web-flow signed automatically. Enabling auto-merge (which we did, at 15:04:38Z) could never have fired.

Recovery required @dependabot recreate, which discarded the branch, threw away a green CI run, and produced a red one — the regenerated lockfile pulled in transitive deps inside pnpm's 24h minimumReleaseAge window.

Why the App token doesn't help

The token authenticates the push; a signing key signs the commit object. A sandbox has the first and not the second, and no amount of write permission changes that. Last Light configures signing nowhere — the only gpgsign settings in this repo are tag.gpgsign for releases.

The merge itself (workflows/prompts/dependabot-ci-fix.md:71) is correct and deliberate — it makes a behind PR current and regenerates lockfiles on conflict. The defect is the mechanism, not the intent.

What the plan proposes

  • PUT /repos/{owner}/{repo}/pulls/{number}/update-branch instead of a sandbox git merge whenever the goal is only "make this PR current". GitHub merges server-side, so the commit is committed by GitHub <noreply@github.com> and web-flow signed. No key management.
  • POST .../actions/runs/{run_id}/rerun-failed-jobs where the goal is only to re-run CI after a transient failure — creates no commit at all. That was the actual need on go-udap#185.
  • Documents the gap neither closes: update-branch can't resolve conflicts, so dirty PRs still need a sandbox commit, still unsigned, still unlandable on those 22 repos.
  • Leaves "should Last Light hold a signing key?" as an explicit open question rather than assuming it.

Filed as a new plan directory rather than a phase of stuck-pr-recovery, since that plan's phases are all complete with execution notes dated 2 Aug. It is the mirror image of that plan: there, recovery made stuck PRs survivable; here, the recovery mechanism is itself what gets the PR stuck.

Note on measuring blast radius

The 22-repo figure is from live GET /repos/{owner}/{repo}/rules/branches/main calls. Do not infer it from github-repos Terraform data — data/yo61/go-udap.yaml reads required_signatures: false, which is the setting for that repo's own additional_rulesets block; the actual rule arrives via builtin_ruleset_names: [default_branch]. The config file states the opposite of the effective policy.

🤖 Generated with Claude Code

cliftonc and others added 7 commits August 2, 2026 17:46
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The dependency-PR-resilience and stuck-PR-recovery work replaced state read
from six sites over seven stores with one resolved `PrState` snapshot per
dispatch plus pure decisions over it. The spec documents that well; the docs
site did not. The four workflow pages each restated a fragment — `requires-human`
five times across two pages, the run lock three times, the dispatch gate twice —
none of them named the snapshot, showed the ordering of the guards, or drew
anything, and nowhere did the site say why a team should care.

New `/docs/pr-state` page in a Concepts nav section above Workflows. It leads
with the team problem rather than the refactor: a pull request is the unit of a
release workflow, several actors work it at once, and without one agreed answer
you get duplicate pushes, repeated escalation comments, reviews of a tree being
rewritten, and PRs that stop silently. Then the mechanism, anchored on a real
run's PR-state panel.

Five hand-authored SVG diagrams (no mermaid on this site, and docs pages are
.astro rather than markdown): the choke point, the ten-guard ladder colour-coded
by exit kind, the fix attempt machine, the review resolver's three verdicts and
the check each leaves, and the dependency lane with both daily crons. The ladder
is generated from a frontmatter array so it cannot drift out of order.
`generate-md.mjs` strips `svg` from the .md mirrors, so every diagram has a prose
or table equivalent beside it — the SVG is never the sole carrier of a fact. The
diagrams hold a `min-width` floor so a phone scrolls the card instead of scaling
8px labels into illegibility.

The four workflow pages lose the duplicated prose and link here instead. Their
"the four PR-scoped workflows" wording is now false — the set is derived from
each workflow's own `pr_scoped: true` key, so an overlay fork keeps the gate —
and is softened to name the packaged four as an example.

Also fixes a pre-existing break in the prev/next chain: Configuration's
"Previous" pointed at Slack integration, skipping the whole Workflows section.

Docs-only. No `apps/server/spec` changes — the spec is already current here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A GitHub App is installed per ACCOUNT, and each installation mints its own
tokens. The harness threaded one statically-configured
`GITHUB_APP_INSTALLATION_ID` into every mint and every App-authed Octokit, so
everything against a second account failed: token mints 422'd ("at least one
repository ... is not accessible to the parent installation"), and every
harness-side call — comments, reactions, check runs, `.lastlight/` fetches,
post-review — 404'd.

Installations are now DISCOVERED and resolved per repository owner.

- `InstallationDirectory` (engine/github/installations.ts) is the single
  owner→installation authority. Fed by every webhook's `payload.installation`
  and by `GET /app/installations` under an App JWT; concurrent misses share one
  request and negatives are cached briefly, so a cron fan-out over N repos costs
  one call, not N. Suspended installations are withheld from resolution (they
  403 every mint) but stay listed, flagged, for the admin surface.

- `GitHubClient` resolves its Octokit per owner, memoized per installation.
  Every method already took `owner` first, so ~50 call sites — `DispatchDeps`,
  the router, dispatcher, pr-state, review-check, repo-config — are unchanged.
  The read-only chat tools get the same treatment; the two `search` tools read
  the account from the query's `repo:`/`org:`/`user:` qualifier.

- The per-run mint resolves from `githubAccess.owner`. An owner with no usable
  installation fails the phase immediately, naming the ACCOUNT — no sandbox, no
  API call.

- Installation repo discovery is keyed by installation id. Those webhooks are
  per-account: against one flat set a second org's `created` reset the managed
  list to just that org and its `deleted` cleared it entirely. `suspend` /
  `unsuspend` are handled too — previously they fell through silently.

- `GITHUB_APP_INSTALLATION_ID` is now OPTIONAL, kept only as the fallback for
  when the JWT lookup itself fails, so an existing single-installation
  deployment keeps its old behaviour with no .env edit.

- `GET /admin/api/managed-repos` reports every installation (account, id, repo
  count, selection, suspended) plus `uninstalledOwners`; Config → Managed repos
  renders them and warns when a managedRepos owner has no installation — the
  condition is visible before it becomes a failed run.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The installations pane named an account and an id and left you to build the
URL by hand — which is the one thing you actually want to do next from every
question it raises (change the repo grant, un-suspend, uninstall).

The path shape depends on the account type and guessing wrong 404s: an org
install lives at `/organizations/<login>/settings/installations/<id>`, a
personal one at a viewer-scoped `/settings/installations/<id>`. So
`installationSettingsUrl()` builds it server-side from the account type, and
returns undefined when that type isn't known yet — a record seeded from a
webhook that carried no `account.type` — so the pane renders plain text rather
than a link that may not resolve. `note()` now takes `account.type` from the
payload and backfills it, so a webhook-learned install is linkable too.

The `uninstalledOwners` warning also gets `appInstallUrl` (derived from
`botName`, which IS the App slug), so it offers the fix instead of only naming
the problem.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every commit built with git in a sandbox is unsigned, and 22 yo61 repos
enforce required_signatures on main. On those repos a fix run that
merges the base does not unblock the PR, it blocks it permanently:
the rule is evaluated over every commit reachable in the PR.

go-udap#185 sat BLOCKED with all eight required checks green and zero
required approvals, solely because of merge commit 747b1c08. Recovery
needed @dependabot recreate, which discarded a green CI run.

Records update-branch (server-side, web-flow signed) as the fix for the
make-this-PR-current case, rerun-failed-jobs where no merge is needed,
and the dirty-PR gap neither one closes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The plan recorded update-branch's signing behaviour as documented
generally but unconfirmed for this endpoint. Running it against
go-udap#189 and nearform#190 produced merge commits committed by
GitHub <noreply@github.com> with verified=true, reason=valid.

Also notes the message difference — update-branch writes "Merge branch
'main' into X", a sandbox merge writes "Merge remote-tracking branch
'origin/main' into X" — so the two are distinguishable in history.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@robinbowes

Copy link
Copy Markdown
Author

Superseded by nearform#268. yo61/lastlight is a fork, so committing a new plan directory here would diverge it from upstream and risk riding along in any future upstream PR. Upstream's own pattern is plans in docs/plans/ with discrete actionable work filed on its tracker, so the issue is the right home.

@robinbowes robinbowes closed this Aug 4, 2026
@robinbowes
robinbowes deleted the docs/signed-branch-updates branch August 4, 2026 17:08
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.

2 participants