feat(connections): replace hard-coded eventPublisher with config-driven detection - #288

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

Inline comments:
In `@packages/riviere-extract-config/src/extraction-config-schema.ts`:
- Around line 227-231: The exported ConnectionsConfig currently includes
eventPublishers and is being reused for both top-level connections and
ModuleConfig.connections; create a separate module-scoped type (e.g.,
ModuleConnectionsConfig) that only contains patterns?: ConnectionPattern[] and
update ModuleConfig to use ModuleConnectionsConfig for its connections property
while keeping ConnectionsConfig (with patterns? and eventPublishers?) for the
top-level; update any references/usages that expect ModuleConfig.connections to
ensure they no longer expose eventPublishers in the module surface.

In `@packages/riviere-extract-config/src/validation-connections.spec.ts`:
- Around line 371-382: The test for validateExtractionConfig currently only
checks the case where eventPublishers[0].fromType === ''; add a second assertion
that constructs a config via createMinimalConfig() with connections: {
eventPublishers: [{ fromType: 'someType', metadataKey: '' }] } and assert
validateExtractionConfig(...).valid is false to cover the metadataKey
empty-string branch and prevent schema regressions; ensure both failing cases
(empty fromType and empty metadataKey) are present in the spec referencing
validateExtractionConfig and createMinimalConfig.

In `@packages/riviere-extract-config/src/validation.ts`:
- Around line 193-199: parseExtractionConfig currently returns early when
isValidExtractionConfig(data) passes and thus skips semantic checks in
validateConnectionsConfig (which runs validateEventPublishers using
collectAllCustomTypeNames), causing configs with invalid
connections.eventPublishers[].fromType to slip through; update
parseExtractionConfig to, after schema validation via isValidExtractionConfig,
call validateConnectionsConfig(config) (and any other semantic validators used
in validateExtractionConfig) and throw/return a failure if it returns any
ValidationError(s) so all extraction config validation (both schema and semantic
rules) happens at load time and fails fast with descriptive errors.

In
`@packages/riviere-extract-ts/src/features/extraction/domain/connection-detection/async-detection/detect-event-publisher-connections.ts`:
- Around line 27-34: The current publishers.flatMap branch treats only string
metadata and marks arrays as missing; modify the logic in
detect-event-publisher-connections (the publishers.flatMap callback that reads
publisher.metadata[metadataKey]) to handle array-valued metadata by iterating
over string entries: if publishedEventType is an array, map each non-empty
trimmed string to resolvePublishTarget(publisher, entry, events, options)
(preserving the same publisher) and flatten the results, otherwise keep the
existing string handling and fallback to handleMissingMetadata when appropriate;
also add a regression test that supplies a publisher with metadata[metadataKey]
as a string[] (e.g., two event names) and asserts two async links are emitted
for that single publisher.

In
`@tools/dev-workflow-v2/src/features/workflow/domain/workflow-implementing.spec.ts`:
- Around line 43-50: Add a positive-path unit test for getTranscriptPath: create
a fresh Workflow via Workflow.createFresh(makeDeps()), call the method that
starts a session (e.g., wf.startSession(...) or wf.start(), depending on the
implementation), then call wf.getTranscriptPath() and assert it returns the
exact expected transcript path string; ensure the test name reflects the
positive case (e.g., 'returns transcript path after session started') and uses
the same helpers (makeDeps) to construct any expected path value so the
assertion is deterministic.

In `@tools/dev-workflow-v2/src/features/workflow/domain/workflow.ts`:
- Around line 170-175: The public getTranscriptPath() throws if
state.transcriptPath is undefined which will break rehydrated workflows from old
event logs; update the fold logic (the fold function in fold.ts that handles
'session-started' events) to populate state.transcriptPath for legacy events
that lack that field (e.g., derive a default/compute it from sessionId or other
available event metadata) so that getTranscriptPath() no longer encounters
undefined, keeping getTranscriptPath() and WorkflowStateError unchanged but
ensuring state.transcriptPath is initialized during rehydration.

