soul.demarkus.io/patterns.md/v18 draft reader meta

Patterns & Conventions

What I've learned about how we write code in this project.

Go Style

Loop Idiom

Always range N for integer loops. Never for i := 0; i < N; i++. This is a hard rule Fritz set early.

Table-Driven Tests

Every test file uses t.Run with named subtests. The pattern:

tests := []struct {
    name string
    // inputs...
    // expected...
}{
    {"descriptive name", ...},
}
for _, tt := range tests {
    t.Run(tt.name, func(t *testing.T) {
        // test body
    })
}

Mock Streams for Handlers

Handler tests construct a bytes.Buffer with a raw request, pass it as a stream, and read the response. No QUIC, no network, fast tests.

Interface-Based Mocking for MCP Handlers

The MCP handler uses a markClient interface instead of *fetch.Client directly. This enables testing handler logic (like version auto-resolution) without a QUIC connection. Stub implementations in tests configure only the methods under test.

t.TempDir() for Fixtures

Never create test fixtures in the working directory. Always t.TempDir(): Go cleans it up automatically.

Build & Test

make all        # Build everything
make test       # Run all tests
make fmt        # Format
make vet        # Vet
make lint       # Run golangci-lint

# Single module / single test
cd server && go test -run TestHandleFetch/path_traversal_blocked ./internal/handler/

# Dev server
./server/bin/demarkus-server -root ./docs/site

IMPORTANT: Build Output Rule

Never run bare go build ./cmd/<name>/; this drops binaries in the current directory, polluting the repo. Always use one of:

  • make client / make server / make all; preferred
  • go build -o bin/<name> ./cmd/<name>/: if building manually
  • go vet ./cmd/<name>/: if you just need to check compilation without producing a binary

Binaries belong in bin/ directories only. Stray binaries in source directories get accidentally committed.

After completing a task: Run bash pre-commit.sh to format, vet, and lint all modules before committing.

Development Workflow

Pre-Commit

Run bash pre-commit.sh before committing. Formats, vets, and lints all modules.

Conventional Commits

Module-scoped: feat(server): description, fix(client): description. This drives auto-versioning and release tagging (server/v0.1.0, etc.).

Git Commits

Fritz handles all commits himself. Never commit on his behalf: just prepare the changes and let him know what's ready.

CI/CD

Tags: server/v0.1.0, client/v0.1.0, protocol/v0.1.0. Push to main triggers auto-release. CI runs test, vet, and golangci-lint for each module.

Architecture (Quick Reference)

Go monorepo, four modules with local replace directives:

  • protocol/: wire format types, parsing, serialization (no network code)
  • server/: QUIC server (depends on protocol)
  • client/: CLI, TUI, MCP server (depends on protocol)
  • tools/: dev utilities

Protocol constants: port 6309, ALPN "mark", scheme mark://.

See Architecture for full details.

Core Invariants

  • Version immutability: every write creates a new version, published versions are permanent
  • Security: no tracking, no telemetry, encrypted transport, capability-based auth
  • Backend parity: every store backend (file, postgres, future ones) must be observably identical. Parity is system-level, not implementation-level: the server/internal/storetest conformance suite is the contract, every backend runs it unchanged, and divergence is a protocol bug. New store-observable behavior lands in the conformance suite first, then in every backend, or it does not ship.

Philosophy

Small and Incremental

Every change should be the smallest working increment. Get something tested and working before moving on. Don't batch up large changes.

Robustness First

Handle the error. Test the edge case. Make it correct before making it elegant.

Simplest Solution

Short functions, clear names, obvious flow. If I find myself writing a comment to explain what code does, the code should be rewritten to not need the comment. Comments explain why, not what.

No Over-Engineering

Don't add features beyond what's asked. Don't refactor surroundings while fixing a bug. Don't add abstractions for one-time operations. Three similar lines are better than a premature helper function.

What I've Learned About Working With Fritz

Fritz values directness. Short answers over long explanations. Working code over architecture astronautics. He'll push back on unnecessary complexity and he's usually right when he does. The best sessions are when we move fast through small, clean changes; each one tested, each one committed. Momentum matters.

GitHub Pages

The pages branch of this repo is the GitHub Pages site. Documentation changes that should be published to the website need to be pushed to the pages branch, not main.

Protocol Patterns

MCP APPEND: Auto-Resolved Versioning

The mark_append MCP tool now auto-resolves expected_version when omitted or 0. Agents no longer need to manually call VERSIONS before appending; the tool handles it internally:

  1. Agent calls mark_append with just url and body
  2. Tool internally calls VERSIONS → parses current from frontmatter
  3. Tool calls APPEND with the resolved version

Optimistic concurrency is still enforced: the server always receives an expected_version. The difference is who provides it:

  • Omitted/0: tool auto-resolves via VERSIONS (one extra round trip, but invisible to the agent)
  • Explicit: agent passes it directly (no extra round trip, useful when the agent already knows the version from a prior fetch)

This does NOT apply to mark_publish: publish requires the agent to have read the document first, so it should always know the version.

APPEND: Getting the Latest Version (CLI/Library)

For non-MCP clients that use APPEND directly, follow this pattern to get the version without fetching the full document:

  1. VERSIONS /path → parse frontmatter, extract current field
  2. APPEND with expected_version: current

VERSIONS response always includes current in its frontmatter metadata.

Example:

VERSIONS /journal.md
→ frontmatter: { "status": "ok", "current": "5", "total": "5", ... }
→ APPEND /journal.md with expected_version=5

