fix: load vault configs from .data/vault/ directory and add unit tests#92
fix: load vault configs from .data/vault/ directory and add unit tests#92github-actions[bot] merged 1 commit intomainfrom
Conversation
Title: fix: load vault configs from .data/vault/ directory and add unit tests
Description:
Summary
- Fix vault model to load configurations from .data/vault/ directory instead of .swamp.yaml
- Update error messages to reference swamp vault create CLI commands
- Update documentation comment to reflect flat vault logical view structure
- Add unit tests for VaultConfig domain model (addresses PR feedback)
- Add integration test to catch vault storage location regressions
## Details
Bug Fix: Vault Configuration Loading
The vault model (swamp/lets-get-sensitive) was still loading vault configurations from the legacy .swamp.yaml
file format. This has been updated to use the YamlVaultConfigRepository which reads from the new
.data/vault/{type}/{id}.yaml storage location.
Before:
const configPath = `${context.repoDir}/.swamp.yaml`;
const configText = await Deno.readTextFile(configPath);
After:
const vaultRepo = new YamlVaultConfigRepository(context.repoDir);
const vaultConfigs = await vaultRepo.findAll();
Updated Error Messages
Error messages in VaultService now reference the CLI commands instead of the legacy .swamp.yaml format:
Before:
Add vault configuration to your .swamp.yaml file:
vaults:
my-vault:
type: aws
After:
Create a vault using:
swamp vault create aws my-vault
Documentation Fix
Updated the interface comment in repo_index_service.ts to reflect the flat vault logical view structure
(/vaults/{vault-name}/) instead of the nested structure (/vaults/{vault-type}/{vault-name}/).
New Unit Tests (PR Feedback)
Added vault_config_test.ts with tests for:
- createVaultConfigId() function
- VaultConfig.create() - creates config with current timestamp
- VaultConfig.fromData() - reconstructs from persisted data
- VaultConfig.toData() - converts to persistable format
- Round-trip serialization preserves data
- Handles empty and complex nested config objects
Regression Test
Added integration test vault configs are loaded from .data/vault/ directory to verify vaults are loaded from
the correct storage location.
Files Changed
Modified:
- src/domain/models/lets-get-sensitive/vault_model.ts - Load vaults from repository
- src/domain/vaults/vault_service.ts - Updated error messages
- src/domain/repo/repo_index_service.ts - Fixed documentation comment
- src/domain/vaults/vault_service_test.ts - Updated error message assertions
- src/domain/vaults/vault_expression_test.ts - Updated error message assertions
- src/domain/models/lets-get-sensitive/vault_model_test.ts - Use new vault storage location
- integration/vault_test.ts - Added regression test
New:
- src/domain/vaults/vault_config_test.ts - Unit tests for VaultConfig
Test plan
- VaultConfig.create() creates config with current timestamp
- VaultConfig.fromData() reconstructs from persisted data
- VaultConfig.toData() converts to persistable format
- Round-trip serialization preserves all data
- Vault configs are loaded from .data/vault/ directory (regression test)
- Error messages reference swamp vault create commands
- All 1254 tests pass
🤖 Generated with https://claude.ai/code
There was a problem hiding this comment.
PR Review: Load vault configs from .data/vault/ directory
Overall Assessment: This PR is well-implemented and follows the project's coding standards. The changes properly migrate vault configuration loading from the legacy .swamp.yaml format to the new .data/vault/{type}/{id}.yaml storage structure. Approved.
✅ No Blocking Issues
The code is ready to merge. All critical requirements are satisfied:
- TypeScript strict mode compliance ✓
- Named exports used throughout ✓
- No
anytypes ✓ - Comprehensive test coverage added ✓
- No security vulnerabilities identified ✓
📝 Suggestions (Non-blocking)
1. Type Safety Enhancement in VaultService.fromRepository
File: src/domain/vaults/vault_service.ts:32
The type cast vaultConfig.type as "aws" | "mock" | "local_encryption" could potentially fail silently if an invalid vault type is stored in the YAML config. Consider validating the type before registration or letting registerVault throw for unsupported types (which it already does).
// Current (works but relies on downstream validation)
type: vaultConfig.type as "aws" | "mock" | "local_encryption",
// Alternative: explicit validation or let registerVault handle it naturally
type: vaultConfig.type, // Let registerVault's switch statement handle invalid typesThis is minor since registerVault already throws for unsupported types.
2. DDD Observation: Repository in Domain Layer
File: src/domain/vaults/vault_service.ts:8
The VaultService imports YamlVaultConfigRepository directly from the infrastructure layer. In strict DDD, domain services typically depend on repository interfaces rather than concrete implementations. However, this is consistent with the existing codebase patterns and is pragmatic for this project size.
3. Error Handling Specificity
File: src/domain/vaults/vault_service.ts:36-38
The catch block silently swallows all errors. Consider logging at debug level (similar to vault_model.ts:79-82) for troubleshooting purposes:
} catch (error) {
// Log at debug level for troubleshooting
getLogger("vaults").debug`Failed to load vault configs: ${error}`;
}4. Test Isolation
File: integration/vault_test.ts
The new integration tests are thorough and well-structured. They properly test the end-to-end flow including vault creation, secret storage, and expression resolution.
✅ DDD Compliance
The changes follow DDD principles appropriately:
- VaultConfig is correctly modeled as an Entity (has unique ID, lifecycle tracked over time)
- YamlVaultConfigRepository is properly scoped to the aggregate root (VaultConfig)
- VaultService acts as a Domain Service coordinating vault operations
- The factory method
VaultService.fromRepository()properly encapsulates the complexity of loading configurations
✅ Security Review
- Secret values are properly escaped in
resolveVaultExpressionsto prevent injection attacks - No credentials or sensitive data exposed in error messages
- Vault configurations stored in
.data/directory (typically gitignored)
✅ Test Coverage
New tests added:
vault_config_test.ts- Unit tests for VaultConfig domain modelintegration/vault_test.ts- Regression test for storage location + E2E vault expression test
Existing tests updated to use new storage format.
LGTM! 🎉
…92) Create a dedicated publishing skill that enforces prerequisites as gates before allowing an extension push. The state machine walks through repo initialization, authentication, manifest validation, collective ownership, version bumping, formatting, and dry-run — each step must pass before the next begins. The final push requires explicit user approval. Moves publishing.md from swamp-extension-model to the new skill and updates all four extension creation skills plus the report skill to cross-reference it. Registers the new skill in BUNDLED_SKILLS so it ships with repo init and repo upgrade. Co-authored-by: Sean Escriva <webframp@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Title: fix: load vault configs from .data/vault/ directory and add unit tests
Description:
Summary
Details
Bug Fix: Vault Configuration Loading
The vault model (swamp/lets-get-sensitive) was still loading vault configurations from the legacy .swamp.yaml
file format. This has been updated to use the YamlVaultConfigRepository which reads from the new
.data/vault/{type}/{id}.yaml storage location.
Before:
const configPath =
${context.repoDir}/.swamp.yaml;const configText = await Deno.readTextFile(configPath);
After:
const vaultRepo = new YamlVaultConfigRepository(context.repoDir);
const vaultConfigs = await vaultRepo.findAll();
Updated Error Messages
Error messages in VaultService now reference the CLI commands instead of the legacy .swamp.yaml format:
Before:
Add vault configuration to your .swamp.yaml file:
vaults:
my-vault:
type: aws
After:
Create a vault using:
swamp vault create aws my-vault
Documentation Fix
Updated the interface comment in repo_index_service.ts to reflect the flat vault logical view structure
(/vaults/{vault-name}/) instead of the nested structure (/vaults/{vault-type}/{vault-name}/).
New Unit Tests (PR Feedback)
Added vault_config_test.ts with tests for:
Regression Test
Added integration test vault configs are loaded from .data/vault/ directory to verify vaults are loaded from
the correct storage location.
Files Changed
Modified:
New:
Test plan
🤖 Generated with https://claude.ai/code