fix: collect writer handles in RawExecutionDriver for extension model dataArtifacts

[stack72]

Summary

Fixes #907

RawExecutionDriver ignored getHandles() from both createResourceWriter and createFileWriterFactory. After method execution, it relied on result.dataHandles from the method's return value — which is undefined for extension models that return {}.

Impact: All extension models using the raw driver produced empty dataArtifacts in their run output. This broke:

• The CLI JSON output view (no artifacts shown)
• Method-summary reports (empty data output table)
• Output log artifact lookup (output_logs.ts filters on dataArtifacts)

The data itself was persisted correctly to disk (writeResource works in-process), which is why this wasn't caught earlier — swamp data get, swamp data list, and CEL expressions like data.latest() all resolve from the filesystem and worked fine.

Fix: Destructure getHandles from both writer factories, then use the combined writer handles as fallback when result.dataHandles is empty. Built-in models that return explicit dataHandles are unaffected — the fallback only triggers when the method returns no handles.

• src/domain/drivers/raw_execution_driver.ts — wire up getHandles from both factories, use as fallback
• src/domain/drivers/raw_execution_driver_test.ts — 3 new tests: writer handles collected when method returns none, explicit handles take precedence, empty when no writes

Test Plan

• 3 new unit tests for RawExecutionDriver covering the fix and backward compatibility
• Full test suite passes (3680 tests, 0 failures)
• deno check, deno lint, deno fmt all clean

🤖 Generated with Claude Code

[github-actions]

Code Review

Clean, focused bug fix. The change correctly wires up getHandles() from both createResourceWriter and createFileWriterFactory, using the collected writer handles as a fallback when result.dataHandles is empty. The fallback logic (result.dataHandles?.length ? result.dataHandles : writerHandles) preserves backward compatibility for built-in models that return explicit handles.

Blocking Issues

None.

Suggestions

None — this is a well-scoped fix with good test coverage (3 tests covering the fix, backward compat, and empty case). The test mocking approach is consistent with other domain-layer tests in the repo.

DDD: RawExecutionDriver correctly acts as a domain service orchestrating writer factories and method execution. The fix uses existing domain collaborator APIs (getHandles) rather than introducing new coupling.

CLAUDE.md: License header present, test file co-located, naming convention followed, no any types, no libswamp boundary violations.

[github-actions]

Adversarial Review

Medium

1. src/domain/drivers/raw_execution_driver.ts:150-152 — dataHandles: [] treated same as undefined

   The fallback condition result.dataHandles?.length ? result.dataHandles : writerHandles treats an explicit empty array [] identically to undefined/missing. If a method deliberately returns { dataHandles: [] } to signal "I intentionally produced zero outputs" while also calling writeResource as a side effect (e.g. writing a tombstone or marker that shouldn't appear in outputs), the driver would incorrectly surface the writer handles.

   In practice this doesn't happen today — methods that return dataHandles: [] in the codebase are test stubs that don't call writeResource, and real extension models return {} (no property at all). But the semantic distinction between "I have no outputs" vs "I didn't track outputs" is collapsed. A comment clarifying the intent would prevent future confusion, though this is not blocking since no current code path triggers the issue.

2. src/domain/drivers/raw_execution_driver_test.ts:161-175 — first test asserts outputs.length > 0 but the mock save() never produces a real handle

   The createMockDataRepo().save() returns { version: 1 } but the test's mock writeResource call at line 164 goes through createResourceWriter which calls the real DataWriter.writeText(). Since the mock repo's allocateVersion returns contentPath: "/tmp/mock", this will attempt to write to /tmp/mock on disk. If /tmp/mock is not writable or doesn't exist as a directory parent, the test would fail with an OS error rather than a clear assertion failure. This is a test fragility issue, not a production concern. The test works today but is implicitly dependent on /tmp being writable.

Low

1. src/domain/drivers/raw_execution_driver.ts:146-149 — writerHandles always computed even when unused

   When result.dataHandles is non-empty, the getResourceHandles() and getFileHandles() calls (array copies via spread) are wasted work. This is negligible overhead — just two small array allocations — but if performance profiling ever flags this hot path, the calls could be deferred behind the condition check.

Verdict

PASS — The fix is correct for its stated purpose. The fallback logic properly handles the extension model case (returns {}) and preserves backward compatibility for built-in models (returns populated dataHandles). The data flow through processDriverOutputs back in method_execution_service.ts correctly picks up the driver's outputs. The edge case in Medium #1 is theoretical and doesn't affect any current code path.