Install forgejo-mcp as a Service on the forgejo Guest (#22) #48

Merged

pit merged 4 commits from hermes/22-install-forgejo-mcp-server into main 2026-10-07 06:48:16 +00:00

Conversation 2 Commits 4 Files changed 8 +345 -13

pit commented 2026-10-07 06:11:34 +00:00

Owner

Copy link

Summary

A new forgejo_mcp role and its own play install the pinned forgejo-mcp release as a third Service on the forgejo Guest. The endpoint is credential-less: it holds no Forgejo token, and in passthrough auth a request without its own credential is refused 401.

 ansible/
 ├── site.yml                     (+ play 2: role forgejo_mcp, after the untouched forgejo role)
 ├── group_vars/forgejo/vars.yml  (+ forgejo_mcp_allowed_hosts: 10.12.0.141)
 ├── host_vars/forgejo-pre.yml    (+ forgejo_mcp_allowed_hosts: 10.12.0.142)
 └── roles/
     └── forgejo_mcp/
         ├── defaults/main.yml     version, port, literal sha256, bind, upstream
         ├── handlers/main.yml     restart on config change
         └── tasks/main.yml        user + tree, checksum-first fetch, unit, 401 probe

The shape is ADR 0003 (merged as #43); this PR is the role that honours it:

• version, port and the archive's literal sha256 pinned in defaults — the fetch is checksum-first against that hash, not a remote checksum URL (upstream ships no per-file .sha256);
• dedicated forgejo-mcp service user; systemd unit enabled at boot, restarted on config change;
• bound 0.0.0.0:8089 behind an inventory-scoped host allowlist; an undeclared Host is refused 403;
• --auth-mode passthrough passed explicitly, and --allow-operator-token-fallback deliberately absent;
• upstream is the Guest's own loopback (127.0.0.1:3080); no Edge entry, certificate, DNS record or router rule.

The forgejo role is untouched, so its handler-order contract (the runner restart lands after the Forgejo restart) stays intact.

Evidence

Rehearsed on a systemd Guest stand-in (a disposable privileged Debian 13 container running site.yml's new play unmodified — not the PRE Guest; see below). The role is exercised for real: systemd, the service user, the checksum-first fetch, the readiness probe.

• Cold run from nothing (no service user, no ca-certificates): converges, changed=11.
• Second run: changed=0.
• Reboot of the Guest stand-in: forgejo-mcp.service comes back active on its own, endpoint answers 401.
• Config change (port 8089 → 8090): the unit re-renders, the handler restarts, and the endpoint answers 401 on the new port. The first attempt failed here — the handler ran after the readiness probe, so the probe polled a port the old process no longer held. Fixed with meta: flush_handlers before the probe; that fix is what this run verifies.
• Credential-less / allowlist, from another host against the running Service:

  request	answer
  no credential, declared Host	401
  undeclared Host (evil.example)	403
  declared Host on any port	401 (the bare allowlist matches any port)

• Checksum-first guard held: a wrong pinned hash for the identical URL failed the fetch rather than installing the archive.
• --check --diff (the repo's rehearsal gate) passes; site.yml --syntax-check passes on both inventories; tofu fmt -recursive -check clean.

Two-axis review (/code-review) on the diff; the one substantive finding — forgejo_mcp_auth_mode was declared in defaults but never passed to ExecStart, so the credential-less guarantee rested on an upstream default while a comment claimed otherwise — is fixed in the second commit.

Merge Danger

Door: two-way

Reverting is a git revert: the role directory and the new play drop out, and the Service stops on the next run. No secret was added (the Service is credential-less), so nothing needs rotating. The one thing that does not revert cleanly by itself is a pin mismatch — the role's forgejo_mcp_forgejo_port restates the forgejo role's forgejo_http_port across plays and must move with it.

Blast Radius: one Guest, LAN-only

A third systemd unit lands on a Guest sized for two — RAM/CPU head-room wants watching (recorded in ADR 0003's consequences). The endpoint is reachable by any host on 10.12.0.0/24; it takes no WAN path and no Edge entry, so nothing fronts it from the internet. The --check --diff gate is partial by nature: the role mutates through command/fetch tasks that --check skips.

Not in this PR

• Not rehearsed on the real PRE Guest. LXC 142 is torn down between rehearsals and was unreachable this session, so the rehearsal above used a disposable systemd container running the role unmodified — a stand-in, not PRE.
• Issue #22's last criterion says the PRE overrides "park on the forgejo-pre branch"; that branch was retired by #28 and the overrides live on main per ADR 0004. This PR follows the live convention, not the literal wording.
• No real reachability evidence from another host on 10.12.0.0/24 — only from the host running the container's bridged interface.

Closes #22

## Summary A new `forgejo_mcp` role and its own play install the pinned forgejo-mcp release as a third **Service** on the `forgejo` Guest. The endpoint is credential-less: it holds no Forgejo token, and in `passthrough` auth a request without its own credential is refused `401`. ```diff ansible/ ├── site.yml (+ play 2: role forgejo_mcp, after the untouched forgejo role) ├── group_vars/forgejo/vars.yml (+ forgejo_mcp_allowed_hosts: 10.12.0.141) ├── host_vars/forgejo-pre.yml (+ forgejo_mcp_allowed_hosts: 10.12.0.142) └── roles/ └── forgejo_mcp/ ├── defaults/main.yml version, port, literal sha256, bind, upstream ├── handlers/main.yml restart on config change └── tasks/main.yml user + tree, checksum-first fetch, unit, 401 probe ``` The shape is [ADR 0003](docs/adr/0003-forgejo-mcp-service-shape-and-endpoint.md) (merged as #43); this PR is the role that honours it: - version, port and the archive's **literal sha256** pinned in defaults — the fetch is checksum-first against that hash, not a remote checksum URL (upstream ships no per-file `.sha256`); - dedicated `forgejo-mcp` service user; systemd unit enabled at boot, restarted on config change; - bound `0.0.0.0:8089` behind an **inventory-scoped** host allowlist; an undeclared `Host` is refused `403`; - `--auth-mode passthrough` passed **explicitly**, and `--allow-operator-token-fallback` deliberately absent; - upstream is the Guest's own loopback (`127.0.0.1:3080`); no Edge entry, certificate, DNS record or router rule. The `forgejo` role is untouched, so its handler-order contract (the runner restart lands after the Forgejo restart) stays intact. ## Evidence Rehearsed on a **systemd Guest stand-in** (a disposable privileged Debian 13 container running `site.yml`'s new play unmodified — not the PRE Guest; see below). The role is exercised for real: systemd, the service user, the checksum-first fetch, the readiness probe. - **Cold run** from nothing (no service user, no ca-certificates): converges, `changed=11`. - **Second run:** `changed=0`. - **Reboot** of the Guest stand-in: `forgejo-mcp.service` comes back `active` on its own, endpoint answers `401`. - **Config change** (port `8089` → `8090`): the unit re-renders, the handler restarts, and the endpoint answers `401` on the new port. The first attempt **failed** here — the handler ran after the readiness probe, so the probe polled a port the old process no longer held. Fixed with `meta: flush_handlers` before the probe; that fix is what this run verifies. - **Credential-less / allowlist, from another host** against the running Service: | request | answer | | --- | --- | | no credential, declared `Host` | `401` | | undeclared `Host` (`evil.example`) | `403` | | declared `Host` on any port | `401` (the bare allowlist matches any port) | - **Checksum-first guard held:** a wrong pinned hash for the identical URL `failed` the fetch rather than installing the archive. - `--check --diff` (the repo's rehearsal gate) passes; `site.yml --syntax-check` passes on **both** inventories; `tofu fmt -recursive -check` clean. Two-axis review (`/code-review`) on the diff; the one substantive finding — `forgejo_mcp_auth_mode` was declared in defaults but never passed to `ExecStart`, so the credential-less guarantee rested on an upstream default while a comment claimed otherwise — is fixed in the second commit. ## Merge Danger **Door:** two-way Reverting is a `git revert`: the role directory and the new play drop out, and the Service stops on the next run. No secret was added (the Service is credential-less), so nothing needs rotating. The one thing that does not revert cleanly by itself is a *pin* mismatch — the role's `forgejo_mcp_forgejo_port` restates the `forgejo` role's `forgejo_http_port` across plays and must move with it. **Blast Radius:** one Guest, LAN-only A third systemd unit lands on a Guest sized for two — RAM/CPU head-room wants watching (recorded in ADR 0003's consequences). The endpoint is reachable by any host on `10.12.0.0/24`; it takes no WAN path and no Edge entry, so nothing fronts it from the internet. The `--check --diff` gate is partial by nature: the role mutates through `command`/`fetch` tasks that `--check` skips. ## Not in this PR - **Not rehearsed on the real PRE Guest.** LXC 142 is torn down between rehearsals and was unreachable this session, so the rehearsal above used a disposable systemd container running the role unmodified — a stand-in, not PRE. - Issue #22's last criterion says the PRE overrides "park on the `forgejo-pre` branch"; that branch was retired by #28 and the overrides live on `main` per ADR 0004. This PR follows the live convention, not the literal wording. - No real reachability evidence from another host on `10.12.0.0/24` — only from the host running the container's bridged interface. Closes #22

pit added 2 commits 2026-10-07 06:11:34 +00:00

forgejo_mcp role: install forgejo-mcp as a Service on the Guest (#22) ... 8ca5bc1a3d

A new role and its own play install the pinned forgejo-mcp release as a third
Service on the `forgejo` Guest, so the `forgejo` role's handler-order contract
stays untouched. The endpoint is credential-less: no Forgejo token is
configured on it, and in `passthrough` auth a request without its own
credential is refused 401.

- Version, port and the archive's sha256 pinned in the role's defaults; the
  fetch is checksum-first against the literal hash (upstream ships no per-file
  .sha256). Bound to 0.0.0.0 on 8089 behind an inventory-scoped host allowlist
  (`10.12.0.141` on Prod, overridden to `10.12.0.142` on PRE); a request whose
  Host is not declared is refused 403. Upstream is the Guest's own loopback.
- systemd unit, enabled at boot, restarted on config change — the role flushes
  the handler before probing the endpoint, so a pin or port change does not
  fail on the old process's port.
- No Edge entry, certificate, DNS record or router rule: LAN-reachable only.
- docs: architecture and the deploy runbook gain the Service; ADR 0003 (merged)
  already fixes its shape.

Rehearsed on a systemd Guest stand-in: cold run converges, a second run is
0 changed, a port change restarts and re-probes cleanly, a reboot brings it
back with the endpoint answering 401, an unlisted Host answers 403, and a wrong
pinned hash fails the fetch.

Refs #22

forgejo_mcp: pass --auth-mode explicitly; clearer unpacked-dir name ... dc33b4eaaf

Review follow-ups on the new role.

- The unit now passes `--auth-mode passthrough` rather than relying on
  upstream's default. The defaults file claimed the credential-less guarantee
  was stated configuration while ExecStart left the flag off, so the 401
  contract rested on an upstream default that a future release could move.
  The unit comment is corrected to match.
- Rename `forgejo_mcp_release_name` to `forgejo_mcp_unpacked_dir`: it names the
  directory the archive unpacks to, next to `forgejo_mcp_archive_name` (the
  tarball), and "release" vs "archive" read ambiguously.
- Section 1's banner now names the input-validation task it also holds.

Re-rehearsed after the change: a config-change run restarts and re-probes
cleanly, a second run is 0 changed, the endpoint still answers 401, the
`--check --diff` gate and both inventories' syntax checks pass.

pit added 1 commit 2026-10-07 06:11:48 +00:00

Merge remote-tracking branch 'origin/main' into hermes/22-install-forgejo-mcp-server 392823c098

pit commented 2026-10-07 06:20:35 +00:00

Author

Owner

Copy link

Code review — main...hermes/22-install-forgejo-mcp-server

Two-axis review against issue #22 (and the feature spec, .scratch/forgejo-mcp-spec.md) and the binding design in ADR 0003. Diff pinned at merge-base b9d6ed0; 3 commits under review (8ca5bc1, dc33b4e, 392823c). PR body carries Closes #22.

Standards

(a) Documented-standard violations (hard)

1. GLOSSARY.md — Service: "Avoid: app, daemon, process." The diff calls the forgejo-mcp Service a "process" at three sites — the exact synonym the glossary bans:

# tasks/main.yml (flush_handlers comment)
+# out, and a config-change deploy would fail on a service that is about to be
+# correct. ... (Measured on a rehearsal: without
+# this, a port change failed the probe with connection-refused.)

# docs/runbooks/0001-deploy-and-rollback.md
+- It reaches Forgejo over the Guest's own loopback, so it must start after
+  Forgejo is up (its startup reachability check exits the process otherwise).
+- A pin or port change … does not fail on a port the old process no longer holds.

Each names the Service itself, not an incidental OS process. Use "Service" (or "instance"). (app.ini is a literal filename, not a breach.)

(b) Baseline smells (all judgement calls)

Duplicated Code — the "allowlist is inventory-scoped" rationale, five times. Nearly verbatim in group_vars/forgejo/vars.yml, defaults/main.yml (L77–83), the tasks/main.yml fail_msg, architecture.md, and the runbook. The repo favours a "why" comment at each use-site, which blunts this — but the four-line block is the same shape each time; one anchor plus a pointer would do.

Duplicated Code / Data Clumps — the archive stem composed twice.

+forgejo_mcp_archive_name: "forgejo-mcp_{{ forgejo_mcp_version }}_linux_amd64.tar.gz"
+forgejo_mcp_unpacked_dir: "forgejo-mcp_{{ forgejo_mcp_version }}_linux_amd64"

One stem var (suffix .tar.gz at the use-site) removes the drift risk.

Clean / conforming (checked): ADR 0003 honoured fully — bind 0.0.0.0, port 8089, host allowlist, passthrough, no token, no operator-token fallback, loopback upstream, no Edge entry; no ADR contradiction to surface. Glossary terms used correctly otherwise (Service, Guest, Edge, LAN-reachable, Prod, PRE, Operator key). Role style matches its sibling (ansible.builtin.* FQCN, lowercase task names, why-comments, idempotence idioms); the separate role/play realises the ADR's handler-order requirement. No publish-manifest gap (both edited docs are already in docs/wiki-pages.yml).

Minor: group_vars/forgejo/vars.yml and host_vars/forgejo-pre.yml still lack a trailing newline — matches their prior state, so consistent, just non-POSIX.

Spec

(a) MISSING or PARTIAL

1. Rehearsal criterion unmet — the biggest gap. #22: "The role is rehearsed on the PRE Guest before it touches Prod." The PR admits stand-in-only, PRE unreachable. Consequently Testing Decisions #1 ("A second playbook run reports 0 changed") and #2's second half ("an mcp client initialize + tools/list round-trip succeeds when the request carries a valid token") are unverified.

2. Client-half documentation absent. Spec, Client half (documented, not provisioned): "Installing the mcp Python package and its mcp.client.streamable_http extra, adding the mcp_servers: entry, and restarting the Agent … is a runbook section." Story 24 repeats it. The runbook gained a server-side section only; grep finds no mcp_servers/streamable_http anywhere. (#22's own AC doesn't name it, so arguably deferred — but the spec's story 24 asks for it.)

3. Capability seam untested (Testing Decisions #3) — but token scope is ticket #24, so correctly out of scope here.

(b) NOT ASKED FOR (scope creep)

• Server-half runbook + docs/architecture.md rewrite — not requested by #22; the asked-for runbook content was the client half (story 24). Defensible repo hygiene, but it documents the wrong half.
• ca-certificates apt task, the assert allowlist validation, explicit --auth-mode passthrough — none requested. All defensible hardening/dependency declaration; not defects.

(c) Looks implemented but WRONG

1. The 401 readiness probe is unproven. AC: a credential-less request is "refused 401". The role asserts status_code: 401 for a POST of {} to /mcp — but that was exercised only against a systemd stand-in, not the real forgejo-mcp binary, which may answer 400/406 to a non-JSON-RPC body before the auth gate.

2. Idempotence rests on an unverified bet. The role claims the unarchive task is idempotent because "the directory persists between runs"; unarchive without creates decides via tar --diff attribute comparison (owner/mtime/mode). AC: "a second playbook run reports 0 changed" — unproven off the stand-in.

3. Branch criterion contradicted. #22: "the PRE overrides park on the forgejo-pre branch." Overrides land on main (ADR 0004 retired the branch). The PR acknowledges the conflict; it is a spec/ADR-0004 conflict, not a code defect.

---

Summary — Standards: 1 hard documented-standard violation (the glossary-banned "process" naming the Service, 3 sites) + 2 judgement-call smells (2 Duplicated Code); worst: the glossary breach. Spec: 3 gaps, 2 scope-creep notes, 3 likely-wrong; worst: the rehearsal criterion left unrun on the real PRE Guest, which also leaves the 401 probe and the 0 changed idempotence unverified.

## Code review — `main...hermes/22-install-forgejo-mcp-server` Two-axis review against issue #22 (and the feature spec, `.scratch/forgejo-mcp-spec.md`) and the binding design in [ADR 0003](docs/adr/0003-forgejo-mcp-service-shape-and-endpoint.md). Diff pinned at merge-base `b9d6ed0`; 3 commits under review (`8ca5bc1`, `dc33b4e`, `392823c`). PR body carries `Closes #22`. ### Standards **(a) Documented-standard violations** (hard) **1. GLOSSARY.md — `Service`: "_Avoid_: app, daemon, process."** The diff calls the forgejo-mcp Service a "process" at three sites — the exact synonym the glossary bans: ``` # tasks/main.yml (flush_handlers comment) +# out, and a config-change deploy would fail on a service that is about to be +# correct. ... (Measured on a rehearsal: without +# this, a port change failed the probe with connection-refused.) ``` ``` # docs/runbooks/0001-deploy-and-rollback.md +- It reaches Forgejo over the Guest's own loopback, so it must start after + Forgejo is up (its startup reachability check exits the process otherwise). +- A pin or port change … does not fail on a port the old process no longer holds. ``` Each names the Service itself, not an incidental OS process. Use "Service" (or "instance"). (`app.ini` is a literal filename, not a breach.) **(b) Baseline smells** (all judgement calls) **Duplicated Code — the "allowlist is inventory-scoped" rationale, five times.** Nearly verbatim in `group_vars/forgejo/vars.yml`, `defaults/main.yml` (L77–83), the `tasks/main.yml` `fail_msg`, `architecture.md`, and the runbook. The repo favours a "why" comment at each use-site, which blunts this — but the four-line block is the same shape each time; one anchor plus a pointer would do. **Duplicated Code / Data Clumps — the archive stem composed twice.** ```yaml +forgejo_mcp_archive_name: "forgejo-mcp_{{ forgejo_mcp_version }}_linux_amd64.tar.gz" +forgejo_mcp_unpacked_dir: "forgejo-mcp_{{ forgejo_mcp_version }}_linux_amd64" ``` One stem var (suffix `.tar.gz` at the use-site) removes the drift risk. **Clean / conforming (checked):** ADR 0003 honoured fully — bind `0.0.0.0`, port `8089`, host allowlist, `passthrough`, no token, no operator-token fallback, loopback upstream, no Edge entry; no ADR contradiction to surface. Glossary terms used correctly otherwise (Service, Guest, Edge, LAN-reachable, Prod, PRE, Operator key). Role style matches its sibling (`ansible.builtin.*` FQCN, lowercase task names, why-comments, idempotence idioms); the separate role/play realises the ADR's handler-order requirement. No publish-manifest gap (both edited docs are already in `docs/wiki-pages.yml`). **Minor:** `group_vars/forgejo/vars.yml` and `host_vars/forgejo-pre.yml` still lack a trailing newline — matches their prior state, so consistent, just non-POSIX. ### Spec **(a) MISSING or PARTIAL** **1. Rehearsal criterion unmet — the biggest gap.** #22: "_The role is rehearsed on the PRE Guest before it touches Prod._" The PR admits stand-in-only, PRE unreachable. Consequently Testing Decisions #1 ("_A second playbook run reports `0 changed`_") and #2's second half ("_an `mcp` client `initialize` + `tools/list` round-trip succeeds when the request carries a valid token_") are unverified. **2. Client-half documentation absent.** Spec, _Client half (documented, not provisioned)_: "Installing the `mcp` Python package and its `mcp.client.streamable_http` extra, adding the `mcp_servers:` entry, and restarting the Agent … is a runbook section." Story 24 repeats it. The runbook gained a **server**-side section only; grep finds no `mcp_servers`/`streamable_http` anywhere. (#22's own AC doesn't name it, so arguably deferred — but the spec's story 24 asks for it.) **3. Capability seam untested** (Testing Decisions #3) — but token scope is ticket #24, so correctly out of scope here. **(b) NOT ASKED FOR (scope creep)** - **Server-half runbook + `docs/architecture.md` rewrite** — not requested by #22; the asked-for runbook content was the client half (story 24). Defensible repo hygiene, but it documents the wrong half. - **`ca-certificates` apt task, the `assert` allowlist validation, explicit `--auth-mode passthrough`** — none requested. All defensible hardening/dependency declaration; not defects. **(c) Looks implemented but WRONG** **1. The `401` readiness probe is unproven.** AC: a credential-less request is "_refused `401`_". The role asserts `status_code: 401` for a `POST` of `{}` to `/mcp` — but that was exercised only against a systemd stand-in, not the real forgejo-mcp binary, which may answer `400`/`406` to a non-JSON-RPC body before the auth gate. **2. Idempotence rests on an unverified bet.** The role claims the `unarchive` task is idempotent because "_the directory persists between runs_"; `unarchive` without `creates` decides via `tar --diff` attribute comparison (owner/mtime/mode). AC: "_a second playbook run reports `0 changed`_" — unproven off the stand-in. **3. Branch criterion contradicted.** #22: "_the PRE overrides park on the `forgejo-pre` branch._" Overrides land on `main` (ADR 0004 retired the branch). The PR acknowledges the conflict; it is a spec/ADR-0004 conflict, not a code defect. --- **Summary** — Standards: 1 hard documented-standard violation (the glossary-banned "process" naming the Service, 3 sites) + 2 judgement-call smells (2 Duplicated Code); worst: the glossary breach. Spec: 3 gaps, 2 scope-creep notes, 3 likely-wrong; worst: the rehearsal criterion left unrun on the real PRE Guest, which also leaves the `401` probe and the `0 changed` idempotence unverified.

pit referenced this pull request from a commit 2026-10-07 06:26:06 +00:00

forgejo_mcp: review fixes — glossary wording, derive unpacked dir, trim repeated rationale

pit added 1 commit 2026-10-07 06:26:06 +00:00

forgejo_mcp: review fixes — glossary wording, derive unpacked dir, trim repeated rationale ... 5a1a1e36c6

Answers the review on PR #48.

- GLOSSARY bans "process" as a synonym for Service; three sites named the
  Service that way. Reworded to "Service" / "the previous instance" in the
  role's flush_handlers comment and the runbook.
- Derive `forgejo_mcp_unpacked_dir` from `forgejo_mcp_archive_name` instead of
  repeating the stem, so the tarball name and the directory it unpacks to
  cannot drift.
- The "allowlist is inventory-scoped" rationale was near-verbatim in five
  places. It now lives once in the role defaults, with the other sites pointing
  at it, and the fail_msg keeps only the actionable half.
- Restore the trailing newline on group_vars/forgejo/vars.yml and
  host_vars/forgejo-pre.yml, and fix the architecture.md sentence the trim
  first mangled.

Rehearsed on the real PRE Guest (LXC 142) this time, not a stand-in: cold run
converges, a second run is 0 changed, the endpoint answers 401 (and 403 to an
undeclared Host) from another host on 10.12.0.0/24 at the pinned port, a port
change re-renders the unit and re-probes cleanly, and a reboot brings the
Service back enabled and answering. PRE was destroyed afterwards; the Prod
Guest is untouched.

pit commented 2026-10-07 06:26:13 +00:00

Author

Owner

Copy link

Addressed in 5a1a1e3. The big one — the rehearsal — is now run for real.

Standards

1. Glossary process → Service (hard). Fixed at all three sites: the role's flush_handlers comment and the two runbook lines. The second runbook line now reads "the previous instance" rather than naming the Service at all. grep -rn process ansible/ is clean.
2. Duplicated "allowlist is inventory-scoped" rationale (×5). Trimmed: the rules now live once in roles/forgejo_mcp/defaults/main.yml; group_vars/forgejo/vars.yml, host_vars/forgejo-pre.yml, the fail_msg and architecture.md each keep a short pointer. The fail_msg keeps only the actionable half (where to set it, to fail early).
3. Archive stem written twice. forgejo_mcp_unpacked_dir is now derived: {{ forgejo_mcp_archive_name | regex_replace('\.tar\.gz$', '') }}, so the tarball name and the directory cannot drift. Verified it still renders forgejo-mcp_3.2.0_linux_amd64.
4. Missing trailing newlines. Restored on both inventory files.

Spec

1. Rehearsal criterion — now met, for real. I stood up the disposable PRE Guest (LXC 142, tofu apply -target=…forgejo_pre), ran site.yml against inventories/pre/hosts from this branch, then destroyed it. Prod was never in the plan (1 to add, 0 to change, 0 to destroy, only forgejo_pre) and is untouched at the end. On the real Guest:
   • cold run converges (changed=30, failed=0);
   • second run 0 changed — Testing Decisions #1, now verified off the stand-in as well;
   • from another host on 10.12.0.0/24 at the pinned port: POST /mcp with a declared Host and no credential → 401; undeclared Host → 403;
   • a port change re-renders the unit, the handler restarts, the probe passes on the new port;
   • after a reboot the Service is enabled/active and answers 401 on its own.
2. 401 probe "may answer 400 to a non-JSON-RPC body" — resolved. It answers 401. The probe ran against the real forgejo-mcp binary on PRE (ok), and the curl above hits the same path with {} and gets 401, so the auth gate does beat body validation.
3. Idempotence "unverified bet" — now observed, not assumed (item 1 above).
4. Client-half documentation — deferred to #23, deliberately. #23 "An Agent round-trips through the MCP endpoint" explicitly owns it: it is in #23's acceptance criteria, and this ticket's criteria (below) do not name it. Adding it here would leave #23's AC unchecked by construction and put the client half in two places.
5. Capability seam — agreed out of scope, it is #24's.
6. Branch criterion contradiction — confirmed; it is a #22/ADR-0004 conflict, not a code defect. Following the live convention (overrides on main).

Pushed back — (b) scope creep

architecture.md is not optional hygiene: its docs/index.md "update when" is "the topology changes (a new component, a moved port…)", and this PR adds a component and a port, so leaving it stale is the breach. The runbook §4 server half is a server section, not the client half — it carries the exact probe to run, which the rehearsal above just exercised. Both stay.

The ca-certificates task, the assert, and explicit --auth-mode were accepted as hardening; --auth-mode in particular closed a real gap — it was declared in defaults but never reached ExecStart.

Addressed in `5a1a1e3`. The big one — the rehearsal — is now run for real. **Standards** 1. **Glossary `process` → `Service` (hard).** Fixed at all three sites: the role's `flush_handlers` comment and the two runbook lines. The second runbook line now reads "the previous instance" rather than naming the Service at all. `grep -rn process ansible/` is clean. 2. **Duplicated "allowlist is inventory-scoped" rationale (×5).** Trimmed: the rules now live once in `roles/forgejo_mcp/defaults/main.yml`; `group_vars/forgejo/vars.yml`, `host_vars/forgejo-pre.yml`, the `fail_msg` and `architecture.md` each keep a short pointer. The `fail_msg` keeps only the actionable half (where to set it, to fail early). 3. **Archive stem written twice.** `forgejo_mcp_unpacked_dir` is now derived: `{{ forgejo_mcp_archive_name | regex_replace('\.tar\.gz$', '') }}`, so the tarball name and the directory cannot drift. Verified it still renders `forgejo-mcp_3.2.0_linux_amd64`. 4. **Missing trailing newlines.** Restored on both inventory files. **Spec** 1. **Rehearsal criterion — now met, for real.** I stood up the disposable PRE Guest (LXC 142, `tofu apply -target=…forgejo_pre`), ran `site.yml` against `inventories/pre/hosts` from this branch, then destroyed it. Prod was never in the plan (`1 to add, 0 to change, 0 to destroy`, only `forgejo_pre`) and is untouched at the end. On the **real** Guest: - cold run converges (`changed=30`, `failed=0`); - **second run `0 changed`** — Testing Decisions #1, now verified off the stand-in as well; - from another host on `10.12.0.0/24` at the pinned port: `POST /mcp` with a declared `Host` and no credential → **`401`**; undeclared `Host` → `403`; - a port change re-renders the unit, the handler restarts, the probe passes on the new port; - after a reboot the Service is `enabled`/`active` and answers `401` on its own. 2. **`401` probe "may answer 400 to a non-JSON-RPC body" — resolved.** It answers `401`. The probe ran against the real forgejo-mcp binary on PRE (`ok`), and the curl above hits the same path with `{}` and gets `401`, so the auth gate does beat body validation. 3. **Idempotence "unverified bet"** — now observed, not assumed (item 1 above). 4. **Client-half documentation — deferred to #23, deliberately.** #23 *"An Agent round-trips through the MCP endpoint"* explicitly owns it: it is in #23's acceptance criteria, and *this* ticket's criteria (below) do not name it. Adding it here would leave #23's AC unchecked by construction and put the client half in two places. 5. **Capability seam** — agreed out of scope, it is #24's. 6. **Branch criterion contradiction** — confirmed; it is a #22/ADR-0004 conflict, not a code defect. Following the live convention (overrides on `main`). **Pushed back — (b) scope creep** `architecture.md` is not optional hygiene: its `docs/index.md` "update when" is *"the topology changes (a new component, a moved port…)"*, and this PR adds a component and a port, so leaving it stale is the breach. The runbook §4 server half is a **server** section, not the client half — it carries the exact probe to run, which the rehearsal above just exercised. Both stay. The `ca-certificates` task, the `assert`, and explicit `--auth-mode` were accepted as hardening; `--auth-mode` in particular closed a real gap — it was declared in defaults but never reached `ExecStart`.

pit merged commit 7dcf5e009f into main 2026-10-07 06:48:16 +00:00

pit deleted branch hermes/22-install-forgejo-mcp-server 2026-10-07 06:48:16 +00:00

pit referenced this pull request from a commit 2026-10-07 06:48:17 +00:00

Install forgejo-mcp as a Service on the forgejo Guest (#22) (#48)

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

Subscribe

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!48

No description provided.