LOOKUP: Tag and Rate Documents on Publish

LOOKUP (the catalog verb) ranks results by author-declared tags and importance, matched against tags + title. For soul/knowledge docs to be findable and well-ranked, set these as publisher metadata on PUBLISH:

  • tags: comma-separated subject labels, e.g. tags: lookup,catalog,verb. This is the primary match target: an untagged doc is only found by words in its title.
  • importance: a float in [0,1] (default 0.5 when absent). Use it sparingly to float genuinely critical docs (index hubs, architecture) above routine notes. It is a bounded prior, not an override: it never floats an unmatched doc onto the results.

How to set them:

  • CLI: demarkus -X PUBLISH -meta tags=lookup,catalog -meta importance=0.9 mark://host/doc.md (the -meta key=value flag is repeatable).
  • MCP mark_publish and broker mark_publish: pass a metadata object, e.g. {"tags": "lookup,catalog", "importance": 0.9}.

The server interprets exactly tags, importance, and title (declared → first H1 → basename); all other metadata stays opaque and is reachable only via LOOKUP's filter axis. See /plans/lookup-verb.md.

Tech Debt

graphstore Save() lock scope

Save() holds RLock across JSON marshal + disk I/O. Fine while Save is only called from CrawlAndPersist (infrequent, user-triggered). If concurrent or periodic saves are added later, snapshot state under lock and write outside it to avoid blocking Merge() during I/O. Two concurrent Save() calls would also race on the .tmp file. Comment added in code at client/internal/graphstore/store.go.

No Co-Authored-By in Commits

Never add Co-Authored-By: Claude ... or any AI co-author trailer to commit messages. Fritz is the sole author. This is a strict rule.

Knowledge System Join URL (canonical)

/knowledge-join takes the full https:// URL of the broker's MCP gateway, and the helper script (plugins/claude-code-knowledge/scripts/knowledge-join.sh) hard-requires the https:// scheme; a scheme-less host fails validation outright.

For the live system the join target is the apex broker host, https://knowledge.demarkus.io. It is not a broker.knowledge.* subdomain (that form was a documentation invention and is wrong). The slug derives from the first DNS label, so https://knowledge.demarkus.io registers as the knowledge MCP server. The generic placeholder form is https://knowledge.<org>.

This was corrected on the GitHub Pages /scenarios/knowledge-system/ page after it shipped with the wrong URL. When documenting the join flow anywhere, verify against the command doc + script, not memory.

Shell installer: never swallow failures

install.sh is the one place where a swallowed error becomes a silently broken deployment. PR #263 surfaced this five times across five review rounds (ufw allow, join-URL generation, uninstall rm, OIDC secret read, token migration mv). The rule for every state-changing shell operation:

  • Check the result. A bare cmd or cmd || true that mutates state must either surface the failure (log_warn + a failure counter) or abort (log_error; exit 1). || true is only for genuinely idempotent probes (systemctl stop on an already-stopped unit), never for removals or writes.
  • Verify destructive ops, don't trust exit codes. Uninstall's remove_path rm's then test -e to confirm the target is gone, incrementing an error counter; the function reports "completed with N errors" and exits non-zero rather than logging "complete" over a partial teardown.
  • Never reassign a path before its migration succeeds. The sticky-tokens block pointed the server at the new tokens path only after a checked mv; an unchecked move would strand the server reading a missing file.
  • Validate untrusted inputs before reading. A secret file gets -f (regular file, not a FIFO that blocks head) plus a stat-based mode/owner check (reject group/world-readable, reject foreign owner) before it is read.
  • Test seams are gated. DEMARKUS_TEST_SYSTEMD_DIR is honored only under DEMARKUS_INSTALL_TESTMODE=1; a stray production override can't misplace units.

Verification without a Linux box: extract a function with awk '/^name\(\)/,/^\}$/', stub the privileged ops (systemctl, useradd, chown), point the dir constants at a tmpdir, and assert on the generated artifacts.

Lessons from the #288 session (2026-08-11/12)

Grep the SPEC before designing protocol behavior

The flat-file fix went through two designs (plugin-side adoption, then store-side migration) before discovering SPEC 9.8 already mandated the opposite behavior. For anything touching protocol semantics, search SPEC.md for the behavior first: the spec may have already decided, or it may need amending in the same PR (amending is legitimate; that is what happened, "flat files are not documents"). A related win: redefining semantics can dissolve release-ordering coupling entirely; declaring flat files non-documents removed the store-fix-first constraint between two PRs that otherwise needed lock-step releases.

Addressing PR comments: fetch review bodies, not just inline comments

CodeRabbit parks "outside diff range" findings in the review body (pulls/N/reviews), not the inline comment API (pulls/N/comments). Fetching only inline comments silently misses them (happened twice this session). Always pull both, and diff comment IDs against the already-addressed set to find what is new.

Version-pin merge conflicts resolve to main plus one patch

The bump workflow releases race feature PRs, so plugin version fields conflict routinely. Resolution rule: take main's file and bump the version one patch above main's value; never keep the branch's stale number.

Comment length is now enforced mechanically

scripts/check-comment-length.sh runs in pre-commit: any // block over 3 lines touched by the change (merge-base with main, working tree included) fails. Package docs, //go: directives, and //nolint are exempt; the 853 pre-existing long blocks are grandfathered until touched. Write comments to the limit up front instead of trimming after review.

trail
  1. soul.demarkus.io v18