In
`@tools/dev-workflow-v2/src/features/workflow/infra/persistence/workflow-definition.spec.ts`:
- Around line 48-51: The test currently rebuilds from
WORKFLOW_DEFINITION.initialState() and never exercises
WORKFLOW_DEFINITION.fold(); change the test to reduce at least one event through
WORKFLOW_DEFINITION.fold(initialState, event) (use a representative event
object) to produce a new state, then call
WORKFLOW_DEFINITION.buildWorkflow(foldedState, makeWorkflowDeps()) and assert
the expected currentStateMachineState (or alternatively rename the test to
reflect it is the initial-state case). Target the
WORKFLOW_DEFINITION.initialState(), WORKFLOW_DEFINITION.fold(),
WORKFLOW_DEFINITION.buildWorkflow(), and makeWorkflowDeps() symbols when
updating the test.

In
`@tools/dev-workflow-v2/src/features/workflow/infra/persistence/workflow-definition.ts`:
- Around line 43-48: The fold method currently swallows all safeParse()
failures; update fold (in workflow-definition.ts) to distinguish unknown engine
events from known workflow event types: first check the incoming event.type
against the set of valid workflow event types (the same list/enum used by
Workflow.appendEvent or derived from WORKFLOW_EVENT_SCHEMA), and if the type is
not a workflow event return state (no-op); if the type is a known workflow event
but WORKFLOW_EVENT_SCHEMA.safeParse(event) fails, throw or surface an error
(fail fast) instead of returning state so malformed persisted workflow events
are not silently ignored before calling applyEvent.

---

Outside diff comments:
In `@packages/riviere-extract-config/extraction-config.schema.json`:
- Around line 603-625: The schema currently puts eventPublishers onto the shared
$defs/connectionsConfig so modules[n].connections can validate an
eventPublishers array that is never consumed; fix by splitting the connection
defs: create a new $defs/moduleConnections (copy of connectionsConfig minus the
eventPublishers property) and create a separate $defs/topLevelConnections that
includes eventPublishers (or keep the existing connectionsConfig for top-level).
Then change the module-level ref from `#/`$defs/connectionsConfig to
`#/`$defs/moduleConnections (ensure top-level still uses the def that contains
eventPublishers) so only the real top-level path validates eventPublishers.

In
`@tools/dev-workflow-v2/src/features/workflow/domain/workflow-implementing.spec.ts`:
- Around line 20-33: The tests for startSession only check repository presence
but ignore the transcript argument; update both specs that call
spec.given().when((wf) => wf.startSession('', ...)) to also assert the emitted
event includes transcriptPath set to the provided value (e.g.
expect(events[0]).toHaveProperty('transcriptPath', '') or
expect(events[0].transcriptPath).toBe('')), so the session-started event emitted
by startSession(...) is validated for transcriptPath as well.

