Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 63 additions & 0 deletions canon/constraints/bash-test-rig-assignment-chain-discipline.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
---
uri: klappy://canon/constraints/bash-test-rig-assignment-chain-discipline
title: "Bash Test Rigs: Assignment Chains Need Explicit Newlines"
audience: canon
exposure: nav
tier: 2
voice: neutral
stability: stable
tags: ["constraint", "bash", "shell", "tests", "ci", "epoch-8"]
epoch: E0008
date: 2026-04-26
derives_from:
- "canon/principles/ritual-is-a-smell.md"
governs: "Authoring discipline for shell-script test rigs in this codebase"
---

# Bash Test Rigs: Assignment Chains Need Explicit Newlines

> Bash silently concatenates two assignments separated by missing whitespace. The result parses, runs, and produces wrong values. Tests against the wrong value pass or fail nondeterministically depending on what the prior test left behind. Authors lose hours.

## Summary

Consider this pattern:

```bash
RAW=$(curl -sf "$URL/mcp" -X POST \
-H "Content-Type: application/json" \
-d '...')RESULT=$(extract_json "$RAW")
```

The intent is two statements on two lines. The bug is the dropped newline between `)` and `RESULT=`. Bash parses this as a single command-substitution statement assigning to `RAW`, with the literal characters `RESULT=$(extract_json "$RAW")` becoming part of `RAW`'s right-hand side. The actual `RESULT` variable is never assigned — it inherits whatever value the prior test set, or remains empty.

No syntax error. No runtime error. Just a silently wrong test that asserts against last-test-leftover state.

## Detection

The `bash -n` syntax check passes — the construct is valid bash. Detection requires either (a) reading the script with attention to line boundaries after every closing `)` of a command substitution, or (b) running shellcheck (which catches some related cases but not all).

The cheapest preventive: between any two assignments in a test, leave a blank line. Visual whitespace as discipline. The `bash -n` check still passes; the pattern is now legible to humans.

## When This Matters

Any shell-script test rig in this codebase, especially `tests/cloudflare-production.test.sh`. The pattern is most dangerous in tests that are structured as repeated `RAW=$(curl); RESULT=$(extract_json "$RAW"); INNER=$(echo "$RESULT" | python3 -c ...)` sequences — exactly the structure of all the smoke tests for new MCP actions.

## Origin

Surfaced in klappy/oddkit#140 (PR-2.1 of the link-rot-elimination campaign). When test 14g.2 (supersession-walk smoke) was added, the boundary between its closing `)` and test 14h's `RESULT=$(...)` lost a newline during an `str_replace` operation. The `RAW` variable for test 14h was never set; test 14h's assertion ran against the response from test 14g.2, which happened to be a `FOUND` response. Test 14h asserts `NOT_FOUND`, so it failed — but the failure message showed the URI from test 14g.2, which was confusing to debug because the reported URI was not the URI the test was supposedly calling.

Detection cost: the failure was caught by post-merge CI on klappy/oddkit#140 because preview testing exercised it. The fix was a one-character newline insertion, but the diagnostic time was non-trivial.

## What This Demands

For shell-script test rig authors:

1. After any closing `)` of a command substitution, the next character must be a newline (not another assignment, not a semicolon, not a space-then-assignment).
2. When adding a new test by copy-pasting an existing block and modifying, run `bash -n script.sh` AND visually verify the diff has no joined lines.
3. Prefer leaving a blank line between distinct test blocks. Visual separation makes joined-line bugs visible.

Future improvement: a CI lint rule that flags `\)\w*=` (close-paren immediately followed by an assignment) in `.sh` files would catch this mechanically. Defer per Use Only What Hurts; if the bug recurs, lint graduates from defer to active.

## See Also

