feat: wire up --dry-run flag to prevent all writes#75
Conversation
When --dry-run is passed, the CLI now runs commands normally (fetches, processing) but suppresses all write operations: - Database writes: ReadOnlyTabularStorage wraps real storage, forwarding reads and no-oping puts/deletes/setupDatabase - File downloads: BootstrapDownloadTask logs what it would download and exits early - Cache writes: SecFetchFileOutputCache.saveOutput skips file writes - Init command: shows what config/dirs would be created without writing The SEC_DRY_RUN DI token is set in the preAction hook from the global --dry-run option, and createStorage wraps repositories automatically. runCommand prints a "Dry run" banner when active. https://claude.ai/code/session_01G4XE6pxyLRhiM2wAepVTZj
There was a problem hiding this comment.
Pull request overview
This PR implements a --dry-run flag for the CLI that suppresses all write operations while still allowing reads and processing to proceed normally. The flag is wired through a SEC_DRY_RUN DI token set in the preAction hook.
Changes:
- Adds
ReadOnlyTabularStoragewrapper that forwards reads and no-ops writes for database operations - Suppresses file writes in
SecFetchFileOutputCache.saveOutputand download operations inBootstrapDownloadTask - Registers
SEC_DRY_RUNtoken from the global--dry-runcommander option, with a dry-run banner inrunCommand
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/config/tokens.ts | Adds SEC_DRY_RUN service token |
| src/cli/isDryRun.ts | New helper to check dry-run state from the DI registry |
| src/cli/runCommand.ts | Replaces old no-op dry-run with a banner + action execution |
| src/cli/runCommand.test.ts | Updates test to verify banner is printed and action still runs |
| src/commands/index.ts | Registers SEC_DRY_RUN in preAction hook from global options |
| src/config/createStorage.ts | Wraps storage in ReadOnlyTabularStorage when dry-run is active |
| src/storage/ReadOnlyTabularStorage.ts | New decorator that no-ops all write methods on ITabularStorage |
| src/fetch/SecFetchFileOutputCache.ts | Skips file writes in saveOutput during dry-run |
| src/task/bootstrap/BootstrapDownloadTask.ts | Logs and skips downloads during dry-run |
| src/cli/groups/init.ts | Shows what would be created instead of writing during dry-run |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const dryRun = | ||
| globalServiceRegistry.has(SEC_DRY_RUN) && globalServiceRegistry.get(SEC_DRY_RUN); |
There was a problem hiding this comment.
The isDryRun() helper (defined in src/cli/isDryRun.ts) encapsulates exactly this globalServiceRegistry.has(SEC_DRY_RUN) && globalServiceRegistry.get(SEC_DRY_RUN) check. Consider using it here instead of duplicating the logic, for consistency and to have a single place to change if the dry-run detection mechanism evolves.
src/fetch/SecFetchFileOutputCache.ts
Outdated
| if (globalServiceRegistry.has(SEC_DRY_RUN) && globalServiceRegistry.get(SEC_DRY_RUN)) { | ||
| return; |
There was a problem hiding this comment.
The isDryRun() helper (defined in src/cli/isDryRun.ts) encapsulates exactly this check. Consider using it here instead of duplicating the logic, for consistency and to have a single place to change if the dry-run detection mechanism evolves.
src/config/createStorage.ts
Outdated
| } | ||
| return new SqliteTabularStorage(getDb(), table, schema, primaryKeyNames, indexes as any); | ||
|
|
||
| if (globalServiceRegistry.has(SEC_DRY_RUN) && globalServiceRegistry.get(SEC_DRY_RUN)) { |
There was a problem hiding this comment.
Same as in the other files: the isDryRun() helper from src/cli/isDryRun.ts encapsulates this exact check. Using it here would avoid duplicating the logic in yet another place.
| .command("init") | ||
| .description("Interactive first-run setup wizard") | ||
| .action(async () => { | ||
| const dryRun = parseGlobalOptions(parent).dryRun; |
There was a problem hiding this comment.
The init command is explicitly skipped in the preAction hook (line 28 of src/commands/index.ts: if (commandName === "init") return;), which means SEC_DRY_RUN is never registered in the global service registry for init. As a result, isDryRun() called inside runCommand will return false, and the "Dry run — no data will be written" banner won't be printed when running init --dry-run. The local dryRun variable here (parsed directly from commander) will work, but the banner in runCommand will be missing. Consider registering SEC_DRY_RUN before the early return for init, or at the top of the init action.
| const dryRun = parseGlobalOptions(parent).dryRun; | |
| const dryRun = parseGlobalOptions(parent).dryRun; | |
| if (dryRun) { | |
| process.env.SEC_DRY_RUN = "true"; | |
| } else { | |
| delete process.env.SEC_DRY_RUN; | |
| } |
|
@copilot open a new pull request to apply changes based on the comments in this thread |
… command banner (#77)
When --dry-run is passed, the CLI now runs commands normally (fetches,
processing) but suppresses all write operations:
reads and no-oping puts/deletes/setupDatabase
exits early
The SEC_DRY_RUN DI token is set in the preAction hook from the global
--dry-run option, and createStorage wraps repositories automatically.
runCommand prints a "Dry run" banner when active.
https://claude.ai/code/session_01G4XE6pxyLRhiM2wAepVTZj