CMS ↔ Forgejo build log — SAMA tensions encountered

A diary of moments where building the self-hosted CMS-with-real-git-commits made the SAMA convention either help us instantly or fail to have an opinion. Written as we built [c14 commitFile + c21 admin-fork wire-in], not after the fact.

Tension 1 — c14 has no sibling test, by design

Where it bit: I added commitFile() and getFileSha() to c14_forgejo.ts. Those are mutations against an external service. SAMA's Modeled rule says "every behaviour has a test file as its sibling". But c14_forgejo.test.ts would either need a real Forgejo (turns the unit suite into an integration suite) or a mocked fetch global (couples the test to implementation details, not behaviour).

What I did: Split the work. The pure parts — message format, noreply email shape — moved into c31_forgejo_commit.ts with a real sibling test c31_forgejo_commit.test.ts. The c14 wrapper itself stays untested at unit level.

Candidate SAMA refinement: Modeled needs a sub-rule for c14:

A c14_*.ts file is exempt from the sibling-test rule when its entire body is HTTP request construction + response parsing. Its coverage comes from (a) c31 sibling tests for any pure helpers, and (b) integration tests separately marked (*.contract.test.ts or e2e Playwright). Without this carve-out, the rule pushes I/O into mocks, which test the mock more than the I/O.

Tension 2 — failure semantics for cross-boundary mutations

Where it bit: Admin POST does two writes — Forgejo commit AND local filesystem. SAMA prescribes which layer (c14 for HTTP, c21 for compositions, c13 for the SQLite proposal row), but is silent on what order and how to roll back when one succeeds and the next fails. There's no SAMA letter for "transactional intent".

What I did: Added a per-failure-mode discriminated union (c14_forgejo.ts ForgejoCommitOutcome) so the handler can render a useful error per kind: conflict | not_found | auth | network | other. Chose Forgejo-first / FS-second deliberately — Forgejo's contents API is the synchronous source of truth, FS is a derived view. Documented the matrix in cms-forgejo-plan.md.

Candidate SAMA refinement: Add a doctrine — not a layer letter, but a discipline:

Boundary contracts. Any function that crosses a process or network boundary returns a discriminated-union outcome, never an exception, and never a bare boolean. Failure kinds are part of the type, the renderer per kind is one switch in c51.

This isn't strictly a SAMA layer rule — it's an idiom. But the absence of it made me reach for try/catch in the handler, which is the wrong layer for "what does conflict mean to the user".

Tension 3 — types-with-data-source vs types-with-pure-logic

Where it bit: ForgejoCommitOutcome lives in c14_forgejo.ts. CommitMessageInput lives in c31_forgejo_commit.ts. Both describe "a Forgejo commit" — but one is the I/O surface, the other is the input we shape. The c21 handler imports from both. Felt wrong but also unavoidable.

What I did: Followed the existing codebase pattern — types travel with the data source (c13_database.ts owns ProposalRow, c14_forgejo.ts owns the response shape, c31 owns the inputs we parse). Resisted the urge to make a c31_forgejo_types.ts barrel.

SAMA verdict: No refinement needed. The current pattern is internally consistent — types live with whoever first defines them, imports are explicit. A barrel would have been less honest. SAMA's "no barrel re-exports" rule already covers this case if you take its spirit seriously.

Tension 4 — schema migration without an IF NOT EXISTS

Where it bit: Added a forgejo_commit_sha column to the existing proposals table. SQLite has no ALTER TABLE … ADD COLUMN IF NOT EXISTS. The pre-existing pattern in c13_database.ts is CREATE TABLE IF NOT EXISTS … at startup — purely additive, no migrations. This is the first time we needed to evolve a table.

What I did: Read PRAGMA table_info(proposals), check column names, only ALTER if the column is missing. Idempotent across container restarts and across rebuilds with old data volumes.

Candidate SAMA refinement: c13 layer needs a discipline note:

Migration is additive. Schema changes go in a migrations block of getDb() after the CREATE TABLE IF NOT EXISTS block, guarded by a PRAGMA probe (or sqlite_master query). Never destructive. Never renames. If you need a rename, it's two migrations: add the new column, deprecate the old one.