- [Ritual Is a Smell](klappy://canon/principles/ritual-is-a-smell) — visual blank lines between tests are a low-cost pattern; CI lint is the durable fix when ritual breaks down
51 changes: 51 additions & 0 deletions canon/constraints/oddkit-action-registration-completeness.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
---
uri: klappy://canon/constraints/oddkit-action-registration-completeness
title: "Adding an oddkit Action: Update Both Switch and VALID_ACTIONS"
audience: canon
exposure: nav
tier: 2
voice: neutral
stability: stable
tags: ["constraint", "oddkit", "actions", "vodka", "registration", "epoch-8"]
epoch: E0008
date: 2026-04-26
derives_from:
- "canon/principles/vodka-architecture.md"
governs: "Process for adding a new MCP action surface to klappy/oddkit"
---

# Adding an oddkit Action: Update Both Switch and VALID_ACTIONS

> A new MCP action in oddkit must be registered in two places, not one. Wiring only the dispatch switch ships an action that the validator rejects before the dispatch is reached. Smoke tests pass locally (where validation may be stubbed), production calls fail.

## Summary

`workers/src/orchestrate.ts` carries two collaborating but separate registration sites:

1. **The dispatch switch in `handleUnifiedAction`.** Maps `action: "<name>"` to the implementation function (`runX`).
2. **The `VALID_ACTIONS` array.** Used by the input validator to reject unknown actions before dispatch is even attempted.

Both must include the new action's name. Updating only the switch is the most common authoring mistake — the action is reachable in the code, but the validator returns `Unknown action` to every caller because the name isn't in the allowlist.

## Detection

Surface symptom: callers see `Unknown action: <name>. Valid actions: [list without yours]` and the smoke test that exercises the new tool fails on first invocation.

Mechanical detection: the audit landing in PR-2.3 does not catch this — it's an internal-consistency check inside the worker, not a canon-content check. Until a typecheck-time invariant exists (e.g., a single source-of-truth table that produces both the dispatch handler and the `VALID_ACTIONS` array), this is a process-level constraint enforced by reviewer attention and the PR template.

## Origin

Surfaced in klappy/oddkit#140 (PR-2.1 of the link-rot-elimination campaign). Initial implementation registered `oddkit_resolve` in the dispatch switch but not in `VALID_ACTIONS`. Cursor Bugbot caught it as one of three bugs on the initial commit. Without the catch, the resolver would have shipped to prod with `Unknown action: resolve` as the response to every call, and the smoke tests in the same PR would have failed on first invocation.

## What This Demands

When opening any PR that adds a new oddkit action, the PR description must state: "This action is registered in (a) the dispatch switch and (b) the `VALID_ACTIONS` array." Reviewers verify both. The PR template (when one exists) should include this as a checklist item.

## Future Work

A typecheck-time invariant would eliminate the need for this constraint as a process control. Possible shape: replace the dispatch switch with a `Record<ActionName, Handler>` plus `const VALID_ACTIONS = Object.keys(handlers)`. Defer per Use Only What Hurts — if this bug recurs, the typecheck-time invariant graduates from defer to active.

## See Also

- [Vodka Architecture](klappy://canon/principles/vodka-architecture) — the discipline this constraint protects (the action surface is the vodka-thin contract; missing entries break the contract silently)
- [Release Validation Gate](klappy://canon/constraints/release-validation-gate) — the process control that caught this (Cursor Bugbot)
82 changes: 82 additions & 0 deletions canon/constraints/superseded-by-shape-normalization.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
---
uri: klappy://canon/constraints/superseded-by-shape-normalization
title: "superseded_by Values: Three Shapes Allowed, One Resolution Algorithm"
audience: canon
exposure: nav
tier: 2
voice: neutral
stability: stable
tags: ["constraint", "supersession", "frontmatter", "resolver", "vodka", "epoch-8"]
epoch: E0008
date: 2026-04-26
derives_from:
- "canon/methods/supersession.md"
- "canon/principles/identity-resolved-by-protocol.md"
- "canon/principles/vodka-architecture.md"
governs: "How tools that read superseded_by frontmatter normalize across authoring conventions"
---

# superseded_by Values: Three Shapes Allowed, One Resolution Algorithm

> Canon authors write `superseded_by` in three different shapes. Tools that read that field MUST normalize across all three; pushing the work to authors is the same anti-pattern this canon was built to solve.

## Summary

The `superseded_by` frontmatter field appears in canon documents in three shapes:

1. **Full canonical URI**: `superseded_by: "klappy://canon/x/y"`
2. **Repo-relative path with `.md`**: `superseded_by: "canon/x/y.md"`
3. **Repo-relative path without `.md`**: `superseded_by: "canon/x/y"`

All three are valid authoring conventions. Tools that resolve supersession (the resolver, the audit, future consumers) normalize across them. The normalization algorithm:

1. Try the value as a direct URI lookup. Hit → done.
2. Try the value as a direct path lookup. Hit → done.
3. If the value lacks `.md` and a path lookup with `.md` appended hits, use that.
4. If the value is not a `klappy://` URI, try `klappy://` + value-with-`.md`-stripped. Hit → done.
5. Miss after all four → treat as dangling reference.

Cycle detection MUST key on the canonical URI of each entry, not on the raw `superseded_by` string. Otherwise a chain that uses URI form in one step and path form in the next will not deduplicate.

## Why Three Shapes Coexist

Canon predates `oddkit_resolve`. Before the resolver shipped, supersession was a metadata convention readers parsed by hand or with ad-hoc scripts; "what shape goes in `superseded_by`" was not enforced and authors converged on whatever they typed first. The shapes accreted naturally; mass-rewriting them all to one shape is busywork that doesn't fix any consumer-visible problem.

The Vodka rule applies: the resolver normalizes; consumers and authors don't. Pushing this work outward repeats the link-rot anti-pattern (delegating identity-vs-location resolution to N consumers).

## When This Matters

Any tool that reads `superseded_by` and acts on its value:

- `oddkit_resolve` — authoritative; v0.25.0 ships the normalization
- `oddkit_audit` — relies on `oddkit_resolve` (or its inlined equivalent) for resolution
- Future tools that traverse supersession chains
- Any external consumer that walks `superseded_by` itself instead of calling the resolver

External consumers that walk `superseded_by` themselves are violating `klappy://canon/principles/identity-resolved-by-protocol`. The fix is to call `oddkit_resolve`, not to add a fourth shape-handler.

## Detection

Surface symptom: a `superseded_by` value pointing at a real successor returns `NOT_FOUND` or a truncated chain. The resolver's `warning` field on the response surfaces "X not in the index" — that warning is a hint that either (a) the resolver's normalization is missing a shape, or (b) the value genuinely points at a phantom.

When in doubt, query the index for the literal `superseded_by` string in both URI form and path form. If either exists, the resolver is missing a normalization; if neither, the canon entry is broken.

## Origin

Surfaced in klappy/oddkit#140 (PR-2.1 of the link-rot-elimination campaign). The initial resolver implementation looked up `superseded_by` strictly as a URI. The independent Sonnet 4.6 validator dispatched per release-validation-gate found that `klappy://docs/oddkit/proactive/dolche-vocabulary` had `superseded_by: "canon/definitions/dolcheo-vocabulary.md"` (path form, with `.md`). The terminus existed in the index as `klappy://canon/definitions/dolcheo-vocabulary` and resolved directly, but the chain walk could not reach it. Validator BLOCKED the PR. Fix landed in v0.25.0 as a normalization helper covering all three shapes.

## What This Demands

For tools that read `superseded_by`: normalize across all three shapes per the algorithm above. Use the canonical URI for cycle detection.

For canon authors: any of the three shapes is acceptable. Write what reads naturally. Don't mass-rewrite for consistency.

For new consumers: don't walk `superseded_by` directly. Call `oddkit_resolve`.

## See Also

- [Identity Is Resolved By The Protocol](klappy://canon/principles/identity-resolved-by-protocol) — the principle this constraint operationalizes
- [Supersession](klappy://canon/methods/supersession) — the metadata model
- [Vodka Architecture](klappy://canon/principles/vodka-architecture) — why the resolver carries normalization (thin layer absorbs the work; consumers stay simple)
- [Release Validation Gate](klappy://canon/constraints/release-validation-gate) — the gate that surfaced this
- [oddkit_resolve spec](klappy://docs/oddkit/specs/oddkit-resolve) — the resolver implementation
4 changes: 3 additions & 1 deletion docs/oddkit/specs/oddkit-audit.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ When terminological drift, projection staleness, or epoch gaps cause concrete pa
}
```

- `scope.paths` — repo-relative path prefixes. Default: full repo excluding `docs/archive/`.
- `scope.paths` — repo-relative path prefixes. **Default: `["writings/"]`** — the actual link-rot pain surface. Other paths (`canon/`, `odd/`, `docs/`) are explicit opt-in. v2.1 amendment: cold-cache fetching ~560 files exceeded a typical 120s CI curl budget; the smaller default ships in v0.26.0. Reversal is one-line if a real consumer demonstrates wider need.
- `scope.since_commit` — limit findings to files changed since this ref. Default: full audit. PR-mode CI sets this to merge-base.

No `checks` field. There's one check; it always runs. No `severity_floor`. Workflow decides what to fail on.
Expand Down Expand Up @@ -176,3 +176,5 @@ Net-new action. No existing callers.
## Origin

Drafted on 2026-04-26 alongside `oddkit_resolve` (DRAFT v4). v1 of this spec proposed four checks (dead-reference + terminological-drift + projection-staleness + epoch-gaps) plus a deprecated-terms registry, epoch-completeness rules, and an `audit_allow:` frontmatter field. v2 (this revision) cut to one check and one allowlist mechanism per the operator's Vodka discipline. The other three checks and supporting registries moved to the deferred-concerns ledger with explicit revisit triggers.

**v2.1 amendment (2026-04-26, end of PR-2.3a)**: default scope narrowed from "full repo excluding `docs/archive/`" to `["writings/"]`. Surfaced when the v0.26.0 implementation's CF Preview test 14j (default-scope audit) timed out at 120s — cold-cache fetching ~560 files via the worker's zip-extract path exceeded the curl budget. Real reasons the smaller default is honest: PR-2.2's actual cleanup was writings-only; the April-9 audit classified non-writings broken refs as intentional templates/site-routes/historical-archive (Classes A–E); writings is where authors write `klappy://` URIs as body links most often. Wider scope is one explicit `scope.paths` arg away. If a real consumer demonstrates wider need, the default broadens (or parallelized fetching graduates from the deferred-concerns ledger). Reversal is one-line.