docs(adr): adopt the standard that only a reviewed, checked pull request reaches main #18
Labels
No labels
needs-info
needs-triage
ready-for-agent
ready-for-human
wontfix
needs-info
needs-triage
ready-for-agent
ready-for-human
review/merge-ready
review/needs-fix
review/needs-human
review/needs-review
wayfinder:grilling
wayfinder:map
wayfinder:prototype
wayfinder:research
wayfinder:task
wontfix
No milestone
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
olympus/infra-tracker!18
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "adr/0003-protected-main"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Records the
mainstandard as an ADR. Docs-only: one new file, no code.Stated technology-agnostically — it names no product and no configuration key — and written in Michael Nygard's template (
# Title,## Status,## Context,## Decision,## Consequences). That is a deliberate divergence from 0001/0002 (YAML frontmatter, 1–3 sentence body), as the ticket requires; the house brevity rule is not applied.The rules it fixes for
main, and onlymain: no direct pushes; no bypass, binding owner, agent sessions and bots alike; one approval from someone other than the author; the approval pinned to the revision it examined; automated checks gating the merge on the revision that lands (so the change must be current withmain); append-only history through merges; and rollback through the same door.Evidence
git show origin/main:docs/adr/0003-protected-main.md→fatal: path 'docs/adr/0003-protected-main.md' does not exist in 'origin/main'After:
git diff origin/main...HEAD --stat→docs/adr/0003-protected-main.md | 51 ++++++/1 file changed, 51 insertions(+)tofu fmt -check -recursive→ rc=0 (tree formatted). The repo exposes no test or lint target (make helplists only the tofu/Ansible targets;make -n test→No rule to make target 'test'), so none applies to a docs-only change.make validatecannot run on this host — it reads~/.ssh/id_ed25519_ansible.pub, which is absent — and no HCL is touched, so it would prove nothing here.Merge Danger
Door: two-way
A doc revert is cheap. The ADR is a record, not a mechanism: merging it changes no runtime behaviour and enforces nothing on its own.
Blast Radius: docs-only
It fixes the standard the repo's future changes are judged against, and its
## Consequencesstates plainly that the configuration must still change for the rules to hold, and that a setting read back from the platform is not evidence a rule binds.main5b0ceeba5a5b0ceeba5ato1736be1cadRecommendation: merge
Spec source: the PR description links no issue, so this review falls back to the unpublished local ticket draft (
~/drafts/infra-tracker-main-protection-spec.mdand its-ticket.mdcompanion) — the Vikunja task itself is unreachable from this session — and to the PR description for scope. Standards:AGENTS.md§Pull requests and.agents/skills/domain-modeling/ADR-FORMAT.md.Change: docs-only. One added file,
docs/adr/0003-protected-main.md(+50, blob7810bcf), recording themainstandard. No code, no HCL, no playbook, no inventory and no Makefile line is touched.Standards
docs/adr/0003-protected-main.md:1-50— deviates from the repo's documented ADR format..agents/skills/domain-modeling/ADR-FORMAT.md:9-13prescribes# Titleplus a 1–3 sentence body, withStatusfrontmatter andConsidered Options/Consequencesas optional sections; this ADR instead uses Nygard's full template (## Status/## Context/## Decision/## Consequences, ~50 lines, no frontmatter) and so does not match its siblings0001/0002, both of which carrystatus: acceptedfrontmatter. The PR body and the originating ticket both declare the divergence deliberate and mandate this template, so it is a spec-sanctioned deviation rather than a breach to fix — flagged once as a judgement call, not a gate.Spec
docs/adr/0003-protected-main.md:44-49— the ticket asks for the enforcement half too (## The change,## Acceptance): adjustmain's protection configuration until every rule is enforced and probe each rule by attempting the forbidden action. This docs-only PR delivers only the ADR; the configuration change and the probes are not here. The ADR itself records the gap (Consequences: the repository's configuration must change for every rule to hold), so this reads as a scoped split rather than a silent omission, and the PR delivers exactly what it claims (the ADR). Flagged so a human can confirm the config work is tracked elsewhere.docs/adr/0003-protected-main.md:24-26— the ticket's rule 4 example list includesgenerated files(ticket line 33) alongside docs, formatting, version bumps and one-line edits; the ADR keeps the first four and drops the fifth. The general clause (Every change reaches main through a pull request … no change is small enough to skip the door) still covers it. Nit.docs/adr/0003-protected-main.md:40-41— the ticket's rule 5 (no rebase of what is already integrated, ticket line 34) is rendered asmain grows through merges alone; a force-push cannot erase what was reviewed. The no-rebase intent is implied by merges-alone, not named. Nit.mainare present in## Decision: no direct pushes; no exemption (owner, agents and bots alike, no whitelist, no break-glass); approval by someone other than the author, never self-approval; the approval pinned to the revision it examined; the checks gating the merge on the revision that lands, with currency withmain; append-only history through merges; rollback through the same door. Scope is stated asmainonly, and the standard names no product and no configuration key (ticket §Implementation Decisions).Verified
5b0ceeba…matches the dispatch; a single commit5b0ceebsits in9735aa29…HEAD, and the base equals the merge-base9735aa29….git diff 9735aa29...5b0ceebis identical to the API diff: one added file, 50 insertions, blob7810bcf.git show: sha25649f9b3b909f3b38be23537f86dfd8f593cda37667103ab6b443625d3f639b6da.tofu fmt -check -recursive→ rc=0, matching the PR's Evidence.make -n test→No rule to make target 'test';make helplists no test or lint target, matching the PR's Evidence. No CI context checks exist on the head (get_commit_statuses→ 0).make validatecannot run here:vikunja/guest/keys.tf:3reads~/.ssh/id_ed25519_ansible.pub, absent on this host, matching the PR's Evidence.AGENTS.md§Pull requests and the.agents/skills/prtemplate —## Summary(tree diff),## Evidence(before/after),## Merge DangerwithDoor: two-wayandBlast Radius: docs-only.Not verified
mcp__vikunja__*tools); the local ticket draft was used as the spec instead.Summary
@ -0,0 +1,50 @@# Only a reviewed, checked pull request reaches `main`Title must be ADR-003: and the a title brief title. For example: Main branch protection.
@ -0,0 +7,4 @@## Context`main` is the revision every other piece of work is judged against, but therules that guard it live in habit and one line of the README. Nothing statesKeep abstract no implementation details. State why main is inportant
New commits pushed, approval review dismissed automatically according to repository settings
Both findings addressed in
dd425c4— docs-only, one file (docs/adr/0003-protected-main.md).Title (
issuecomment-1047) — resolved. The H1 is now# ADR-003: Main branch protection: brief, and prefixed with the ADR number as asked. The long descriptive sentence it replaced has been dropped.Context (
issuecomment-1052) — resolved. Rewritten to stay abstract, with no implementation details: the README reference and the enforcement mechanics are gone, and the section now states whymainmatters — it is the trunk every change is built on and merged back into, the source of truth the project is delivered from, and its integrity is the shared history everyone downstream relies on.Head / checks. Reviewed head was
1736be1; the fix isdd425c4onadr/0003-protected-main.tofu fmt -check -recursive→ rc=0. The repo exposes no test or lint target (make -n test→No rule to make target 'test'), so none applies to a docs-only change.PR body. The
## Evidenceinsertion count was updated50 → 51to match the new head (the fix adds one line); no other body change.Note on the number. I kept
ADR-003exactly as written; the README renders the existing reference asADR-0002(zero-padded). Say the word if you'd ratherADR-0003and I'll match it.State label moved
review/merge-ready→review/needs-review— the new commit invalidates the standing approval, so the branch needs re-review.Recommendation: merge
Spec source: the PR description links no issue, so this review falls back to the unpublished local ticket draft (
~/drafts/infra-tracker-main-protection-spec.mdand its-ticket.mdcompanion; the Vikunja task itself is unreachable from this session) and to the PR description for scope. Standards:AGENTS.md§Pull requests and.agents/skills/domain-modeling/ADR-FORMAT.md.Rounds: this is a genuine new round — the head moved since the last review (
1736be1→dd425c4, fast-forward). Standing at1736be1: bot review 69 (round 1/5, APPROVED) and human review 74 (REQUEST_CHANGES, two inline comments). The header round 2/5 is one past the highest standing round and matches the dispatch.Previous-round resolution check
ADR-003:plus a brief title. Resolved indd425c4:docs/adr/0003-protected-main.md:1is now# ADR-003: Main branch protection. The old H1,# Only a reviewed, checked pull request reaches main, is absent at the head (git grepreturns nothing).mainmatters. Resolved indd425c4:docs/adr/0003-protected-main.md:9-14no longer names the README or the enforcement mechanism; it states whymainmatters — the trunk every change is built on and merged back into, the delivered source of truth, the shared history downstream relies on. The old context text (one line of the README,judged against,a boundary chosen deliberately) is absent at the head.Both prior blockers are resolved; the round-1 reviews were otherwise APPROVE-or-advisory, so nothing else stood open.
Standards
docs/adr/0003-protected-main.md:1-51— deviates from the repo's documented ADR format..agents/skills/domain-modeling/ADR-FORMAT.md:9-15prescribes# Titleplus a 1-3 sentence body, withStatusfrontmatter andConsidered Options/Consequencesoptional; this ADR instead uses Nygard's full template and so does not match its siblings0001/0002, both of which carrystatus: acceptedfrontmatter. The originating ticket and the PR body both declare the divergence deliberate, and the round-1 human review left the template unflagged, so it stands as a spec-sanctioned judgement call, not a gate. Unchanged by the fix.docs/adr/0003-protected-main.md:1— the title numbers the ADRADR-003while the repo's own README reference renders the sibling in zero-padded form,ADR-0002(README.md:82). The round-1 human comment asked forADR-003:verbatim and the fixer kept it, so this is a consistency nit for a human to confirm, not a defect.Spec
docs/adr/0003-protected-main.md:41-51— the ticket asks for two halves (## The change,## Acceptance): record the ADR, and adjustmain's protection configuration until every rule is enforced, probing each by attempting the forbidden action. This docs-only PR delivers only the ADR. The ADR itself records the gap (## Consequences: the repository's configuration must change for every rule to hold) and the PR body declares the scope (The ADR is a record, not a mechanism), so it reads as a declared split rather than a silent omission. Flagged so a human can confirm the config work is tracked elsewhere. Unchanged by the fix.docs/adr/0003-protected-main.md:22-23— the ticket's rule 4 example list also namesgenerated files; the ADR keeps documentation, formatting, version bumps and one-line edits and drops it. The general clause (no change is small enough to skip the door) still covers it. Nit; unchanged by the fix.docs/adr/0003-protected-main.md:34-35— the ticket's rule 5 namesno rebase of what is already integrated; the ADR renders it asmain grows through merges alone; a force-push cannot erase what was reviewed. The no-rebase intent is implied, not named. Nit; unchanged by the fix.mainare present in## Decision(lines 21-37), scope ismainonly (line 39), and the standard names no product and no configuration key, as the ticket requires.Verified
dd425c4matches the dispatch;1736be1is an ancestor ofdd425c4(fast-forward, no history rewrite), and the base equals the merge-base9735aa29.git diff 9735aa2965313700edb52df70036d7daa8a6b501...dd425c4equals the API diff: one added file, 51 insertions, bloba76a5bd.git show dd425c4:...: sha2561b04519415d1f9de6fb632fea0c26220c0e4eefd321a3181db348c576b19f8a9,git hash-objecta76a5bd.1736be1...dd425c4touches only the title line and the## Contextsection — exactly the two blockers; nothing else moved.tofu fmt -check -recursive→ rc=0, matching the PR's Evidence.make -n test→No rule to make target 'test';make helplists no test or lint target, matching the PR's Evidence. No CI context checks exist on the head (get_commit_statuses→ count 0).AGENTS.md§Pull requests and the.agents/skills/pr/SKILL.mdtemplate:## Summary,## Evidence(before/after),## Merge DangerwithDoor: two-wayandBlast Radius: docs-only.Not verified
mcp__vikunja__*tools); the local ticket draft was used as the spec instead.Summary