docs(adr): adopt the standard that only a reviewed, checked pull request reaches main #18

Merged
pit merged 2 commits from adr/0003-protected-main into main 2026-10-10 14:49:09 +00:00
Member

Summary

Records the main standard as an ADR. Docs-only: one new file, no code.

 docs/adr/
 ├── 0001-cancel-superseded-ci-runs.md
 ├── 0002-dev-ci-vault-for-test-credentials.md
+└── 0003-protected-main.md   # the standard: a reviewed, checked PR, or nothing lands

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 only main: 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 with main); append-only history through merges; and rollback through the same door.

Evidence

  • Before: 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(+)
  • Repo checks: tofu fmt -check -recursive → rc=0 (tree formatted). The repo exposes no test or lint target (make help lists only the tofu/Ansible targets; make -n test → No rule to make target 'test'), so none applies to a docs-only change. make validate cannot 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.
  • No playbook, inventory, role, HCL or Makefile line is altered.

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 ## Consequences states 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.

## Summary Records the `main` standard as an ADR. Docs-only: one new file, no code. ```diff docs/adr/ ├── 0001-cancel-superseded-ci-runs.md ├── 0002-dev-ci-vault-for-test-credentials.md +└── 0003-protected-main.md # the standard: a reviewed, checked PR, or nothing lands ``` 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 only `main`: 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 with `main`); append-only history through merges; and rollback through the same door. ## Evidence - **Before:** `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(+)` - **Repo checks:** `tofu fmt -check -recursive` → rc=0 (tree formatted). The repo exposes no test or lint target (`make help` lists only the tofu/Ansible targets; `make -n test` → `No rule to make target 'test'`), so none applies to a docs-only change. `make validate` cannot 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. - No playbook, inventory, role, HCL or Makefile line is altered. ## 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 `## Consequences` states 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.
The rules guarding `main` lived in habit and one README line, so a gap in the
repository's configuration could not be told from a boundary chosen on purpose.
Record the standard as ADR 0003 — no direct pushes, every change through a pull
request, reviewed by someone other than the author, the approval pinned to the
revision it examined, automated checks gating the merge on the revision that
lands, append-only history, and a rollback through the front door — stated
without naming any platform or configuration key, because the mechanism is an
implementation of the standard rather than the decision itself.

`## Consequences` carries what must not be assumed: the configuration has to
change for every rule to hold, some rules hold today and some do not, and a rule
counts as enforced only where the forbidden action was attempted and refused.
bot-efesto force-pushed adr/0003-protected-main from 5b0ceeba5a to 1736be1cad 2026-10-10 14:34:04 +00:00 Compare
bot-minos approved these changes 2026-10-10 14:35:00 +00:00
Dismissed
bot-minos left a comment

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.md and its -ticket.md companion) — 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, blob 7810bcf), recording the main standard. No code, no HCL, no playbook, no inventory and no Makefile line is touched.

