feat(config): add per-project container snapshot policy#1042
Merged
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
nhopeatall
approved these changes
Mar 24, 2026
Collaborator
nhopeatall
left a comment
There was a problem hiding this comment.
Summary
LGTM — clean, well-structured config plumbing for per-project container snapshot policy.
The implementation correctly follows existing patterns throughout:
- Migration: Simple nullable
ALTER TABLE ADD COLUMN— no behavior change for existing projects, null semantics are correct for fallback-to-defaults. - Schema pipeline:
snapshotEnabledandsnapshotTtlMsflow correctly throughProjectRow→buildBaseProjectFields()→ProjectConfigRaw→ProjectConfigSchemawith proper null→undefined coercion via??. - Critical edge case:
snapshotEnabled: falseis correctly preserved — the??operator doesn't coalesce onfalse, so an explicit disable at the project level won't be silently ignored. - Router defaults: Env var parsing follows the established
Number(...) || defaultpattern. All four defaults are sensible (disabled, 24h TTL, 5 max, 10GB max size). - Refactoring: Extracting
buildBaseProjectFields()is clean — all fields verified accounted for, andagentModels/agentEngineSettingscorrectly remain inmapProjectRowsince they depend onprojectAgentConfigs. - Tests: Good coverage of the null/true/false/value matrix for snapshot fields across mapper, repository, and router config layers.
- Bonus fix:
maxInFlightItems: nullwas correctly added tobaseProjectRowtest fixture (was missing previously).
All CI checks pass.
🕵️ claude-code · claude-opus-4-6 · run details
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements per-project container snapshot policy stored in config so that router workers can resolve snapshot behavior consistently.
snapshot_enabled(BOOLEAN) andsnapshot_ttl_ms(INTEGER) columns to theprojectstable via migration0046_add_snapshot_policy.sql. Null means fall back to router defaults — no behavior change for existing projects.ProjectRow,ProjectConfigRaw, andProjectConfigSchemasosnapshotEnabledandsnapshotTtlMsflow throughmapProjectRow()→loadConfigFromDb()→loadProjectConfig()alongside all existing project settings.RouterConfigandrouterConfigbacked by env vars (SNAPSHOT_ENABLED,SNAPSHOT_DEFAULT_TTL_MS,SNAPSHOT_MAX_COUNT,SNAPSHOT_MAX_SIZE_BYTES). Project-level values override these defaults when set; null/missing project values fall back to router defaults.configMapper.test.ts(null→undefined mapping, explicit true/false, TTL value),configRepository.test.ts(end-to-end schema mapping including null fallback), androuter/config.test.ts(all four snapshot defaults).buildBaseProjectFields()frommapProjectRow()to keep the function within biome's cognitive complexity limit.Trello card: https://trello.com/c/IIIkvePP/555-as-the-system-i-need-per-project-container-snapshot-policy-stored-in-config-so-that-router-workers-can-resolve-snapshot-behavior
Test plan
npm test)npm run lint)npm run typecheck)configMapper.test.ts— snapshot field null/true/false/value cases all passconfigRepository.test.ts— snapshot config mapping group all passrouter/config.test.ts— snapshot default values all pass🤖 Generated with Claude Code
🕵️ claude-code · claude-sonnet-4-6 · run details