olympus/infra-forge
3
0
Fork 0
Code Issues 2 Pull requests Projects Releases Packages Wiki Activity Actions

ADR: the forgejo-mcp Service's shape and credential-less endpoint (#21) #43

Merged
pit merged 4 commits from hermes/21-forgejo-mcp-adr into main 2026-10-06 20:32:59 +00:00
Conversation 3 Commits 4 Files changed 3 +99 -1
pit commented 2026-10-06 16:12:48 +00:00
Owner
Copy link

Summary

The decision record that gates the forgejo-mcp work: the Service's shape and its endpoint's identity. Documentation-only; no role or Stack changes.

 docs/adr/
 ├── 0001-forgejo-actions-runner-on-guest.md
 ├── 0002-main-protected-at-the-forge.md
+├── 0003-forgejo-mcp-service-shape-and-endpoint.md
 └── index.md                    (+1 row for 0003)

Fixes, in prose, the choices everything after #21 depends on:

  • a dedicated role for forgejo-mcp as a third Service on the forgejo Guest, separate from the forgejo role, with its own play;
  • Streamable HTTP at /mcp (not SSE, not stdio);
  • a credential-less endpoint in passthrough mode, operator-token fallback deliberately off — the Service holds no Forgejo credential;
  • bind on the Guest interface behind a host allowlist, fail-closed;
  • upstream to Forgejo over the Guest's loopback, not through the Edge;
  • no Edge entry, certificate, DNS record or router rule — LAN-reachable, not WAN-published.

Each rejected alternative is recorded once: the signed OCI image, SSE/stdio, a configured Service token, and the operator-token fallback.

Evidence

