Files
ehoandClaude Sonnet 5 02d41536df refactor(brief): move the action lifecycle into the machine (RD-12)
The action lifecycle (Idle | Busy | Failed) lived in an imperative
store-level signal, set from ten call sites outside the reducer. The
reducer could not enforce which action transitions are legal.

Add `action` to `BriefState.Loaded`, driven by three new messages
(ActionStarted, ActionFinished, ActionFailed) and handled in `reduce`.
Replace every `actionState.set(...)` call in `brief.store.ts` with the
matching `dispatch`. `BriefLoaded` resets `action` to Idle, so a fresh
load clears a stale action error instead of letting it outlive the
reload.

`busy` and `lastError` stay as `computed`s on the store with a
byte-identical public signature — they are the render seam for four
components and two page templates, and the union belongs in the
machine, not the components. `revealBigNummer` still sets only
`Failed`, never `Busy` — an existing asymmetry, not changed here.
`SaveState`, `org-template.store.ts`, and `pendingPublish` are out of
scope (RD-13, RD-14).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-04 18:27:09 +02:00

7.4 KiB

RD-12 — Move the brief's action lifecycle into the machine

Status: done Source: PLAN.md 1b#2a

Why

brief.store.ts keeps the action lifecycle in a store-level signal, set imperatively from about ten places entirely outside the reducer:

private actionState = signal<ActionState>({ tag: 'Idle' });
readonly busy = computed(() => this.actionState().tag === 'Busy');
readonly lastError = computed(() => {  });

So the machine cannot enforce which action transitions are legal, and ActionState has two producers and zero consumers that keep the union — both stores immediately collapse it back to a boolean plus a nullable string, the exact shape its own doc comment says it exists to remove.

Move it into the machine's Loaded state and the reducer owns it, like every other state change in this house.

Read first

  • libs/shared/src/application/action-state.ts — 9 lines, both types. Only ActionState is in scope; SaveState is RD-14's.
  • apps/ssp/src/app/brief/application/brief.store.tsactionState at 44, busy at 45, lastError at 46, and the setter sites in flushSave (207), resetDemo (226), previewLetter (251), revealBigNummer (269) and transition (280)
  • apps/ssp/src/app/brief/domain/brief.machine.ts — the Loaded variant (PascalCase since RD-11) and reduce
  • apps/ssp/src/app/brief/ui/brief.page.ts:50-126 — the <app-async> wrapper. Decision 2 depends on it.

