Skip to content

docs(decisions): 0238 — fabrika re-implements v1 rather than calling it - #4728

Merged
usirin merged 2 commits into
mainfrom
umut/adr-0238-fabrika-isolation
Aug 2, 2026
Merged

docs(decisions): 0238 — fabrika re-implements v1 rather than calling it#4728
usirin merged 2 commits into
mainfrom
umut/adr-0238-fabrika-isolation

Conversation

@usirin

@usirin usirin commented Aug 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Adds ADR 0238 — the founder ruling that fabrika calls no v1 code.

Purely additive: one .decisions/ file, nothing else.

What it records

fabrika's stated posture was that it may call pipeline-cli but never grows into it. The wave-0 pilot (#4704 / #4724) showed that does not survive contact — and the reason it fails is the deletion test: a fabrika that calls v1 can never be the thing that replaces it, because every call keeps the old tree alive.

Two forces surfaced it during the pilot:

  • There is no legal way to make the call. cli-invocation-guard reds a bare pipeline-cli in a runnable fence anywhere under claude-plugins/, and the fix it suggests uses the variable expansion the fabrika conventions ban (ADR 0232's isolation verifier). Verified both ways — clean without the call, red with it.
  • Wrapping to dodge that multiplies. The pilot needed two wrapper verbs whose only job was relaying an upstream answer, out of seven for one skill. Nineteen skills on that pattern rebuild pipeline-cli inside fabrika by accretion, which is what the original posture existed to prevent.

Two things it is careful about

It does not make packages/pipeline-cli/ removable. 31 CI workflows invoke that package directly, independent of any skill — it is the guard layer, not v1's skill tooling. This makes the v1 skills deletable once fabrika replaces them; retiring the package is a separate, larger question the ADR explicitly declines to answer.

Not every v1 capability becomes a fabrika verb — some become nothing. Where a thing is already enforced elsewhere, fabrika does not compute a second answer to it. The pilot dropped its adr classify verb on that test, because a fabrika copy of the guard vocabulary could contradict cp-classify at the merge gate, and two answers to a gated question are worse than a dependency.

Process

Sweep run: 8 shortlisted, all opened. One real hit — ADR 0232, the isolation-verifier decision this ADR's reasoning rests on — now cited. 0228/0229 (script relay) and 0236 (eval harness) are adjacent but re-decide nothing here. No supersede, no amend-in-part.

decisions-index validate passes. Vocabulary impact: none — "the deletion test" is already canonical in .glossary/LANGUAGE.md; this applies it rather than coining anything.

Control plane: guard-content-probe classifies this guard-touching, so it routes as §CP and needs a @kamp-us/control-plane approval at head. Not reworded to shake that verdict — see #2617 on the probe's calibration.

Companion PR: #4724 carries the skill and contract changes this decision governs.

@usirin
docs(decisions): 0238 — fabrika re-implements v1 rather than calling it
f142fed 
Founder ruling in-session on the wave-0 pilot. Records the isolation rule and
the reasoning that forced it, so sessions on other tasks inherit it.
@github-actions

github-actions Bot commented Aug 2, 2026 •
edited
Loading

Copy link
Copy Markdown

No preview deploy

  • No preview deploy for this PR — its diff touches no deploy-relevant path, so no preview stack was minted and e2e is not applicable. (6c24ebc)
  • web — Stage pr-4728 torn down.

notusirin
notusirin previously approved these changes Aug 2, 2026
View reviewed changes
@usirin

usirin commented Aug 2, 2026

Copy link
Copy Markdown
Member Author

review-doc: advisory — blocking-set PR (§CP — approval-gated)

PR #4728 touches a guard-touching .decisions/** ADR (§CP by content, ADR 0164 — the shared guard-content-probe verb flagged it: .decisions/0238-fabrika-reimplements-v1-never-calls-it.md → guard-touching [guard-vocabulary-match]). My verdict is advisory only: it does not authorize a merge. Under the §CP hard gate (ADR 0135), a @kamp-us/control-plane member approves this at its current head and ship-it then enqueues it (ADR 0048 single merge authority) — there is no human hand-merge in the §CP path.

Reviewed-head: @ f142fed

Required-namespace set for this diff (class-probe classify --namespaces): 1 → review-doc. No code, skills, or UI-affecting path present, so no sibling gate is required or dispatched.

Acceptance criteria

  • [N/A] Acceptance criteria — no linked issue (docs-only, ADR 0075). Zero closing keywords in the body and every changed path is .decisions/**, so the conversation-authored carve-out applies: this is a legitimate state, not a broken seam. The verdict rests on doc hygiene alone.

Doc hygiene

ADR contradiction sweep (Step 4a)

  • [PASS] Sweep reproduced independently: 233 ADRs scanned, 195 live-accepted and uncited in scope, 8 shortlisted — matching the PR body’s "8 shortlisted, all opened" exactly, with cites 0232 confirmed as the only citation. Shortlist: 0228, 0229, 0231, 0181, 0224, 0073, 0233, 0178. I opened each, plus 0236 (a semantic addition the tool does not surface). No same-question conflict with a live accepted ADR:
    • 0228 / 0229 / 0231 / 0233 (the derive-vs-relay family) govern v1’s own extracted skill scripts under epic Pipeline skills embed 3,510 lines of shell across 252 fenced blocks #4435 relaying in-tree verbs. 0238 governs a fabrika verb wrapping across the v1 boundary — a different subject at a different level. 0228’s "relay is sanctioned" and 0238’s ban on "wrapper verbs whose only behaviour is relaying an upstream answer" are not opposite answers to one question: the first is a script-to-verb rule inside v1, the second a verb-to-verb rule across the v1/fabrika seam, and fabrika did not exist when 0228–0233 were written.
    • 0236 — same day, same fabrika tag, and notably not on the mechanical shortlist — places fabrika’s eval system inside the v1 package’s eval-harness module. Still not a conflict: 0238’s binding scope is "no fabrika skill and no fabrika verb," and 0236 pins that harness’s invocation sites to an operator’s shell and a review-skill spawn, neither of which is a fabrika skill or verb. 0238’s own carve-out — that it does not make the package removable, with deletability scoped to the v1 skills — covers exactly this case.
    • A clean sweep is not evidence of no contradiction; the above is a read, not an exit code.

Falsifiable claims — verified against freshly fetched origin/main

  • [PASS] cli-invocation-guard reds a bare pipeline-cli in a runnable fence anywhere under claude-plugins/ — exact. The job scans both *.md and *.sh under claude-plugins on both legs, and its own header states it reds on any bare invocation inside runnable shell in the plugin corpus.
  • [PASS] The fix it suggests uses the banned variable expansion — literal. The guard’s command module emits a FIX line telling the author to resolve the shim once per fence via a CLAUDE_PLUGIN_ROOT-rooted assignment, and ADR 0232 records that this interpolated idiom is refused as "too complex to verify." Live corroboration: this review session hit that same isolation-verifier refusal repeatedly on ordinary compound shell.
  • [PASS] "Verified both directions: the guard is clean without the call" — corroborated. The no bare pipeline-cli invocation in runnable shell check is success on companion PR feat(fabrika): the /adr skill and its derived CLI contract — wave-0 pilot (#4704) #4724 at 5f38c5d.
  • [PASS] Both adr-sweep scars (adr-sweep shortlist --json writes its payload to stderr, not stdout #4723) — real. adr-sweep shortlist --json writes its payload to stderr, not stdout #4723 documents the --json payload riding the failure channel to stderr with stdout at zero bytes; and exiting non-zero on its own informative case is the tool’s documented design (review-doc Step 4a: "non-zero means there is a shortlist to clear").
  • [PASS] "the deletion test" is already canonical in .glossary/LANGUAGE.md — it is defined there as a named term. "No vocabulary impact" is correct.
  • [PASS] Brief arithmetic — Authoring brief: /report — fabrika skill (wave 0) #4705–Authoring brief: /doctor — fabrika skill (wave 3) #4722 is exactly 18 issues, plus the Authoring brief: /adr — the fabrika wave-0 pilot skill #4704 pilot makes 19 skills, matching both "eighteen unfired briefs" and "Nineteen skills." Spot-checked Authoring brief: /report — fabrika skill (wave 0) #4705: its body was amended in place (field 4 now reads "Prior art — read, never called" / "Read these; call none of them"), exactly as ## Records claims.
  • [PASS] Seven verbs minus the dropped one — feat(fabrika): the /adr skill and its derived CLI contract — wave-0 pilot (#4704) #4724’s final inventory carries six (adr next/new/resolve/supersede/amend-in-part/sweep) with no adr classify, consistent with "seven verbs" pre-decision; and the Consequences note that adr sweep "now owes a lexical/rarity ranking that already exists a few directories away" matches the contract’s re-implemented adr sweep row.
  • [PASS] The three encoding sites — all three land in feat(fabrika): the /adr skill and its derived CLI contract — wave-0 pilot (#4704) #4724: rule 6 "fabrika calls nothing outside fabrika" in the CLI interface convention, the README absent-list line, and authoring-brief field 4.
  • [PASS] "Thirty-three CI workflows call that package directly" — with a precision note. Verified independently: 33 workflow files under .github/workflows/ reference the package, so the claimed number lands exactly. Of those, 31 actually invoke it (a direct node call on the package’s bin entry); the other two do not — deploy.yml names the path only inside comments, and publish.yml is the package’s own release workflow, reading package.json’s version to gate the release tag without invoking a verb. The load-bearing conclusion is untouched: 31 direct, skill-independent CI invocations amply support "it is the guard and enforcement layer, not merely v1’s skill tooling," and the ADR declines the retirement question either way. Not a gate failure — flagged because the count is stated as a fact and the slip runs in the direction that flatters the argument. Optional tightening: "31 CI workflows call that package directly," or keep 33 and say "depend on" rather than "call."

Notes for the control-plane approver (non-blocking)

  1. Merge ordering. ## Records says this is "landed with feat(fabrika): the /adr skill and its derived CLI contract — wave-0 pilot (#4704) #4724" and ## Decision says the superseded README line "is amended by the same change" — but feat(fabrika): the /adr skill and its derived CLI contract — wave-0 pilot (#4704) #4724 is still open. If 0238 lands alone, main briefly carries an accepted ADR contradicted by the un-amended README line it supersedes. Recommend approving and enqueueing feat(fabrika): the /adr skill and its derived CLI contract — wave-0 pilot (#4704) #4724 alongside or ahead of this one.
  2. Optional citation. 0231/0233 carry the "must be a tested pipeline-cli verb" phrasing and sat on my shortlist, but the PR body’s sweep summary names only 0228/0229/0236. My read is adjacent-not-conflicting — their corpus is v1’s Pipeline skills embed 3,510 lines of shell across 252 fenced blocks #4435 extraction, which predates fabrika — but one line in ## Context would close the residual reading that a fabrika skill’s decision-computing shell owes a v1 verb.
  3. The guard-touching classification is correct, and the author did not reword to shake it. That is the right call for an ADR whose Decision constrains what fabrika skills may invoke.

Read the PR head (§HEAD): every file under review was sourced from f142fed5047dc9bc561a61079b3eb644a132f6d8 via a per-run ref, read-only — never the launched checkout’s working copy, and no working tree was switched or mutated (§RO).

All checks pass. review-doc does not merge, and this verdict does not authorize one: a @kamp-us/control-plane approval at head is what unblocks ship-it’s enqueue.

Verdict-written: 2026-08-02T01:38:22Z

@usirin
fix(decisions): 0238 counts workflows that invoke pipeline-cli, not m…
6c24ebc 
…ention it

33 files reference the package; 31 invoke a verb. deploy.yml names the path
only in comments, and publish.yml is the package's own release workflow -
its pipeline-cli line is `pnpm --filter @kampus/pipeline-cli typecheck`,
which runs the package's own script rather than calling a verb.

The original count came from grep -rl | wc -l, which counts mentions, and
was written as "call". Caught in review. Either number supports the claim it
sits under, but the ADR states it as fact and bodies are immutable once
accepted, so it is cheaper to be right now than to amend later.
This was referenced Aug 2, 2026
Merged
Closed
@usirin

usirin commented Aug 2, 2026

Copy link
Copy Markdown
Member Author

review-doc: advisory — blocking-set PR (§CP — approval-gated)

PR #4728 touches a guard-touching .decisions/** ADR (§CP by content, ADR 0164 — the shared guard-content-probe verb flagged it: .decisions/0238-fabrika-reimplements-v1-never-calls-it.md → guard-touching [guard-vocabulary-match]). My verdict is advisory only: it does not authorize a merge. Under the §CP hard gate (ADR 0135) a @kamp-us/control-plane member approves this at its current head and ship-it then enqueues it (ADR 0048 single merge authority) — there is no human hand-merge in the §CP path. This verdict does not authorize that approval.

Reviewed-head: @ 6c24ebc

Re-review at a new head. The prior round was advisory-PASS at f142fed5; this re-binds to 6c24ebcd after the factual correction. Inter-head diff verified independently: exactly one line changed, the ## Consequences workflow-count sentence. Nothing else moved.

Required-namespace set (dispatch). class-probe classify --namespaces → 1 namespace → review-doc (probe exit 0; 1 changed file(s) → has-docs). No apps/web/src/ path, so review-design is not dispatched; no code or skills class present. Diff is purely additive: one file, 104+/0-.

Acceptance criteria

  • [N/A] No linked issue — conversation-authored ADR, doc-surface-only diff (.decisions/** alone). Legitimate issueless state under ADR 0075, not a broken seam; confirmed no Fixes/Closes/Resolves keyword in the body. The hygiene checklist is the whole gate.

The corrected claim — re-counted from scratch

  • [PASS] "Thirty-one CI workflows invoke that package directly" — 31 is correct. Counted against freshly-fetched origin/main (49a2290):
    • 33 workflow files match pipeline-cli under .github/workflows/.
    • deploy.yml — both hits sit on comment lines (:33, :115); zero non-comment references. Correctly excluded.
    • publish.yml — the package's own release workflow: a tag-grammar regex, a package.json version read, pnpm --filter @kampus/pipeline-cli typecheck, … build, and working-directory: packages/pipeline-cli. It runs the package's own scripts and invokes no verb. Correctly excluded. (Precision note: the framing offered for this review said its only matching line was the typecheck one — there are also build and working-directory. The exclusion and the resulting count are unaffected.)
    • The remaining 31 each carry a real verb invocation — node packages/pipeline-cli/src/bin.ts <verb> (plus src/tools/ci-required/bin.ts in ci.yml). I enumerated and inspected all 31 individually rather than trusting the subtraction.

Other falsifiable claims spot-checked (7 beyond the count)

  • [PASS] cli-invocation-guard reds a bare pipeline-cli in a runnable fence anywhere under claude-plugins/ — the workflow scans find claude-plugins -type f \( -name '*.md' -o -name '*.sh' \), the whole corpus.
  • [PASS] The fix it suggests is the ${CLAUDE_PLUGIN_ROOT:-…} expansion — the guard's literal FIX string in packages/pipeline-cli/src/tools/cli-invocation-guard/command.ts:43 is exactly that shape.
  • [PASS] ADR 0232 refuses a variable-rooted command — 0232's Context names the interpolated "${CLAUDE_PLUGIN_ROOT:-…}/…" idiom as "refused as 'too complex to verify'". Cited ADR is live-accepted on origin/main.
  • [PASS] The adr-sweep scars — adr-sweep shortlist --json writes its payload to stderr, not stdout #4723's title is verbatim "adr-sweep shortlist --json writes its payload to stderr, not stdout"; and the exit-non-zero-on-its-own-informative-case scar reproduced in my own sweep run below (shortlist found → exit 1).
  • [PASS] "the deletion test" is already canonical in .glossary/LANGUAGE.md — defined there as a named term; the ADR applies it and coins nothing.
  • [PASS] The eighteen unfired briefs Authoring brief: /report — fabrika skill (wave 0) #4705–Authoring brief: /doctor — fabrika skill (wave 3) #4722 — arithmetic checks (18), and both endpoints resolve to real authoring-brief issues.
  • [PASS] Records — the three encoding sites (fabrika CLI interface convention, README absent-list, authoring-brief contract) are all present in companion PR feat(fabrika): the /adr skill and its derived CLI contract — wave-0 pilot (#4704) #4724. The verb-count narrative is consistent: 7 pilot verbs minus the dropped adr classify leaves the 6 in the landed contract.

Doc hygiene

One non-blocking note for the author (not a gate failure)
The PR description still reads "33 CI workflows call that package directly" — the superseded number. The committed artifact is correct, and the PR body is not a surface this gate governs, so it does not fail the review. But ship-it squash-merges, so the stale 33 would land in the commit message. Worth a one-line body edit before approval, since it is free now and immutable after.

Read the PR head (§HEAD): every file under review sourced from 6c24ebcd71f1468381f2ccab371c74760c4eeb64 via a per-run ref (git show "$PR_REF:<path>"), fetched and asserted equal to the pinned head. No checkout, no working-tree mutation. Ground-truth checks ran against a freshly-fetched origin/main.

All checks pass at this head. review-doc does not merge, and for this §CP PR the verdict is advisory: a @kamp-us/control-plane approval at 6c24ebcd is what lets ship-it enqueue it.

Verdict-written: 2026-08-02T02:00:29Z

@usirin
usirin added this pull request to the merge queue Aug 2, 2026
Merged via the queue into main with commit 61c17a9 Aug 2, 2026
46 checks passed
@usirin
usirin deleted the umut/adr-0238-fabrika-isolation branch August 2, 2026 02:44
@usirin usirin mentioned this pull request Aug 2, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@notusirin notusirin notusirin approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@usirin @notusirin