Numbering: next free number taken at write time — docs/adr/ held 0001 and 0002, so this lands at 0003, with the index row added. The publish manifest publishes docs/adr/* as a glob, so no manifest edit is needed.

  • Before: no ADR for the Service; docs/adr/index.md listed 0001–0002.
  • After: docs/adr/index.md lists 0001–0003; make fmt exits 0; every relative link in the changed docs resolves in-repo.

Grounding checked against the upstream project and the parent spec, not restated from memory:

  • Transport, bind/allowlist fail-closed behaviour (refuses to start on a non-loopback bind with no allowlist; 403 on an undeclared Host), passthrough 401, and the --allow-operator-token-fallback warning all read back from upstream's v3.2.0 README.
  • The pinned archive's sha256 (bf8f744d…7039df) was verified by downloading forgejo-mcp_3.2.0_linux_amd64.tar.gz and hashing it — it matches the literal pinned in the parent spec.

Merge Danger

Door: two-way

Docs-only; reverting is a git revert of one file plus one table row. The record takes the next free ADR number rather than a reserved one, so a conflicting ADR landing first would want a renumber before merge; the index row and the filename are the only places the number appears.

Blast Radius: documentation

The rationale is not inert — #22 (role), #23 (Agent round-trip) and #24 (token scope) are written against it, so a wrong line here propagates. Reviewing the why matters more than the wording.

## Summary The decision record that gates the forgejo-mcp work: the Service's shape and its endpoint's identity. Documentation-only; no role or Stack changes. ```diff docs/adr/ ├── 0001-forgejo-actions-runner-on-guest.md ├── 0002-main-protected-at-the-forge.md +├── 0003-forgejo-mcp-service-shape-and-endpoint.md └── index.md (+1 row for 0003) ``` Fixes, in prose, the choices everything after #21 depends on: - a dedicated role for forgejo-mcp as a third **Service** on the `forgejo` Guest, separate from the `forgejo` role, with its own play; - Streamable HTTP at `/mcp` (not SSE, not stdio); - a **credential-less** endpoint in `passthrough` mode, operator-token fallback deliberately off — the Service holds no Forgejo credential; - bind on the Guest interface behind a host allowlist, fail-closed; - upstream to Forgejo over the Guest's loopback, not through the Edge; - no Edge entry, certificate, DNS record or router rule — LAN-reachable, not WAN-published. Each rejected alternative is recorded once: the signed OCI image, SSE/stdio, a configured Service token, and the operator-token fallback. ## Evidence Numbering: next free number taken at write time — `docs/adr/` held 0001 and 0002, so this lands at **0003**, with the index row added. The publish manifest publishes `docs/adr/*` as a glob, so no manifest edit is needed. - **Before:** no ADR for the Service; `docs/adr/index.md` listed 0001–0002. - **After:** `docs/adr/index.md` lists 0001–0003; `make fmt` exits 0; every relative link in the changed docs resolves in-repo. Grounding checked against the upstream project and the parent spec, not restated from memory: - Transport, bind/allowlist fail-closed behaviour (`refuses to start` on a non-loopback bind with no allowlist; `403` on an undeclared `Host`), `passthrough` `401`, and the `--allow-operator-token-fallback` warning all read back from upstream's v3.2.0 README. - The pinned archive's sha256 (`bf8f744d…7039df`) was verified by downloading `forgejo-mcp_3.2.0_linux_amd64.tar.gz` and hashing it — it matches the literal pinned in the parent spec. ## Merge Danger **Door:** two-way Docs-only; reverting is a `git revert` of one file plus one table row. The record takes the *next free* ADR number rather than a reserved one, so a conflicting ADR landing first would want a renumber before merge; the index row and the filename are the only places the number appears. **Blast Radius:** documentation The rationale is not inert — #22 (role), #23 (Agent round-trip) and #24 (token scope) are written against it, so a wrong line here propagates. Reviewing the *why* matters more than the wording.
pit force-pushed hermes/21-forgejo-mcp-adr from 4d8e53c77d to eefe98c378 2026-10-06 20:06:22 +00:00 Compare
pit commented 2026-10-06 20:11:01 +00:00
Author
Owner
Copy link

Standards review — PR #43

Axis: does the diff follow this repo's documented coding standards?
Diff: git diff main...origin/hermes/21-forgejo-mcp-adr (commits 632dff0, eefe98c; docs-only: new docs/adr/0003-forgejo-mcp-service-shape-and-endpoint.md + one row in docs/adr/index.md).
Standards sources: AGENTS.md, docs/index.md, docs/adr/index.md, GLOSSARY.md + docs/agents/domain.md, plus the Fowler smell baseline.

Hard violations

  1. Vocabulary/glossary contradiction — documented standard.
    GLOSSARY.md:69-72 defines Service as a closed list: "Forgejo and the Actions runner are the Services on the forgejo Guest" (_Avoid_: app, daemon, process). The ADR names a "third Service" (l.1, l.9) — extending a term whose definition excludes it — and calls the same thing "the server": "The token, not the server, is the capability boundary" (l.33), "the MCP server needs no rewrite" (l.52). docs/agents/domain.md:43 requires "the term as defined in GLOSSARY.md … Don't drift to synonyms," and docs/index.md:14 says the glossary is updated when "a term is coined, or one drifts." Neither the glossary nor a conflict flag was added.

  2. Unit/role count is wrong — verifiable defect, not taste.
    "A third Ansible role and a second systemd unit land on a Guest sized for one Service" (l.69). The role already writes two units — forgejo.service (ansible/roles/forgejo/tasks/main.yml:80) and forgejo-runner.service (:226) — so forgejo-mcp's unit would be the third; only roles/forgejo exists (ansible/roles/, site.yml), so its role would be the second. The record misstates what the change adds.

Judgement calls (baseline smells)

  • Duplicated Code: the credential-less rationale is stated three times — passthrough refusal (l.28-30), fallback off (l.35-39), and Consequences "no reusable Forgejo credential" (l.79-80). Decision+consequence echo is ADR-normal; the third pass adds no new cost.
  • Mysterious Name: "the server"/"the MCP server" (l.33, l.52) has two candidate referents (forgejo-mcp vs Forgejo). Name the Service.
  • Formatting drift: heading is 98 chars vs 57/75 in 0001/0002; the added index row is 404 chars vs 277/219. No prose linting is enforced, so this is only a rhythm break.
  • Possible Divergent Change: 998 words / ~7 sub-decisions vs 530/618 in 0001/0002, against "one file per decision." It reads as one decision with sub-choices (like 0001), so flagging only.

Privacy rule respected — only 10.12.0.141, already published in docs/architecture.md; no DDNS name or external IP. Index row and file added per convention; relative links resolve.

Standards axis: 6 findings (2 hard, 4 judgement calls). Worst: the l.69 unit/role count.

## Standards review — PR #43 Axis: does the diff follow this repo's documented coding standards? Diff: `git diff main...origin/hermes/21-forgejo-mcp-adr` (commits `632dff0`, `eefe98c`; docs-only: new `docs/adr/0003-forgejo-mcp-service-shape-and-endpoint.md` + one row in `docs/adr/index.md`). Standards sources: `AGENTS.md`, `docs/index.md`, `docs/adr/index.md`, `GLOSSARY.md` + `docs/agents/domain.md`, plus the Fowler smell baseline. ### Hard violations 1. **Vocabulary/glossary contradiction** — *documented standard*. `GLOSSARY.md:69-72` defines **Service** as a closed list: "Forgejo and the Actions runner are the Services on the `forgejo` Guest" (`_Avoid_: app, daemon, process`). The ADR names a "third **Service**" (l.1, l.9) — extending a term whose definition excludes it — and calls the same thing "the server": "The token, not the server, is the capability boundary" (l.33), "the MCP server needs no rewrite" (l.52). `docs/agents/domain.md:43` requires "the term as defined in `GLOSSARY.md` … Don't drift to synonyms," and `docs/index.md:14` says the glossary is updated when "a term is coined, or one drifts." Neither the glossary nor a conflict flag was added. 2. **Unit/role count is wrong** — *verifiable defect, not taste*. "A third Ansible role and a second systemd unit land on a Guest sized for one Service" (l.69). The role already writes two units — `forgejo.service` (`ansible/roles/forgejo/tasks/main.yml:80`) and `forgejo-runner.service` (`:226`) — so forgejo-mcp's unit would be the **third**; only `roles/forgejo` exists (`ansible/roles/`, `site.yml`), so its role would be the **second**. The record misstates what the change adds. ### Judgement calls (baseline smells) - **Duplicated Code**: the credential-less rationale is stated three times — `passthrough` refusal (l.28-30), fallback off (l.35-39), and Consequences "no reusable Forgejo credential" (l.79-80). Decision+consequence echo is ADR-normal; the third pass adds no new cost. - **Mysterious Name**: "the server"/"the MCP server" (l.33, l.52) has two candidate referents (forgejo-mcp vs Forgejo). Name the Service. - **Formatting drift**: heading is 98 chars vs 57/75 in 0001/0002; the added index row is 404 chars vs 277/219. No prose linting is enforced, so this is only a rhythm break. - **Possible Divergent Change**: 998 words / ~7 sub-decisions vs 530/618 in 0001/0002, against "one file per decision." It reads as one decision with sub-choices (like 0001), so flagging only. Privacy rule respected — only `10.12.0.141`, already published in `docs/architecture.md`; no DDNS name or external IP. Index row and file added per convention; relative links resolve. _Standards axis: 6 findings (2 hard, 4 judgement calls). Worst: the l.69 unit/role count._
pit commented 2026-10-06 20:11:02 +00:00
Author
Owner
Copy link

Spec review — PR #43

Axis: does the diff match what the originating issue/spec asked for?
Spec source: Forgejo issue #21 — "ADR: the forgejo-mcp Service's shape and credential-less endpoint" (parent #17).
Diff: git diff main...origin/hermes/21-forgejo-mcp-adr (commits 632dff0, eefe98c).

1. Wrongly implemented — role/unit/Guest counts contradict spec and repo

ADR line 69 (Consequences): "A third Ansible role and a second systemd unit land on a Guest sized for one Service". Every count is wrong. The repo holds one role (ansible/roles/ contains only forgejo; site.yml lists only forgejo) and two systemd units (forgejo.service, forgejo-runner.service). So the incoming role is the second role and its unit the third unit — the ADR swapped the two ordinals. The Guest is sized for two Services, not one: spec #17/GLOSSARY.md line 71 — "Forgejo and the Actions runner are the Services on the forgejo Guest" — and the ADR itself says the pin lands "beside the other two Services' pins" (line 20). Directly contradicts #21's "a dedicated Ansible role for forgejo-mcp as a third Service on the forgejo Guest".

2. Missing/partial — endpoint identity omits the port

#21 asks for "the shape of the forgejo-mcp Service and the identity of its endpoint". The ADR fixes transport (/mcp), bind and allowlist host (10.12.0.141), but never states the port — "beside its port" (line 63) is the only mention, and grep for 8089 in the ADR returns nothing. #17 names it: http://10.12.0.141:8089/mcp. The endpoint's actual address is therefore not recorded.

Scope creep

None. The intro framing, Consequences and Update-when sections are standard ADR format; the --allowed-origins note is required by #17's Implementation Decisions.

Acceptance criteria met

  • Numbering/index: 0003 is the next free number (0001/0002 exist); a matching index row was added. ✅
  • The seven "why" decisions all recorded: separate role vs extending; Streamable HTTP over SSE/stdio; passthrough over a configured token; fallback off; bind+allowlist fail-closed; loopback upstream; no Edge entry. ✅
  • Four rejected alternatives, each once: signed OCI image (amd64-only, persistent-container pattern); SSE and stdio; a Service-configured token; enabling the operator-token fallback. ✅
  • Correction commit eefe98c is accurate: the host allowlist is a Host-header check, not a source-address control. ✅

Spec axis: 2 findings (1 wrongly implemented, 1 missing/partial, 0 scope creep). Worst: the l.69 count error, which contradicts #21's "third Service".

## Spec review — PR #43 Axis: does the diff match what the originating issue/spec asked for? Spec source: Forgejo issue **#21** — "ADR: the forgejo-mcp Service's shape and credential-less endpoint" (parent #17). Diff: `git diff main...origin/hermes/21-forgejo-mcp-adr` (commits `632dff0`, `eefe98c`). ### 1. Wrongly implemented — role/unit/Guest counts contradict spec and repo ADR line 69 (Consequences): "A third Ansible role and a second systemd unit land on a Guest sized for one Service". Every count is wrong. The repo holds **one** role (`ansible/roles/` contains only `forgejo`; `site.yml` lists only `forgejo`) and **two** systemd units (`forgejo.service`, `forgejo-runner.service`). So the incoming role is the *second* role and its unit the *third* unit — the ADR swapped the two ordinals. The Guest is sized for **two** Services, not one: spec #17/`GLOSSARY.md` line 71 — "Forgejo and the Actions runner are the Services on the `forgejo` Guest" — and the ADR itself says the pin lands "beside the other two Services' pins" (line 20). Directly contradicts #21's "a dedicated Ansible role for forgejo-mcp as a third Service on the `forgejo` Guest". ### 2. Missing/partial — endpoint identity omits the port #21 asks for "the shape of the forgejo-mcp Service and the **identity of its endpoint**". The ADR fixes transport (`/mcp`), bind and allowlist host (`10.12.0.141`), but never states the port — "beside its port" (line 63) is the only mention, and grep for `8089` in the ADR returns nothing. #17 names it: `http://10.12.0.141:8089/mcp`. The endpoint's actual address is therefore not recorded. ### Scope creep None. The intro framing, Consequences and Update-when sections are standard ADR format; the `--allowed-origins` note is required by #17's Implementation Decisions. ### Acceptance criteria met - **Numbering/index:** 0003 is the next free number (0001/0002 exist); a matching index row was added. ✅ - **The seven "why" decisions** all recorded: separate role vs extending; Streamable HTTP over SSE/stdio; `passthrough` over a configured token; fallback off; bind+allowlist fail-closed; loopback upstream; no Edge entry. ✅ - **Four rejected alternatives**, each once: signed OCI image (amd64-only, persistent-container pattern); SSE and stdio; a Service-configured token; enabling the operator-token fallback. ✅ - **Correction commit `eefe98c`** is accurate: the host allowlist is a `Host`-header check, not a source-address control. ✅ _Spec axis: 2 findings (1 wrongly implemented, 1 missing/partial, 0 scope creep). Worst: the l.69 count error, which contradicts #21's "third Service"._ 
- Consequences: the incoming role is the *second* (only `forgejo` exists) and
  its unit the *third* (`forgejo.service`, `forgejo-runner.service`); the
  Guest is sized for two Services, not one.
- Endpoint identity: record the port (`8089`) alongside bind and allowlist,
  so the endpoint address is fully stated.
- GLOSSARY.md: the Service list is now open to forgejo-mcp, closing the
  contradiction the standards review flagged.
- Name forgejo-mcp instead of "the server"/"the MCP server" where the
  referent was ambiguous; trim the index row and title line-length drift.
# Conflicts:
#	docs/adr/index.md
pit commented 2026-10-06 20:32:24 +00:00
Author
Owner
Copy link

Review addressed — commit be769dc

Both review axes are answered in a new commit on the branch (be769dc, plus a merge of main at 72a7e2b). Diff against main is now three files: docs/adr/0003-...md, docs/adr/index.md, GLOSSARY.md.

Standards review (comment 500)

  1. Vocabulary/glossary contradiction — the glossary was the drifting artifact, so it is updated here (per docs/index.md:14): GLOSSARY.md's Service entry now reads "Forgejo, the Actions runner and forgejo-mcp are the Services on the forgejo Guest". Nothing else in the repo enumerates the Services, so the contradiction is closed in one place. The ambiguous "the server"/"the MCP server" referents (l.33, l.52) now name forgejo-mcp.
  2. Unit/role count — corrected. Consequences now reads: "A second Ansible role and a third systemd unit land on a Guest sized for two Services". Verified against the tree: ansible/roles/ holds only forgejo (so incoming = second role); the role writes forgejo.service (tasks/main.yml:80) and forgejo-runner.service (:226) (so incoming = third unit); the Guest runs two Services.

Judgement calls:

  • Duplicated Code: left as-is — the three credential-less passes are decision, rejection rationale, and consequence; trimming the third would drop the "no reusable credential" consequence the Operator cares about. Flagged, not actioned.
  • Formatting drift: the 0003 heading is now 75 chars (in line with 0001/0002), and the index row is trimmed from 404 to 362 chars. Prose linting isn't enforced; this was rhythm only.
  • Possible Divergent Change: left as one file — as the review notes, it reads as one decision with sub-choices (like 0001).

Spec review (comment 501)

  1. Role/unit/Guest counts — same fix as above; the three wrong ordinals are gone.
  2. Endpoint identity omits the port — fixed. The bind decision now states port 8089 and the full endpoint http://10.12.0.141:8089/mcp, matching #17.

The "third Service" phrasing (heading intro, l.9) is kept where it is spec-accurate — #21/#17 both call forgejo-mcp "a third Service" — and the glossary now agrees with it.

Merge conflict

main advanced to 860ace7 (PR #46, ADR 0004) while this was open; #46 deliberately left 0003 free, so no renumber was needed. The only conflict was both PRs adding a row to docs/adr/index.md; both rows are kept in numeric order (0003 then 0004). mergeable: true.

make fmt exits 0; all relative links in the changed docs resolve in-repo.

## Review addressed — commit `be769dc` Both review axes are answered in a new commit on the branch (`be769dc`, plus a merge of `main` at `72a7e2b`). Diff against `main` is now three files: `docs/adr/0003-...md`, `docs/adr/index.md`, `GLOSSARY.md`. ### Standards review (comment 500) 1. **Vocabulary/glossary contradiction** — the glossary was the drifting artifact, so it is updated here (per `docs/index.md:14`): `GLOSSARY.md`'s **Service** entry now reads "Forgejo, the Actions runner and forgejo-mcp are the Services on the `forgejo` Guest". Nothing else in the repo enumerates the Services, so the contradiction is closed in one place. The ambiguous "the server"/"the MCP server" referents (l.33, l.52) now name **forgejo-mcp**. 2. **Unit/role count** — corrected. Consequences now reads: *"A second Ansible role and a third systemd unit land on a Guest sized for two Services"*. Verified against the tree: `ansible/roles/` holds only `forgejo` (so incoming = second role); the role writes `forgejo.service` (`tasks/main.yml:80`) and `forgejo-runner.service` (`:226`) (so incoming = third unit); the Guest runs two Services. Judgement calls: - **Duplicated Code:** left as-is — the three credential-less passes are decision, rejection rationale, and consequence; trimming the third would drop the "no reusable credential" consequence the Operator cares about. Flagged, not actioned. - **Formatting drift:** the 0003 heading is now 75 chars (in line with 0001/0002), and the index row is trimmed from 404 to 362 chars. Prose linting isn't enforced; this was rhythm only. - **Possible Divergent Change:** left as one file — as the review notes, it reads as one decision with sub-choices (like 0001). ### Spec review (comment 501) 1. **Role/unit/Guest counts** — same fix as above; the three wrong ordinals are gone. 2. **Endpoint identity omits the port** — fixed. The bind decision now states port `8089` and the full endpoint `http://10.12.0.141:8089/mcp`, matching #17. The "third **Service**" phrasing (heading intro, l.9) is kept where it is spec-accurate — #21/#17 both call forgejo-mcp "a third Service" — and the glossary now agrees with it. ### Merge conflict `main` advanced to `860ace7` (PR #46, ADR 0004) while this was open; #46 deliberately left 0003 free, so **no renumber was needed**. The only conflict was both PRs adding a row to `docs/adr/index.md`; both rows are kept in numeric order (0003 then 0004). `mergeable: true`. `make fmt` exits 0; all relative links in the changed docs resolve in-repo.
pit merged commit a9146d751a into main 2026-10-06 20:32:59 +00:00
pit deleted branch hermes/21-forgejo-mcp-adr 2026-10-06 20:32:59 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
needs-info
Waiting on reporter for more information

  
needs-triage
Maintainer needs to evaluate this issue

  
ready-for-agent
Ready for an agent to pick up; see docs/agents/

  
ready-for-human
Requires human implementation

  
wontfix
Will not be actioned

  
needs-info
Waiting on reporter for more information

  
needs-triage
Maintainer needs to evaluate this issue

  
ready-for-agent
Fully specified, ready for an AFK agent

  
ready-for-human
Requires human implementation

  
review/merge-ready
No blocking findings; human merge pending

  
review/needs-fix
Reviewer found blocking findings; fixer dispatched

  
review/needs-human
Circuit stopped: needs a human decision

  
review/needs-review
Awaiting review by the review agent

  
wayfinder:grilling
HITL ticket: conversation; grilling + domain-modeling

  
wayfinder:map
The map itself: destination, decisions so far, and the fog

  
wayfinder:prototype
HITL ticket: a cheap concrete artifact to react to

  
wayfinder:research
AFK ticket: read docs/APIs for a fact a decision waits on

  
wayfinder:task
HITL or AFK ticket: manual work that unblocks a decision

  
wontfix
Will not be actioned

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
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
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-forge!43
No description provided.