fix(cli): preserve @ prefix in direct type execution lookups (swamp-club#349)#1380
Conversation
…lub#349) Direct type execution (`swamp model @type method run ...`) stripped the @ prefix before creating a ModelType, producing a normalized key that didn't match the registry. The lookup missed, fell through to the auto-resolver, and conflicted with already-pulled extensions. Remove the unnecessary @-stripping in both the model (run.ts) and workflow (workflow_run.ts) direct type resolvers. Fix a downstream error message in direct_execution.ts that was re-adding the @ (now redundant). Closes swamp-club#349 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
CLI UX Review
Blocking
None.
Suggestions
None.
Verdict
PASS — Pure bug fix. The only user-visible change is the error message in direct_execution.ts, which previously would have shown a double-@ prefix ('@myorg/custom-model' rendered as '@@myorg/custom-model' due to the bug); it now correctly reflects what the user typed. No new flags, commands, help text, or output modes were changed.
There was a problem hiding this comment.
Code Review
Blocking Issues
None.
Suggestions
-
Redundant variable in
workflow_run.ts:285: After the fix,const typeStr = typeArg;is an identity assignment. Consider usingtypeArgdirectly in theModelType.createcall to remove the unnecessary intermediate. -
Redundant variable in
run.ts:249: Similarly,const typeArg = input.typeArg;just shadows the input field. Could useinput.typeArgdirectly. -
Duplicated test helpers in
integration/direct_type_execution_test.ts:initializeTestRepoandrunCliCommandare re-implemented locally despite being already exported fromtest_helpers.ts. The localinitializeTestRepoadds.swamp/logswhich the shared version doesn't include — if that directory is needed, consider adding it to the shared helper; otherwise import the existing one. (Note:withTempDirand the local prefix are fine to keep local since the shared helpers don't provide that.)
Summary
Clean, well-scoped bug fix. The root cause (stripping @ before ModelType.create produced a different normalized key for registry lookup) is correctly identified and fixed in both the model and workflow direct-type paths, plus the downstream error message. Test coverage is solid with both a unit test proving the lookup and integration tests verifying end-to-end behavior for both model and workflow paths. DDD usage is appropriate — ModelType is correctly used as a Value Object throughout.
There was a problem hiding this comment.
Adversarial Review
Critical / High
None found.
Medium
None found.
Low
-
integration/direct_type_execution_test.ts:97— Extension model code usesnpm:zod@4inline import. The integration test'sEXTENSION_MODEL_CODEfixture usesimport { z } from "npm:zod@4"which will be dynamically imported at test time. If the npm cache is cold and network is unavailable, this could cause a flaky test failure. In practice this is mitigated by zod already being a project dependency (cached from the lockfile), and this pattern matches existing integration tests — so it's theoretical. No action needed. -
src/libswamp/models/run_test.ts:768— No-opcreateAndSaveDefinitionmock. The mockasync (_type, _def) => {}silently discards the save. This is correct for what the test is verifying (theauto_createdevent is yielded with the rightmodelType), but means the test doesn't verify that the persisted definition also carries the correct@-prefixed type. The integration tests cover this end-to-end path, so this is acceptable.
Verdict
PASS — Clean, minimal bug fix. The root cause was that run.ts and workflow_run.ts both stripped the @ prefix from typeArg before passing it to ModelType.create(). Since ModelType.normalize() preserves @ (it's semantically significant for user-namespace resolution), stripping it produced a different normalized key (e.g., myorg/custom-model vs @myorg/custom-model), causing registry lookup misses. The fix correctly removes the stripping in both paths and fixes the now-redundant @ re-addition in the direct_execution.ts error message. The change is safe for non-@ types too: in the CLI path, isDirectExecution gates on startsWith("@") so typeArg is always @-prefixed; in the workflow path, non-@ types were already unaffected by the old stripping logic. Test coverage is thorough — unit test for the core path and two integration tests for the regression scenario.
Summary
swamp model @type method run ...) was stripping the@prefix before creating aModelType, producing normalized keyswamp/issue-lifecycleinstead of@swamp/issue-lifecycle. The registry lookup missed, fell through to the auto-resolver, and conflicted with already-pulled extensions.@-stripping in both the model (run.ts) and workflow (workflow_run.ts) direct type resolvers.direct_execution.tsthat was re-adding the@(now redundant sincetypeArgcarries it).Closes swamp-club#349
Test Plan
run_test.ts— direct execution resolves@myorg/custom-modelthrough the registry@typedirect execution resolves local extension without auto-resolver cascade@typeresolves local extension without auto-resolver cascadedeno check,deno lint,deno fmtall cleandeno run compilesucceeds🤖 Generated with Claude Code