Decisions (pre-made, don't relitigate)

  1. action becomes a field on BriefState.Loaded, carrying the same three cases (Idle | Busy | Failed{error}), driven by three new messages — ActionStarted, ActionFinished, ActionFailed — handled in reduce. The imperative actionState.set(...) calls become dispatch(...).

  2. This is safe because every action trigger is template-gated, and that was verified, not assumed. brief.page.ts:55 opens <ng-template appAsyncLoaded>, which renders only when remoteData() is Success — i.e. when the machine is Loaded. All three entry points sit inside it: the reset button (:83), previewLetter (:101, :120) and revealBigNummer (:102). transition backs submit/approve/reject/send, reachable only from the same surface, and flushSave runs from the debounced autosave, which only fires while editing a loaded brief.

    If you add an action trigger outside that slot, this design breaks. Do not add one.

  3. busy and lastError stay as computeds on the store. They are the render seam, not a second encoding: four components take busy = input(...)behandel-scherm, letter-composer, org-template-editor, rejection-comments — and two pages read store.busy()/store.lastError() directly. A boolean is right at that boundary; the union is right in the machine. Do not push the union down into the components — it would churn four components and their stories for no gain.

  4. BriefLoaded resetting action to Idle is intended. A fresh load clears a stale action error, which is a small behaviour improvement: today a failed action's message can outlive a reload. Let the reducer do it, and say so in a comment.

  5. flushSave sets both saveState and actionState today. Keep both. The autosave failure legitimately surfaces in two places — the small save indicator and the action error line. Only the actionState half becomes a dispatch here; leave saveState exactly as it is.

  6. Do not touch org-template.store.ts, pendingPublish, or SaveState. RD-13 folds org-template (including pendingPublish, the one genuine illegal-state pair), and RD-14 moves SaveState and deletes action-state.ts. action-state.ts therefore still exists after this ticket, exporting only SaveState plus an ActionState that brief no longer imports.

Files

  • apps/ssp/src/app/brief/domain/brief.machine.ts (+ .spec.ts)
  • apps/ssp/src/app/brief/application/brief.store.ts (+ .spec.ts)

Not action-state.ts (RD-14 deletes it). Not org-template.store.ts (RD-13). No UI files.

Steps

  1. Add action to BriefState.Loaded and the three messages to BriefMsg; handle them in reduce, including the BriefLoaded reset from decision 4.
  2. Add reducer spec cases (see Acceptance).
  3. Replace each actionState.set(...) in brief.store.ts with the matching dispatch.
  4. Re-point busy and lastError at the machine's Loaded.action, keeping their public signatures identical so no UI file changes.
  5. Run npm run gen:behaviour-spec — new it() titles otherwise fail the drift check.
  6. Update this ticket's Status: to done and the README's RD-12 row to done.
  7. Commit all of it together.

Acceptance criteria

Dry-run against the tree before handover, with the measured baselines: brief.store.ts has 14 actionState occurrences and brief.machine.ts has 0 action messages; both must invert. saveState is 5 and must stay 5. The busy/lastError declarations are 2 and must stay 2.

B=apps/ssp/src/app/brief
git grep -c "actionState" -- $B/application/brief.store.ts        # MUST return nothing
git grep -n "ActionState" -- $B                                    # MUST return nothing
git grep -c "ActionStarted\|ActionFinished\|ActionFailed" -- $B/domain/brief.machine.ts  # >= 3

The render seam is unchanged, so no UI file was touched:

git diff --name-only HEAD | grep -c "brief/ui/" || true   # MUST be 0
git grep -c "readonly busy\|readonly lastError" -- $B/application/brief.store.ts   # still 2

SaveState and org-template are untouched (decision 6):

git diff --name-only HEAD | grep -cE "action-state|org-template" || true   # MUST be 0
git grep -c "saveState" -- $B/application/brief.store.ts   # unchanged: still 5

New reducer cases:

- ActionStarted moves a loaded brief to Busy
- ActionFailed carries the error
- ActionFinished returns to Idle
- BriefLoaded resets a stale action error to Idle
- an action message is a no-op when the brief is not loaded
npm run ci   # exits 0

Verification

npm run ci. No story, no .mdx, no libs/shared/src/ui/**, so --full is not required.

If you do run the full gate, pass timeout: 600000 on the Bash call — it takes about 8 minutes and the harness backgrounds anything longer than 120s, which would end your turn with the work uncommitted.

Out of scope

  • org-template.store.ts and pendingPublish — RD-13.
  • SaveState, and deleting action-state.ts — RD-14.
  • The four busy = input(...) components and their stories (decision 3).
  • The NO_SUBORGS/NO_TABLES-should-be-Empty finding — optional RD-34.

Risks

  • Decision 2 is the load-bearing assumption. It holds today because of one <ng-template appAsyncLoaded>. Re-read brief.page.ts:50-126 and confirm before you start; if any trigger has moved outside that slot since this ticket was written, stop and say so rather than adding a guard that changes behaviour.
  • revealBigNummer sets only Failed, never Busy. Do not "fix" that asymmetry here — it is existing behaviour, and changing it is a separate decision.
  • Keep busy/lastError signatures byte-identical. They are read from two page templates; a renamed or re-typed member turns a pure refactor into a UI change.
  • behaviour-spec.mdx drift from the new spec titles. Run gen:behaviour-spec in the same commit.