feat(riviere-cli): add hello command - #275

Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/ci.yml:
- Around line 45-52: The CI step "Build better-sqlite3 native bindings" uses
`npx --yes prebuild-install` and `npx --yes node-gyp rebuild`, which fetches
unpinned tooling at runtime; change the step to invoke the local binaries
shipped with the better-sqlite3 package instead of npx and ensure node-gyp is
pinned: replace the commands that run inside the BS3_DIR block to call
"$BS3_DIR/node_modules/.bin/prebuild-install" (and
"$BS3_DIR/node_modules/.bin/node-gyp rebuild" if you keep the fallback), and if
you need node-gyp add it to devDependencies so its version is locked in
pnpm-lock.yaml (or remove the node-gyp fallback entirely if you only want to run
the locked prebuild-install).

In `@package.json`:
- Around line 91-94: Remove "better-sqlite3" from the root package.json
ignoredBuiltDependencies array so the workspace-level allowlist in
pnpm-workspace.yaml can control native builds; specifically edit the
ignoredBuiltDependencies block (remove the "better-sqlite3" entry), commit the
change, then re-run CI to confirm the manual native rebuild step in
.github/workflows/ci.yml (lines that perform a rebuild) is no longer required.

In `@tools/dev-workflow-v2/commands/implement-feature.md`:
- Around line 31-38: Add a new step documenting the ADDRESSING_FEEDBACK state
between CHECKING_FEEDBACK and REFLECTING: describe that when CHECKING_FEEDBACK
finds review comments you enter ADDRESSING_FEEDBACK, list the actions to take
(implement requested changes, run tests/`pnpm verify`, push commits and update
the PR), record the iteration and results, and specify transitions (loop back to
AWAITING_CI to wait for CI on new commits or proceed to REFLECTING once
reviewers are satisfied); reference the state name ADDRESSING_FEEDBACK and the
preceding/following states CHECKING_FEEDBACK, AWAITING_CI, and REFLECTING so the
flow is unambiguous.

In `@tools/dev-workflow-v2/commands/workflow.md`:
- Around line 3-5: Remove the redundant self-assignment of CLAUDE_PLUGIN_ROOT in
the command invocation: the env var CLAUDE_PLUGIN_ROOT is already expected to be
set, so invoke npx tsx with the existing
"${CLAUDE_PLUGIN_ROOT}/src/entrypoint/workflow-cli.ts" and $ARGUMENTS without
prefixing CLAUDE_PLUGIN_ROOT="${CLAUDE_PLUGIN_ROOT}"; update the example
invocation in the documentation to simply call npx tsx
"${CLAUDE_PLUGIN_ROOT}/src/entrypoint/workflow-cli.ts" $ARGUMENTS.

In `@tools/dev-workflow-v2/package.json`:
- Line 2: Update the package.json "name" property to use the
`@living-architecture` scope (e.g., change "name": "dev-workflow-v2" to the scoped
name "@living-architecture/dev-workflow-v2") so the package follows the monorepo
naming guidelines; locate and update the "name" field in the package.json file
to the new scoped value.

In `@tools/dev-workflow-v2/README.md`:
- Line 42: Update the diagram label and/or visualization to reflect that the
BLOCKED transition returns to the previously active state (not always
IMPLEMENTING); reference blocked.ts and the BLOCKED state logic and change the
label text from "returns to pre-blocked state" to something explicit like
"returns to previous state (e.g., IMPLEMENTING)" or add a note/annotation tied
to the BLOCKED arrow, or redraw the arrow to indicate it returns to the prior
state rather than only pointing to IMPLEMENTING.

In `@tools/dev-workflow-v2/src/domain/workflow-error.ts`:
- Around line 1-6: The WorkflowError constructor should capture the stack like
other errors in the codebase: after calling super(message) and setting this.name
= 'WorkflowError', invoke Error.captureStackTrace?.(this, this.constructor) to
ensure the stack trace is properly captured; use this.constructor (not the
literal class name) to match the existing pattern used across other error
classes.

In `@tools/dev-workflow-v2/src/entrypoint/fixtures/workflow-cli-test-fixtures.ts`:
- Around line 128-130: The silent return after resolving steps hides missing
fixture states; instead, detect when stateSteps[targetState] is undefined and
fail fast by throwing an explicit error (or using an assertion) that includes
the targetState value and a clear message; update the block around
runWorkflow([...], deps) / stateSteps and targetState to validate steps !==
undefined and throw (or assert) so typos or unimplemented states surface
immediately.

