# 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](src/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](src/c31_forgejo_commit.ts)
with a real sibling test [c31_forgejo_commit.test.ts](src/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](src/c14_forgejo.ts)) 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](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](src/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](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](src/c14_forgejo.ts) — 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](src/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](src/c31_git_parse.ts) with 11 sibling tests.

**The proof:** Two Playwright tests:
1. [git-native-proof.spec.ts](e2e/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](e2e/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.