fix(drivers): resolve vault sentinels for out-of-process drivers (swamp-club#263)

[stack72]

Summary

• Resolve vault sentinel tokens (__SWAMP_VSEC_*__) in ExecutionRequest.methodArgs and ExecutionRequest.globalArgs before dispatching to out-of-process drivers (docker, custom extension drivers)
• The raw driver already resolves sentinels internally via DefaultMethodExecutionService.execute(), but the non-raw dispatch path sent sentinels through unresolved — models received literal sentinel strings instead of decrypted secret values
• Resolution operates on structuredClone data from the definition, so persisted state is never mutated

Test Plan

• Added unit test: registers a mock capture driver via DriverTypeRegistry, dispatches through executeWorkflow with a VaultSecretBag containing sentinel mappings, and asserts the captured ExecutionRequest has resolved plaintext values — not sentinels
• Verified against reproduction scenario: workflow step with ${{ vault.get(...) }} input under docker driver now resolves correctly (sentinel __SWAMP_VSEC_8c14a9ea_0__ → redacted ***)
• All 5528 existing tests pass
• deno check, deno lint, deno fmt clean

Closes swamp-club#263

🤖 Generated with Claude Code

[github-actions]

Code Review

No blocking issues found. This is a well-scoped bug fix with solid test coverage.

What the PR does well:

• Correctly identifies the gap: raw driver resolves vault sentinels internally, but out-of-process drivers (docker, custom) received unresolved sentinel strings
• Resolution placement is right — at the dispatch boundary in DefaultMethodExecutionService, before handing off to the driver. Drivers stay vault-unaware, which is the correct DDD boundary
• Definition integrity preserved: globalArguments and getMethodArguments() both return structuredClone copies, so the persisted definition is never mutated
• The isEmpty guard avoids unnecessary work when no secrets are present
• Test is thorough: registers a mock capture driver, dispatches through the full executeWorkflow path, and asserts resolved plaintext values in the captured ExecutionRequest
• Design doc update clearly explains the two driver paths and the cloning guarantee
• All imports stay within the domain layer — no libswamp boundary violations

Suggestions

1. Minor: assertEquals(capturedRequest !== null, true) at line 2888 could use assertExists(capturedRequest) from @std/assert for a more descriptive failure message. Not blocking — the existing pattern matches line 2711 in the same file, so this is consistent with current convention.

LGTM — safe to merge.

[github-actions]

Adversarial Review

Critical / High

None.

Medium

None.

Low

1. src/domain/models/method_execution_service_test.ts:2836 — Test-registered driver is never unregistered from the global driverTypeRegistry singleton. The test uses a randomized name (test-capture-${crypto.randomUUID().slice(0, 8)}), which prevents collisions, so this won't cause flakiness. However, across a large test suite execution, orphaned entries accumulate in the singleton's map. This is purely theoretical — the process exits after tests and the map entries are tiny.

2. src/domain/models/method_execution_service_test.ts:2888 — assertEquals(capturedRequest !== null, true) is a weaker assertion than assertNotEquals(capturedRequest, null). If the driver never fires, the failure message would say false !== true rather than the more descriptive null !== [something]. Cosmetic only — the subsequent capturedRequest! assertions would also fail, giving enough diagnostic info.

Verdict

PASS. The fix is correct and minimal. Both globalArguments and getMethodArguments() return structuredCloned data, so the mutations on executionRequest.methodArgs/executionRequest.globalArgs at lines 717-722 cannot corrupt persisted definition state. The guard pattern (secretBag && !secretBag.isEmpty) and resolveDeep usage are identical to the existing raw-driver path at lines 249-257, maintaining consistency. The resolveDeep method handles nested objects/arrays recursively and always returns a new object for record inputs, making the as Record<string, unknown> cast safe. The test directly verifies that resolved values (not sentinels) reach the driver, which is the exact bug scenario described in the PR.