In `@tools/dev-workflow-v2/src/entrypoint/workflow-cli.spec.ts`:
- Around line 164-176: The tests for argument validation are running commands
from the initial CLI state so the handlers can early-reject before argument
parsing; update the failing cases (the tests titled 'record-feedback-exists' and
'record-reflection') to mirror the working pattern used in the 'record-pr' tests
by calling setup() followed by runWorkflow(['init'], deps) before invoking
runWorkflow(['record-feedback-exists', ...], deps) and
runWorkflow(['record-reflection', ...], deps) respectively (use the same
runWorkflow/init sequence as in the 'record-pr' tests to ensure the
argument-validation logic in the handlers is actually exercised).

In `@tools/dev-workflow-v2/src/infra/cli/command-handlers.ts`:
- Around line 142-153: handleRecordReviewFailed currently allows an empty
reviewers array; add validation at the start of handleRecordReviewFailed to
ensure reviewers.length > 0 and return an appropriate failure OperationResult if
none provided. Specifically, check the reviewers variable after const reviewers
= args.slice(1) and short-circuit (e.g., return a failure OperationResult or
throw a controlled error consistent with other handlers) instead of calling
engine.transaction; keep the existing call to engine.transaction(..., (w) =>
w.recordReviewFailed(reviewers)) unchanged when validation passes.

In `@tools/dev-workflow-v2/src/infra/cli/environment.spec.ts`:
- Around line 24-61: Update the negative-path tests for getSessionId,
getPluginRoot, and getEnvFilePath to also assert that empty-string ('') and
whitespace-only values (e.g., '   ') are treated as missing: set
process.env['CLAUDE_SESSION_ID'] (and likewise CLAUDE_PLUGIN_ROOT,
CLAUDE_ENV_FILE) to '' and to '   ' in separate assertions and expect each to
throw WorkflowError with the same message 'Missing required env var: <VAR_NAME>'
— keep the existing delete-case assertions, and add the two extra cases per
accessor to ensure the accessors reject effectively-missing inputs.