In
`@tools/dev-workflow-v2/src/features/workflow/entrypoint/workflow-cli-hooks.spec.ts`:
- Around line 155-168: The test title says "Write" but the fixture uses
tool_name: 'Edit' so it doesn't exercise the Write path; update the test's
stdinJson to set tool_name: 'Write' (in the spec test that calls runHook with
tool_input: { file_path: 123 } and tool_use_id: 'tu-write-bad-type') so the hook
executes the Write branch and the existing expect toThrow('Expected string or
undefined in tool_input') validates the intended behavior for write-file path;
keep all other fields the same.

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

Inline comments:
In `@packages/riviere-extract-config/src/validation.ts`:
- Around line 171-199: The validator must also ensure the referenced custom type
actually extracts the required metadataKey used later; update
validateEventPublishers to check not only that publisher.fromType exists but
that the corresponding custom type's extract configuration includes the
metadataKey (the field accessed as publisher.metadata[metadataKey] in
detect-event-publisher-connections). Add or use a helper that collects custom
type extraction fields (e.g., create or call a collectCustomTypeExtractedFields
or extend collectAllCustomTypeNames to return a map from type name -> Set of
extracted field names), change validateEventPublishers signature to accept that
map, and in validateConnectionsConfig build that map and pass it in; if the
custom type for publisher.fromType is missing the metadataKey, return a
ValidationError at the same path (/connections/eventPublishers/{index}/fromType)
with a message instructing to add extract[metadataKey] to that custom type.
- Around line 158-169: collectAllCustomTypeNames currently skips modules with
'$ref', causing eventPublishers.fromType validation to false-negative when
customTypes live in referenced modules; fix by calling expandModuleRefs before
running this validation or by changing collectAllCustomTypeNames to operate on
the expanded modules returned by expandModuleRefs so it gathers customTypes from
resolved modules (ensure you reference collectAllCustomTypeNames and the
expandModuleRefs utility and run this after module expansion, so
eventPublishers.fromType is validated against the full, expanded module list).

In
`@packages/riviere-extract-ts/src/features/extraction/domain/connection-detection/async-detection/detect-event-publisher-connections.spec.ts`:
- Around line 122-133: The test currently only asserts the error message for
detectEventPublisherConnections in strict mode; update the spec to assert the
thrown error class as well by introducing a shared act function that invokes
detectEventPublisherConnections([publisher], eventPublisherConfig(), { strict:
true, repository: 'test-repo' }) and then use
expect(act).toThrow(ConnectionDetectionError) and
expect(act).toThrow(expect.objectContaining({ message:
expect.stringContaining('NonExistentEvent') })); apply the same pattern for the
other strict-mode cases mentioned (the blocks around the other lines) so each
test checks both ConnectionDetectionError and the expected message.

In
`@tools/dev-workflow-v2/src/features/workflow/domain/workflow-implementing.spec.ts`:
- Around line 62-73: The tests for registerAgent and handleTeammateIdle
currently only assert { pass: true } and can still pass if events were emitted;
update both specs to also assert that no events were produced after calling
spec.given().when((wf) => wf.registerAgent('lead','agent-1')) and
spec.given().when((wf) => wf.handleTeammateIdle('agent-1')) respectively—use the
test harness (spec) to fetch emitted events and assert zero events (e.g., check
event list length or emptiness) so the no-op side effect is verified alongside
the result.

In
`@tools/dev-workflow-v2/src/features/workflow/infra/persistence/workflow-definition.spec.ts`:
- Around line 54-63: Add a new spec that mirrors the existing "folds a valid
event onto state" test but for the session-started event: create an event object
with type 'session-started', at timestamp, and transcriptPath set to a test path
string; call WORKFLOW_DEFINITION.fold(WORKFLOW_DEFINITION.initialState(), event)
and then WORKFLOW_DEFINITION.buildWorkflow(state, makeWorkflowDeps()); finally
assert that workflow.getTranscriptPath() strictly equals the transcript path you
provided. Target the same test file and reuse WORKFLOW_DEFINITION.fold,
WORKFLOW_DEFINITION.initialState, buildWorkflow, makeWorkflowDeps, and
getTranscriptPath to ensure replay/rehydration coverage for transcriptPath.

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

Inline comments:
In @.claude/commands/create-tasks.md:
- Line 226: The current directive for the Opus subagent in the "Append
architectural annex via plan-review" step forces "Return only the one-line
summary", which can strip essential blocker details; update the behavior in the
instruction that spawns the subagent (the line referencing model `opus` and the
prompt to run `packages/riviere-role-enforcement/skills/plan-review.md`) so that
when the `plan-review` skill identifies a blocker it returns the full blocker
details (or a clearly marked "BLOCKER" section) instead of truncating to one
line, and ensure the runner relays that full blocker content verbatim to the
user while still relaying one-line summaries for non-blocker cases.

In `@packages/riviere-extract-config/src/validation.ts`:
- Around line 164-166: The loop that sets fieldsByType currently overwrites
existing sets when multiple modules define the same custom type; update the
logic in the for (const [typeName, rule] of Object.entries(module.customTypes))
loop to merge/union extracted field names instead of replacing them: if
fieldsByType has an existing Set for typeName, add Object.keys(rule.extract ??
{}) into that Set, otherwise create a new Set; use the symbols fieldsByType,
typeName, and rule.extract to locate and modify the code so metadataKey
validation sees all fields from all modules.

In
`@packages/riviere-extract-ts/src/features/extraction/domain/connection-detection/async-detection/detect-event-publisher-connections-missing-metadata.spec.ts`:
- Around line 101-119: Add a new spec that constructs a malformed runtime
array-value for the publisher metadata using JSON.parse (instead of a TS cast)
to include non-string values (e.g., 42, null) and assert the same
fallback/uncertain result; locate where tests call buildComponent and
detectEventPublisherConnections and add a case similar to the existing
"EmptyArraySender" one but with metadata:
JSON.parse('{"publishedEventType":[42,null]}'), then call
detectEventPublisherConnections(eventPublisherConfig(), defaultOptions) and
expect an objectContaining with target '_unresolved' and _uncertain mentioning
'"publishedEventType" metadata' to complete invalid-metadata coverage.

In
`@tools/dev-workflow-v2/src/features/workflow/infra/persistence/workflow-definition.spec.ts`:
- Around line 76-84: The test currently compares the returned result to the
exact same state object, so in-place mutations would go unnoticed; update the
test that uses WORKFLOW_DEFINITION.initialState() and
WORKFLOW_DEFINITION.fold(state, event) to assert immutability by creating an
independent expected state snapshot (e.g., clone or build a fresh initial state
via WORKFLOW_DEFINITION.initialState()) before calling fold, then assert the
returned result equals that fresh snapshot and also assert the original state
object remains deeply equal to the fresh snapshot to detect in-place mutations.

---

Outside diff comments:
In @.claude/commands/create-tasks.md:
- Line 284: Remove or update the obsolete validation guidance that references
the removed manual "Role Enforcement" section: locate the sentence "**Role
Enforcement:** If task touches enforced packages, Role Enforcement section is
complete with canonical config, expected roles/locations, and any new role
proposals" and either delete it or replace it with a short note pointing authors
to the automated annex workflow (e.g., "If task touches enforced packages, use
the automated annex workflow to provide canonical config and role details") so
the document no longer instructs authors to fill a non-existent manual section.

---

Duplicate comments:
In `@packages/riviere-extract-config/src/validation.ts`:
- Around line 160-162: The validation currently skips modules with '$ref' when
building customTypeFields (the loop over config.modules checking "'$ref' in
module" and module.customTypes), which causes valid fromType values in
connections.eventPublishers to be rejected; instead, ensure module references
are expanded before this validation by invoking expandModuleRefs (or performing
the expansion logic) on the config/modules collection prior to building
customTypeFields and before the similar validation block around
connections.eventPublishers (the other block referenced at lines ~204-210), so
that modules brought in via $ref are present and their customTypes are included
in validation used by parseExtractionConfig().

In
`@tools/dev-workflow-v2/src/features/workflow/infra/persistence/workflow-definition.spec.ts`:
- Around line 40-52: Two tests duplicate verifying that
WORKFLOW_DEFINITION.buildWorkflow(WORKFLOW_DEFINITION.initialState(),
makeWorkflowDeps()) yields currentStateMachineState 'IMPLEMENTING'; remove one
and keep a single test that calls WORKFLOW_DEFINITION.initialState(), builds the
workflow via WORKFLOW_DEFINITION.buildWorkflow(..., makeWorkflowDeps()) and
asserts workflow.getState().currentStateMachineState === 'IMPLEMENTING' to
consolidate coverage and reduce duplication.