This is a one-liner addition to the Architecture page (c13's contract).

Tension 5 — the deploy script is now lying

Where it bit: deploy-tdd-md.sh rsyncs the dev working tree to p620 and rebuilds the image. Now that admin edits commit to git.tdd.md, the next deploy will overwrite those commits with whatever's in dev's working tree. The container is briefly the canonical source between admin save and next deploy. Then dev's working tree wins.

What I did: Documented this as out-of-scope in cms-forgejo-plan.md. Surfaced it in the applied-live page copy ("Until then, sync your local working tree with git pull before the next deploy-tdd-md.sh"). Did not fix the deploy script in this PR — that's a workflow decision, not a code change.

Candidate SAMA refinement: None — this is operational, not a SAMA-layer issue. But it does point at a missing piece in the project's story: there's no diagram of "where the truth lives at each second of a request → deploy → restart cycle". A state-diagram in plan.md or a guides/ page would help.

Tension 6 — same-process self-reference

Where it bit: c21 handler calls c14 (Forgejo PUT). Forgejo lives on the same physical box (host.containers.internal). When it returns success, c21 also writes the same file to the local FS. We're now writing the same bytes twice through different paths, and they could disagree if Forgejo did something weird (LF/CRLF, trailing newline normalisation). SAMA doesn't tell us where to verify they agree.

What I did: Trusted Forgejo to round-trip UTF-8 verbatim. Added no read-after-write check. If this becomes a source of bugs we'd add a c32 helper verifyCommittedContent(sha, expected) that re-reads via getFileSha + a contents fetch.

SAMA verdict: Doesn't need a layer rule. Add to the c14 doctrine note: "external services own their own normalisation; if you care about exact bytes, verify via a follow-up read".

Tension 7 — SAMA doesn't tell you to read the API docs first

Where it bit: I assumed Forgejo's contents API was upsert-on-PUT like GitHub. Wrong. Forgejo requires POST for create (no SHA), PUT for update (SHA mandatory). First admin POST returned a 422 "[SHA]: Required" we then surfaced as "other" failure.

What I did: Branch on priorSha === null in c14_forgejo.commitFile — POST when null, PUT when set. Two-line fix.

SAMA verdict: Not a SAMA problem. SAMA prescribes structure, not external-service semantics. The c14 layer is the right place for this knowledge to live (and it does, in the wrapper). The lesson is generic: c14 contracts must encode every quirk of the service they wrap, because no other layer is allowed to know.

Tension 8 — HTTP status codes through a CDN

Where it bit: I returned 502 Bad Gateway for "Forgejo is unreachable" failures. Cloudflare (our CDN front) replaced our HTML body with its own 502 error page template, so the admin saw a Cloudflare error instead of our diagnostic. The handler did exactly the right thing in HTTP-purist terms; the user-visible result was wrong.

What I did: Switched to 200 OK for non-conflict commit failures (kept 409 for conflict because that's actionable client- side and CDNs don't intercept 4xx). The HTML body carries the error semantics; the status code is just for routing.

Candidate SAMA refinement: Add a c21 doctrine note:

When the site sits behind a CDN that intercepts 5xx responses, render handler-generated error pages with 200 OK (or with a 4xx the CDN passes through). Never let a synthetic 5xx compete with a real upstream 5xx for the user's attention.

This is operational/CDN-specific so it might not belong in SAMA proper. A guides/cloudflare-and-handlers.md page would be more honest than expanding SAMA itself.

Tension 9 — "we don't depend on Forgejo" required actually proving it

Where it bit: The previous iteration committed via Forgejo's HTTP contents API. Operationally fine, but the system did depend on Forgejo being reachable. To prove genuine independence we replaced the Forgejo write-path with a local bare repo + git plumbing inside the container.

What I did: Added c14_git.ts which shells out to git against /app/repo (a bind-mount of ~/repos/tdd.md.git on p620). Plumbing chain per commit: hash-object → read-tree (temp index) → update-index → write-tree → commit-tree → update-ref with the parent SHA as the expected old-value for free optimistic concurrency. The pure parts (commit metadata format, ls-tree output, log-format string) live in c31_git_parse.ts with 11 sibling tests.

The proof: Two Playwright tests:

1. git-native-proof.spec.ts saves a marker via the editor, then SSH's into p620 and reads the bare repo HEAD — must equal the SHA the editor reported. Confirmed: bare repo advanced (bb9c023), Forgejo HEAD stayed at 526fa16.
2. git-native-forgejo-down.spec.ts stops forgejo.service via systemd, performs an admin edit, and verifies the commit lands. The test passes only if Forgejo is genuinely off the edit path. Confirmed: commit eb1d791 landed while Forgejo was returning HTTP 000.

SAMA verdict: No new violations, and the layer hygiene actually stayed clean because the Modeled exemption from tension 1 was already the pattern — c14_git is a shell-out wrapper, c31_git_parse holds the pure helpers with sibling tests. We applied the rule that emerged from the previous build to the next layer that needed it. That's the discipline working.

Tension 10 — naming drift after a refactor

Where it bit: c31_forgejo_commit.ts no longer talks about Forgejo — it owns commit-message format and noreply email shape, which c14_git uses just as well. Filename was a lie.

What I did: Renamed to c31_commit_meta.ts (+ .test.ts). One-import update in c21_handlers_edit.ts. SAMA's "the number is the layer; the layer is the contract" doctrine doesn't strictly require accurate names — it requires accurate prefixes — but the spirit is that file names communicate intent. Drift after refactor is a small but real form of debt.

Candidate SAMA refinement: Note in the Atomic doctrine that when a file's contents diverge from its name during a refactor, a rename is part of the refactor, not optional polish.

Summary — what would I add to SAMA after this PR?

Two concrete additions:

1. Modeled exemption for I/O-only c14 files. Sibling-test rule carves out a path: pure helpers extracted to c31, the I/O wrapper itself can stand alone with integration coverage.
2. Boundary-contract idiom. Cross-process functions return a typed discriminated union, not a thrown exception. The renderer layer (c51) handles each kind explicitly.

Two doctrine notes (not new letters):

3. c13 migrations are additive + probe-guarded. Never destructive in-place, never renames.
4. Source-of-truth state diagram. Document where the canonical bytes live at each step of write → commit → deploy → restart so future contributors don't have to reverse-engineer it.

The four SAMA letters (Sorted, Architecture, Modeled, Atomic) held up. They don't cover every situation — the gaps above are real — but they didn't force us into bad shapes either. The discipline held; the doctrine needs a few footnotes.