In `@tools/dev-workflow-v2/src/infra/cli/environment.ts`:
- Around line 4-25: The three functions getSessionId, getPluginRoot, and
getEnvFilePath currently only check for undefined but allow empty or
whitespace-only env values; update each to read the env var, trim it, and if the
trimmed value is falsy (empty string) throw the same WorkflowError ('Missing
required env var: <NAME>') so blank/whitespace values are rejected at the
boundary; ensure you return the trimmed non-empty string from each function.

In `@tools/dev-workflow-v2/src/infra/cli/hook-io.spec.ts`:
- Around line 19-75: Add tests to cover empty string and whitespace-only values
for required string fields so the parsers reject them: extend parseCommonInput
specs to pass payloads where session_id, transcript_path, cwd, and
hook_event_name are '' and '   ' and assert they throw WorkflowError with
message 'Invalid hook input for HookCommonInput'; similarly extend
parsePreToolUseInput specs to include cases where tool_name and tool_use_id (and
the common required fields) are '' and '   ' and assert they throw WorkflowError
with message 'Invalid hook input for PreToolUseInput'; keep the same
JSON-malformed assertions but add these negative cases to ensure the parsers
validate non-empty, non-whitespace strings.

In `@tools/dev-workflow-v2/src/infra/cli/hook-io.ts`:
- Around line 4-16: HOOK_COMMON_INPUT and PRE_TOOL_USE_INPUT currently allow
empty or whitespace-only strings (e.g., session_id, cwd, hook_event_name,
tool_name, tool_use_id); tighten the schemas by requiring non-empty,
non-whitespace values—replace z.string() for those fields with a
trimmed-and-minimum-length check (e.g., use z.string().trim().min(1) or an
equivalent refine) so blank or whitespace-only inputs are rejected at the hook
boundary while leaving permission_mode and tool_input behavior unchanged.

In `@tools/dev-workflow-v2/src/shell/composition-root.ts`:
- Around line 33-37: The current WorkflowDeps object sets checkPrChecks: () =>
true which unconditionally marks PR checks as passing; replace this with a real
check or a fail-closed stub so CI cannot be bypassed. Update the workflowDeps
entry (or the buildRealDeps factory that creates it) to call the actual check
function (e.g., an existing getPrChecksStatus / verifyPrChecks method) or
implement a placeholder that returns false or throws when it cannot determine CI
status; ensure the symbol names workflowDeps and checkPrChecks are used so the
runtime uses the real verifier instead of the always-true lambda.

In `@tools/dev-workflow-v2/src/workflow-definition/domain/fold.ts`:
- Around line 26-84: The applyEvent reducer currently swallows unknown
WorkflowEvent types by falling through to the final "return state"; update
applyEvent so all intentional no-op event types are handled explicitly (add case
branches that return state for known no-op events) and replace the implicit
fallback with a hard failure: after the switch throw a descriptive error
(including the event.type and any id) for unknown/unhandled events so reducer
drift is caught; keep the existing special-case call to applyTransitioned(event)
at the top and ensure the thrown error references applyEvent and WorkflowEvent
to aid locating the problem.
- Around line 4-12: EMPTY_STATE is duplicated from the adapter's INITIAL_STATE
which risks drift; change EMPTY_STATE to derive from the shared INITIAL_STATE
used by WORKFLOW_ADAPTER.initialState() (import INITIAL_STATE) instead of
repeating the literal, and ensure you clone it (e.g., shallow spread or deep
copy) so applyEvents() and rehydrated workflows share the same
initial-source-of-truth without exposing the original to mutation.

In `@tools/dev-workflow-v2/src/workflow-definition/domain/registry.ts`:
- Around line 16-21: GLOBAL_FORBIDDEN's bashPatterns only match literal bare
tokens and miss prefixed paths or args (e.g., "./git push" or "bash -lc \"git
push ...\""); update GLOBAL_FORBIDDEN.bashPatterns to match command invocations
more robustly by anchoring on word boundaries and allowing optional path
prefixes and intervening args, e.g. patterns that detect (optional ./ or / or
path components) before the command name and use \b after the subcommand (e.g.,
match (?:^|\s|&&|;|[|<>])(?:\.\/|\/|[\w\-/]+\/)?git\s+push\b and similarly for
gh\s+pr\b to catch "gh pr create" etc.), and ensure flags/arguments after the
subcommand don't prevent a match; modify the bashPatterns array in
GLOBAL_FORBIDDEN accordingly.

In
`@tools/dev-workflow-v2/src/workflow-definition/domain/states/addressing-feedback.ts`:
- Around line 14-18: transitionGuard currently blocks all transitions when
ctx.state.feedbackAddressed is false; change it so BLOCKED transitions are
exempted. Update the transitionGuard (the function that calls fail(...) /
pass()) to first check the transition/event type (e.g., ctx.event?.type or
ctx.transition name) and immediately return pass() when the transition is
'BLOCKED', otherwise keep the existing check against ctx.state.feedbackAddressed
and return fail('Feedback not addressed. Run record-feedback-addressed first.')
or pass() accordingly.

In `@tools/dev-workflow-v2/src/workflow-definition/domain/states/awaiting-ci.ts`:
- Around line 12-17: The transitionGuard currently allows moving to
'IMPLEMENTING' when ctx.state.ciPassed is unset; change the guard in
transitionGuard so transitions to 'IMPLEMENTING' are only permitted when
ctx.state.ciPassed === false (i.e., CI explicitly recorded as failed).
Concretely, update the condition that checks ctx.to === 'IMPLEMENTING' to reject
when ctx.state.ciPassed is not strictly false (ctx.state.ciPassed !== false) and
return a clear fail message instructing to run the record-ci-failed action; keep
the existing CHECKING_FEEDBACK check using ctx.state.ciPassed for the pass case.

In
`@tools/dev-workflow-v2/src/workflow-definition/domain/states/checking-feedback.ts`:
- Around line 12-17: The transitionGuard currently only blocks
ADDRESSING_FEEDBACK when ctx.state.feedbackClean === true, allowing a transition
from CHECKING_FEEDBACK to ADDRESSING_FEEDBACK when feedbackClean is unset;
update transitionGuard to require an explicit recorded flag (e.g.,
ctx.state.feedbackRecorded or test for feedbackClean !== undefined) before
permitting ADDRESSING_FEEDBACK: if the recorded flag is missing, return
fail('Feedback not recorded. Run record-feedback-first.') and only allow
ADDRESSING_FEEDBACK when feedbackRecorded === true (or when feedbackClean is
explicitly false), keeping the existing checks for REFLECTING and using the same
fail/pass helpers.

In
`@tools/dev-workflow-v2/src/workflow-definition/domain/workflow-feedback-reflecting.spec.ts`:
- Around line 185-191: The test title "rejects all transitions" is misleading
because the spec only tests COMPLETE -> IMPLEMENTING; either rename the it()
description to something like "rejects transition from COMPLETE to IMPLEMENTING"
or expand the when(...) calls to attempt transitions to every other target state
(e.g., call wf.transitionTo for each possible state after invoking
spec.given(...eventsToComplete()) and assert result.pass is false for each).
Locate the failing test around the describe('COMPLETE state') block that uses
spec.given(...eventsToComplete()) and wf.transitionTo('IMPLEMENTING') and update
the test name or add additional transition attempts accordingly.
- Around line 80-88: The test only asserts preBlockedState but must also confirm
the workflow's actual current state changed; update the spec that calls
spec.given(...eventsToCheckingFeedback()).when((wf) =>
wf.transitionTo('BLOCKED')) (and the similar specs at the other ranges) to
include an assertion that state.currentStateMachineState (from the destructured
{ result, state }) is 'BLOCKED' (e.g.,
expect(state.currentStateMachineState).toBe('BLOCKED')) so the test verifies the
transition, not just the recorded previous state.
- Around line 60-68: The test currently only asserts state.feedbackClean flipped
to false; update the spec that calls recordFeedbackExists(3) (and uses
eventsToCheckingFeedback()) to also assert the actual feedback-exists signal and
the stored count: after when(wf => wf.recordFeedbackExists(3)) add assertions
that the field which records existence (e.g., state.feedbackExists or the
appropriate property on state) is true and that the stored count property (e.g.,
state.feedbackCount or the property that holds the value) equals 3, keeping the
existing feedbackClean assertion.

In `@tools/dev-workflow-v2/src/workflow-definition/domain/workflow-predicates.ts`:
- Around line 20-33: The policy gates currently use substring checks on the raw
command (command.includes(flag) and allowed.some(cmd => command.includes(cmd))),
which leads to false positives/negatives; instead parse the command into argv
tokens (respecting quotes/escapes) and perform exact/positional checks: for
DANGEROUS_FLAGS check against parsedArgs for exact matches or canonical prefixed
forms (e.g., "--flag" or "-f" and "--flag=value"), and for
CURRENT_DEF.allowForbidden?.bash exemptions compare tokens or full parsed
command patterns (not substrings) or run regexes against joined tokens; update
the logic in the block referencing DANGEROUS_FLAGS,
GLOBAL_FORBIDDEN.bashPatterns, getStateDefinition, and
currentDef.allowForbidden?.bash to use a shell-arguments parser (or tokenizer)
and token-based matching rather than raw substring matching.

In
`@tools/dev-workflow-v2/src/workflow-definition/domain/workflow-reviewing-submitting.spec.ts`:
- Around line 214-229: Add a test that covers the "pending CI" case for the
AWAITING_CI -> IMPLEMENTING transition: use spec.given(...eventsToAwaitingCi())
without calling ciPassed() or ciFailed(), then .when(wf =>
wf.transitionTo('IMPLEMENTING')) and assert that result.pass is false and
state.currentStateMachineState remains AWAITING_CI; this ensures
transitionTo('IMPLEMENTING'), eventsToAwaitingCi(), and the spec setup are
exercised for the unset/ongoing CI case.

In `@tools/dev-workflow-v2/src/workflow-definition/domain/workflow-types.spec.ts`:
- Around line 8-10: Replace the array forEach in the test with a for...of loop
so returned values from expect aren't discarded: locate the test "it('accepts
all valid state names')" that iterates STATE_NAMES and change the iteration over
STATE_NAMES to a for (const s of STATE_NAMES) {
expect(STATE_NAME_SCHEMA.parse(s)).toStrictEqual(s) } pattern, keeping the same
assertion using STATE_NAME_SCHEMA and s.

In `@tools/dev-workflow-v2/src/workflow-definition/domain/workflow.ts`:
- Around line 249-251: The verifyIdentity method is a stub that always returns
pass() and ignores the _transcriptPath parameter; add a clear TODO comment above
the verifyIdentity function (referencing verifyIdentity, PreconditionResult,
pass(), and _transcriptPath) stating the intended work (implement identity
verification using the transcript path, validate speaker/metadata, return
appropriate PreconditionResult) and note the parameter is currently unused so it
should be wired into the implementation when addressed.
- Around line 270-284: transitionTo currently calls the
WORKFLOW_REGISTRY[targetState].onEntry and mutates this.state before appending
the 'transitioned' event, which means onEntry changes are not event-sourced and
will be lost on replay; fix by either (A) emitting explicit events for the
onEntry mutations instead of applying them inline (use this.append to push new
events representing the state changes immediately after or as part of the
transition), or (B) move the onEntry application into the fold logic so
applyTransitioned (in fold.ts) invokes WORKFLOW_REGISTRY[to].onEntry (or an
equivalent helper) when handling a 'transitioned' event so those mutations are
recreated during applyEvents/replay; update transitionTo, the append sequence,
and applyTransitioned accordingly to ensure onEntry effects are captured in the
event stream.

In `@tools/dev-workflow-v2/states/awaiting-ci.md`:
- Around line 9-12: The example for the failing CI path is missing the required
failure summary argument for the record-ci-failed command; update the second
checklist item to call `/dev-workflow-v2:workflow record-ci-failed "CI failed:
<brief-reason>"` (or similar) so the CLI receives the required output/reason,
while keeping the subsequent transition instruction to
`/dev-workflow-v2:workflow transition IMPLEMENTING` unchanged; leave the
record-ci-passed and transition CHECKING_FEEDBACK examples as-is.

In `@tools/dev-workflow-v2/states/reviewing.md`:
- Around line 12-15: Add an explicit "reviewer-error" branch before aggregating
verdicts: when waiting for agents to complete, detect crash/invalid JSON
responses and immediately call a new workflow step (e.g.,
`/dev-workflow-v2:workflow record-review-error <failed-agent-names...>` or a
retry/halt action) instead of proceeding to PASS/FAIL aggregation; update the
logic that currently emits `/dev-workflow-v2:workflow record-task-check-passed`,
`/dev-workflow-v2:workflow record-review-passed`, and `/dev-workflow-v2:workflow
record-review-failed` to first validate each agent's JSON and short-circuit to
the reviewer-error branch on parse/runner failures so the workflow halts or
retries rather than recording a partial review.

In `@tools/dev-workflow-v2/states/verifying.md`:
- Around line 8-11: The verifying state steps are out of order: when
verification fails you should immediately invoke /dev-workflow-v2:workflow
record-verify-failed and then /dev-workflow-v2:workflow transition IMPLEMENTING;
after fixing and committing, run /dev-workflow-v2:workflow transition VERIFYING
and re-run verification, and only when it passes call /dev-workflow-v2:workflow
record-verify-passed and /dev-workflow-v2:workflow transition REVIEWING. Ensure
the sequence uses record-verify-failed before transitioning to IMPLEMENTING, and
uses transition VERIFYING after commits to re-run verification.

In `@tools/dev-workflow-v2/tsconfig.json`:
- Around line 1-19: The root tsconfig.json currently contains test-only settings
(types: "vitest/globals" and test includes) so split into the three-file
pattern: keep generic build/compiler options in tsconfig.json, create
tsconfig.lib.json extending ./tsconfig.json for library build specifics
(outDir/rootDir/strict/etc.) and create tsconfig.spec.json extending
./tsconfig.json for tests; move the "types": ["vitest/globals"] entry and any
test-specific "include" globs (e.g., tests/**/*.ts or **/*.spec.ts) into
tsconfig.spec.json and remove vitest from the root "types" array so only
tsconfig.spec.json references vitest/globals, and ensure tsconfig.lib.json and
tsconfig.spec.json are referenced by package-level tooling as needed.

In `@tools/dev-workflow-v2/vitest.config.mts`:
- Around line 3-22: Add the missing root setting to the Vitest config: update
the object passed to defineConfig (the top-level export containing test,
coverage, etc.) to include root: __dirname so Vitest resolves paths consistently
under ESM; locate the exported default object (defineConfig({ ... })) and add
the root: __dirname property alongside test to match other vitest.config.mts
files in the repo.