Standards

  • ADVISORY docs/adr/0003-protected-main.md:1-50 — deviates from the repo's documented ADR format. .agents/skills/domain-modeling/ADR-FORMAT.md:9-13 prescribes # Title plus a 1–3 sentence body, with Status frontmatter and Considered Options / Consequences as optional sections; this ADR instead uses Nygard's full template (## Status / ## Context / ## Decision / ## Consequences, ~50 lines, no frontmatter) and so does not match its siblings 0001/0002, both of which carry status: accepted frontmatter. 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.
  • No smell-baseline findings: the change is documentation, not code, so none of the Fowler smells apply.

Spec

  • ADVISORY docs/adr/0003-protected-main.md:44-49 — the ticket asks for the enforcement half too (## The change, ## Acceptance): adjust main'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.
  • ADVISORY docs/adr/0003-protected-main.md:24-26 — the ticket's rule 4 example list includes generated 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.
  • ADVISORY 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 as main grows through merges alone; a force-push cannot erase what was reviewed. The no-rebase intent is implied by merges-alone, not named. Nit.
  • All seven rules the ticket fixes for main are 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 with main; append-only history through merges; rollback through the same door. Scope is stated as main only, and the standard names no product and no configuration key (ticket §Implementation Decisions).

Verified

  • Head 5b0ceeba… matches the dispatch; a single commit 5b0ceeb sits in 9735aa29…HEAD, and the base equals the merge-base 9735aa29….
  • git diff 9735aa29...5b0ceeb is identical to the API diff: one added file, 50 insertions, blob 7810bcf.
  • The head blob is byte-identical between the API raw download and git show: sha256 49f9b3b909f3b38be23537f86dfd8f593cda37667103ab6b443625d3f639b6da.
  • tofu fmt -check -recursive → rc=0, matching the PR's Evidence.
  • make -n test → No rule to make target 'test'; make help lists no test or lint target, matching the PR's Evidence. No CI context checks exist on the head (get_commit_statuses → 0).
  • make validate cannot run here: vikunja/guest/keys.tf:3 reads ~/.ssh/id_ed25519_ansible.pub, absent on this host, matching the PR's Evidence.
  • The PR body satisfies AGENTS.md §Pull requests and the .agents/skills/pr template — ## Summary (tree diff), ## Evidence (before/after), ## Merge Danger with Door: two-way and Blast Radius: docs-only.
  • No PR comments and no standing reviews exist at this head.

Not verified

  • The originating Vikunja task is unreachable from this session (no mcp__vikunja__* tools); the local ticket draft was used as the spec instead.
  • Platform-side enforcement (whether checks gate the merge, whether a stale branch is blocked) is not readable with this token, and the change touches none of it — out of scope for a docs PR.

Summary

  • Standards: 1 finding, ADVISORY. Worst: the ADR-format deviation — deliberate and spec-sanctioned.
  • Spec: 3 findings, ADVISORY. Worst: the ticket's configuration-and-probe half is not delivered by this docs-only PR, an acknowledged scope split (the ADR records the gap).
  • No BLOCKER on either axis → APPROVE.
<!-- review-meta repo=olympus/infra-tracker pr=18 head=5b0ceeba5ab64396ddf6a3649e0fc215528c74ec round=1/5 verdict=approve --> 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.md` and its `-ticket.md` companion) — 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, blob `7810bcf`), recording the `main` standard. No code, no HCL, no playbook, no inventory and no Makefile line is touched. ### Standards - **ADVISORY** `docs/adr/0003-protected-main.md:1-50` — deviates from the repo's documented ADR format. `.agents/skills/domain-modeling/ADR-FORMAT.md:9-13` prescribes `# Title` plus a 1–3 sentence body, with `Status` frontmatter and `Considered Options` / `Consequences` as optional sections; this ADR instead uses Nygard's full template (`## Status` / `## Context` / `## Decision` / `## Consequences`, ~50 lines, no frontmatter) and so does not match its siblings `0001`/`0002`, both of which carry `status: accepted` frontmatter. 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. - No smell-baseline findings: the change is documentation, not code, so none of the Fowler smells apply. ### Spec - **ADVISORY** `docs/adr/0003-protected-main.md:44-49` — the ticket asks for the enforcement half too (`## The change`, `## Acceptance`): adjust `main`'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. - **ADVISORY** `docs/adr/0003-protected-main.md:24-26` — the ticket's rule 4 example list includes `generated 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. - **ADVISORY** `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 as `main grows through merges alone; a force-push cannot erase what was reviewed`. The no-rebase intent is implied by merges-alone, not named. Nit. - All seven rules the ticket fixes for `main` are 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 with `main`; append-only history through merges; rollback through the same door. Scope is stated as `main` only, and the standard names no product and no configuration key (ticket §Implementation Decisions). ### Verified - Head `5b0ceeba…` matches the dispatch; a single commit `5b0ceeb` sits in `9735aa29…HEAD`, and the base equals the merge-base `9735aa29…`. - `git diff 9735aa29...5b0ceeb` is identical to the API diff: one added file, 50 insertions, blob `7810bcf`. - The head blob is byte-identical between the API raw download and `git show`: sha256 `49f9b3b909f3b38be23537f86dfd8f593cda37667103ab6b443625d3f639b6da`. - `tofu fmt -check -recursive` → rc=0, matching the PR's Evidence. - `make -n test` → `No rule to make target 'test'`; `make help` lists no test or lint target, matching the PR's Evidence. No CI context checks exist on the head (`get_commit_statuses` → 0). - `make validate` cannot run here: `vikunja/guest/keys.tf:3` reads `~/.ssh/id_ed25519_ansible.pub`, absent on this host, matching the PR's Evidence. - The PR body satisfies `AGENTS.md` §Pull requests and the `.agents/skills/pr` template — `## Summary` (tree diff), `## Evidence` (before/after), `## Merge Danger` with `Door: two-way` and `Blast Radius: docs-only`. - No PR comments and no standing reviews exist at this head. ### Not verified - The originating Vikunja task is unreachable from this session (no `mcp__vikunja__*` tools); the local ticket draft was used as the spec instead. - Platform-side enforcement (whether checks gate the merge, whether a stale branch is blocked) is not readable with this token, and the change touches none of it — out of scope for a docs PR. ### Summary - Standards: 1 finding, ADVISORY. Worst: the ADR-format deviation — deliberate and spec-sanctioned. - Spec: 3 findings, ADVISORY. Worst: the ticket's configuration-and-probe half is not delivered by this docs-only PR, an acknowledged scope split (the ADR records the gap). - No BLOCKER on either axis → APPROVE.
pit requested changes 2026-10-10 14:43:44 +00:00
Dismissed
@ -0,0 +1,50 @@
# Only a reviewed, checked pull request reaches `main`
Owner

Title must be ADR-003: and the a title brief title. For example: Main branch protection.

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 the
rules that guard it live in habit and one line of the README. Nothing states
Owner

Keep abstract no implementation details. State why main is inportant

Keep abstract no implementation details. State why main is inportant
Review on PR #18 (pit, two inline findings on ADR 0003):

- Title: prefix with the ADR number and keep it brief, per the reviewer's
  format — `# ADR-003: Main branch protection`.
- Context: keep it abstract, no implementation details, and state why
  `main` matters. Dropped the README reference and the enforcement
  mechanics; the section now says what `main` is (the trunk every change
  is built on and merged back into) and why its integrity is the thing
  the standard protects.

Docs-only: one file, the ADR. No code, no HCL, no playbook touched.
`tofu fmt -check -recursive` -> rc=0.
bot-efesto dismissed bot-minos's review 2026-10-10 14:45:34 +00:00
Reason:

New commits pushed, approval review dismissed automatically according to repository settings

Author
Member

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 why main matters — 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 is dd425c4 on adr/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 ## Evidence insertion count was updated 50 → 51 to match the new head (the fix adds one line); no other body change.

Note on the number. I kept ADR-003 exactly as written; the README renders the existing reference as ADR-0002 (zero-padded). Say the word if you'd rather ADR-0003 and 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.

<!-- fix-meta repo=olympus/infra-tracker pr=18 head=dd425c4c6e39faa18f9678ae381e94c704da47e0 round=1/5 disposition=fixed --> 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 why `main` matters — 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 is `dd425c4` on `adr/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 `## Evidence` insertion count was updated `50 → 51` to match the new head (the fix adds one line); no other body change. **Note on the number.** I kept `ADR-003` exactly as written; the README renders the existing reference as `ADR-0002` (zero-padded). Say the word if you'd rather `ADR-0003` and 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.
bot-minos approved these changes 2026-10-10 14:47:12 +00:00
bot-minos left a comment

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.md and its -ticket.md companion; 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 at 1736be1: 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

  • BLOCKER (human inline, review 74 comment 1047): the title must be ADR-003: plus a brief title. Resolved in dd425c4: docs/adr/0003-protected-main.md:1 is now # ADR-003: Main branch protection. The old H1, # Only a reviewed, checked pull request reaches main, is absent at the head (git grep returns nothing).
  • BLOCKER (human inline, review 74 comment 1052): Context must stay abstract with no implementation details, and state why main matters. Resolved in dd425c4: docs/adr/0003-protected-main.md:9-14 no longer names the README or the enforcement mechanism; it states why main matters — 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

  • ADVISORY docs/adr/0003-protected-main.md:1-51 — deviates from the repo's documented ADR format. .agents/skills/domain-modeling/ADR-FORMAT.md:9-15 prescribes # Title plus a 1-3 sentence body, with Status frontmatter and Considered Options / Consequences optional; this ADR instead uses Nygard's full template and so does not match its siblings 0001/0002, both of which carry status: accepted frontmatter. 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.
  • ADVISORY docs/adr/0003-protected-main.md:1 — the title numbers the ADR ADR-003 while the repo's own README reference renders the sibling in zero-padded form, ADR-0002 (README.md:82). The round-1 human comment asked for ADR-003: verbatim and the fixer kept it, so this is a consistency nit for a human to confirm, not a defect.
  • No smell-baseline findings: the change is documentation, not code, so none of the Fowler smells apply.

Spec

  • ADVISORY docs/adr/0003-protected-main.md:41-51 — the ticket asks for two halves (## The change, ## Acceptance): record the ADR, and adjust main'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.
  • ADVISORY docs/adr/0003-protected-main.md:22-23 — the ticket's rule 4 example list also names generated 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.
  • ADVISORY docs/adr/0003-protected-main.md:34-35 — the ticket's rule 5 names no rebase of what is already integrated; the ADR renders it as main 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.
  • All seven rules the ticket fixes for main are present in ## Decision (lines 21-37), scope is main only (line 39), and the standard names no product and no configuration key, as the ticket requires.

Verified

  • Head dd425c4 matches the dispatch; 1736be1 is an ancestor of dd425c4 (fast-forward, no history rewrite), and the base equals the merge-base 9735aa29.
  • git diff 9735aa2965313700edb52df70036d7daa8a6b501...dd425c4 equals the API diff: one added file, 51 insertions, blob a76a5bd.
  • The head blob is byte-identical between the API raw download and git show dd425c4:...: sha256 1b04519415d1f9de6fb632fea0c26220c0e4eefd321a3181db348c576b19f8a9, git hash-object a76a5bd.
  • The fix diff 1736be1...dd425c4 touches only the title line and the ## Context section — 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 help lists no test or lint target, matching the PR's Evidence. No CI context checks exist on the head (get_commit_statuses → count 0).
  • The PR body satisfies AGENTS.md §Pull requests and the .agents/skills/pr/SKILL.md template: ## Summary, ## Evidence (before/after), ## Merge Danger with Door: two-way and Blast Radius: docs-only.

Not verified

  • The originating Vikunja task is unreachable from this session (no mcp__vikunja__* tools); the local ticket draft was used as the spec instead.
  • Platform-side enforcement (whether checks gate a merge, whether a stale branch is blocked) is not readable with this token, and the change touches none of it — out of scope for a docs PR.

Summary

  • Standards: 2 findings, both ADVISORY. Worst: the ADR-format deviation, deliberate and spec-sanctioned, unchanged by the fix.
  • Spec: 3 findings, all ADVISORY. Worst: the ticket's configuration-and-probe half is not delivered by this docs-only PR, a declared scope split. Unchanged by the fix.
  • No BLOCKER on either axis; both round-1 blockers are resolved → APPROVE.
<!-- review-meta repo=olympus/infra-tracker pr=18 head=dd425c4c6e39faa18f9678ae381e94c704da47e0 round=2/5 verdict=approve --> 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.md` and its `-ticket.md` companion; 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 at `1736be1`: 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 - **BLOCKER (human inline, review 74 comment 1047): the title must be `ADR-003:` plus a brief title.** Resolved in `dd425c4`: `docs/adr/0003-protected-main.md:1` is now `# ADR-003: Main branch protection`. The old H1, `# Only a reviewed, checked pull request reaches main`, is absent at the head (`git grep` returns nothing). - **BLOCKER (human inline, review 74 comment 1052): Context must stay abstract with no implementation details, and state why `main` matters.** Resolved in `dd425c4`: `docs/adr/0003-protected-main.md:9-14` no longer names the README or the enforcement mechanism; it states why `main` matters — 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 - **ADVISORY** `docs/adr/0003-protected-main.md:1-51` — deviates from the repo's documented ADR format. `.agents/skills/domain-modeling/ADR-FORMAT.md:9-15` prescribes `# Title` plus a 1-3 sentence body, with `Status` frontmatter and `Considered Options` / `Consequences` optional; this ADR instead uses Nygard's full template and so does not match its siblings `0001`/`0002`, both of which carry `status: accepted` frontmatter. 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. - **ADVISORY** `docs/adr/0003-protected-main.md:1` — the title numbers the ADR `ADR-003` while the repo's own README reference renders the sibling in zero-padded form, `ADR-0002` (`README.md:82`). The round-1 human comment asked for `ADR-003:` verbatim and the fixer kept it, so this is a consistency nit for a human to confirm, not a defect. - No smell-baseline findings: the change is documentation, not code, so none of the Fowler smells apply. ### Spec - **ADVISORY** `docs/adr/0003-protected-main.md:41-51` — the ticket asks for two halves (`## The change`, `## Acceptance`): record the ADR, and adjust `main`'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. - **ADVISORY** `docs/adr/0003-protected-main.md:22-23` — the ticket's rule 4 example list also names `generated 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. - **ADVISORY** `docs/adr/0003-protected-main.md:34-35` — the ticket's rule 5 names `no rebase of what is already integrated`; the ADR renders it as `main 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. - All seven rules the ticket fixes for `main` are present in `## Decision` (lines 21-37), scope is `main` only (line 39), and the standard names no product and no configuration key, as the ticket requires. ### Verified - Head `dd425c4` matches the dispatch; `1736be1` is an ancestor of `dd425c4` (fast-forward, no history rewrite), and the base equals the merge-base `9735aa29`. - `git diff 9735aa2965313700edb52df70036d7daa8a6b501...dd425c4` equals the API diff: one added file, 51 insertions, blob `a76a5bd`. - The head blob is byte-identical between the API raw download and `git show dd425c4:...`: sha256 `1b04519415d1f9de6fb632fea0c26220c0e4eefd321a3181db348c576b19f8a9`, `git hash-object` `a76a5bd`. - The fix diff `1736be1...dd425c4` touches only the title line and the `## Context` section — 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 help` lists no test or lint target, matching the PR's Evidence. No CI context checks exist on the head (`get_commit_statuses` → count 0). - The PR body satisfies `AGENTS.md` §Pull requests and the `.agents/skills/pr/SKILL.md` template: `## Summary`, `## Evidence` (before/after), `## Merge Danger` with `Door: two-way` and `Blast Radius: docs-only`. ### Not verified - The originating Vikunja task is unreachable from this session (no `mcp__vikunja__*` tools); the local ticket draft was used as the spec instead. - Platform-side enforcement (whether checks gate a merge, whether a stale branch is blocked) is not readable with this token, and the change touches none of it — out of scope for a docs PR. ### Summary - Standards: 2 findings, both ADVISORY. Worst: the ADR-format deviation, deliberate and spec-sanctioned, unchanged by the fix. - Spec: 3 findings, all ADVISORY. Worst: the ticket's configuration-and-probe half is not delivered by this docs-only PR, a declared scope split. Unchanged by the fix. - No BLOCKER on either axis; both round-1 blockers are resolved → APPROVE.
pit approved these changes 2026-10-10 14:49:02 +00:00
pit merged commit 7373d49403 into main 2026-10-10 14:49:09 +00:00
pit deleted branch adr/0003-protected-main 2026-10-10 14:49:10 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
olympus/infra-tracker!18
No description provided.