docs: plan signed branch updates for signature-enforcing repos - #19
Closed
robinbowes wants to merge 8 commits into
Closed
robinbowes wants to merge 8 commits into
robinbowes wants to merge 8 commits into
Conversation
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>
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds
docs/plans/signed-branch-updates/README.md.Every commit Last Light creates with
gitin a sandbox is unsigned, and 22 of the yo61 repos enforcerequired_signaturesonmain. 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#185sat atmergeStateStatus: BLOCKEDwith all eight required checks green,mergeable: MERGEABLEand zero required approvals. The sole blocker was merge commit747b1c08: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 24hminimumReleaseAgewindow.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
gpgsignsettings in this repo aretag.gpgsignfor releases.The merge itself (
workflows/prompts/dependabot-ci-fix.md:71) is correct and deliberate — it makes abehindPR 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-branchinstead of a sandboxgit mergewhenever the goal is only "make this PR current". GitHub merges server-side, so the commit is committed byGitHub <noreply@github.com>and web-flow signed. No key management.POST .../actions/runs/{run_id}/rerun-failed-jobswhere the goal is only to re-run CI after a transient failure — creates no commit at all. That was the actual need ongo-udap#185.update-branchcan't resolve conflicts, sodirtyPRs still need a sandbox commit, still unsigned, still unlandable on those 22 repos.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/maincalls. Do not infer it fromgithub-reposTerraform data —data/yo61/go-udap.yamlreadsrequired_signatures: false, which is the setting for that repo's ownadditional_rulesetsblock; the actual rule arrives viabuiltin_ruleset_names: [default_branch]. The config file states the opposite of the effective policy.🤖 Generated with Claude Code