fix: collective validator error message omits drivers and datastores#761
fix: collective validator error message omits drivers and datastores#761
Conversation
When `swamp extension push` fails collective validation, the error message referenced only "model types, vault types, and workflow names", leaving users confused when a push failed due to a driver or datastore type mismatch. Update the user-facing error message in extension_push.ts and the JSDoc in extension_collective_validator.ts to include driver types and datastore types, matching the validation logic that already checks them. Fixes #755 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Review Summary
Approved - Clean, minimal fix that correctly updates the error message to include driver types and datastore types.
Analysis
Code Quality: The changes are minimal and correct - the updated error message now accurately reflects what validateContentCollectives actually validates (models, vaults, workflows, drivers, and datastores at lines 54-102 of the validator).
TypeScript Strict Mode: N/A - no code changes, only string literals and JSDoc comments.
CLAUDE.md Compliance: Both files have proper AGPLv3 license headers and use named exports.
Domain-Driven Design: The validator is a proper domain service that enforces extension package invariants. The documentation fix improves ubiquitous language consistency.
Security: No concerns - purely message/documentation updates.
Suggestions (non-blocking)
- The test file (
extension_collective_validator_test.ts) includesdriversanddatastoresin themakeMetadatahelper but lacks explicit test cases for driver/datastore validation. Consider adding tests in a follow-up PR to ensure complete coverage.
LGTM - the fix correctly aligns the user-facing error message with the validator's actual behavior.
There was a problem hiding this comment.
Adversarial Review
I systematically attempted to break this code across all adversarial dimensions (logic, error handling, security, concurrency, data integrity, resource management, API contracts).
Verification
-
Validator logic confirmed: The
validateContentCollectivesfunction at lines 84-102 already validates drivers and datastores correctly. The PR accurately claims "no logic changes." -
Design consistency verified: The design doc (
design/extension.md:150-153) states: "All content types — model types, vault types, workflow names, driver types, datastore types — must use the same collective as the extension name." The updated error message now matches this spec. -
Error message location is singular: Grep confirms only one place uses this message pattern (
extension_push.ts:210), so no other locations are left inconsistent.
Critical / High
None.
Medium
None.
Low
- Pre-existing test gap (
extension_collective_validator_test.ts): The test file has no test cases for driver or datastore collective validation. ThemakeMetadatahelper defaults both to empty arrays, and no tests populate them. This is NOT introduced by this PR — the validator code is correct and would pass such tests if they existed — but it does mean the behavior mentioned in the updated error message is untested. Consider adding test coverage in a follow-up.
Verdict
PASS — This is a correct documentation/error message fix. The changes align the user-facing message with the existing validator behavior and design spec. No logic, security, or correctness issues found.
Summary
Fixes #755.
When
swamp extension pushfails collective validation, the error message only mentioned "model types, vault types, and workflow names" — it did not mention drivers or datastores. This was confusing for users whose push failed due to a driver or datastore type not matching the extension collective.Changes:
src/cli/commands/extension_push.ts— updated the user-facingUserErrormessage to include "driver types, and datastore types"src/domain/extensions/extension_collective_validator.ts— updated the JSDoc comment to reflect that drivers and datastores are also validatedNo logic changes; the validator already correctly checked driver and datastore types.
User Impact
Before this fix, a user pushing an extension with a mismatched driver or datastore type would see:
…with no mention of drivers or datastores, leaving them uncertain what was actually wrong.
After this fix, the message reads:
Test Plan
deno check— passesdeno lint— passesdeno fmt --check— passesdeno run test— 3198 tests pass