Development - #1
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR significantly advances ops-backup-runner from a harness-first scaffold into an initial end-to-end backup runner implementation, adding a typed config foundation, a testable CLI, core backup/verify/restore/prune logic, and adapters for PostgreSQL-in-Docker dumping, local/S3 storage, age/none encryption, and Telegram failure notifications—alongside repository harness scripts and CI wiring.
Changes:
- Added a full CLI command surface (
doctor,backup,list,verify,restore,prune) backed by typed config loading/validation and ports/adapters. - Implemented core pipeline building blocks: artifact/manifest model, local backup pipeline, verification (including pg_restore restore-list), retention planning + prune execution, and temp workspace utilities.
- Introduced engineering harness scripts (architecture/docs/security/source hygiene/etc.), updated docs/templates, and added a GitHub Actions CI workflow running verification.
Reviewed changes
Copilot reviewed 51 out of 59 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| tool/verify.mjs | Adds a single-entry “verify” runner orchestrating formatting/lint/typecheck/tests/build + harness checks. |
| tool/lints/architecture-rules.json | Defines architecture boundaries (core/config isolation from adapters, etc.). |
| tool/check-source-hygiene.mjs | Adds repository-wide source hygiene checks (TODO/console/process.env boundary). |
| tool/check-security.mjs | Adds checks for secret/artifact files and runs pnpm audit. |
| tool/check-project-map.mjs | Enforces presence/content of AGENTS.md project map and key directories. |
| tool/check-env-example.mjs | Validates .env.example exists and doesn’t contain secret-looking values. |
| tool/check-docs.mjs | Enforces required docs and validates local markdown links. |
| tool/check-commit-message.mjs | Enforces scoped Conventional Commit format. |
| tool/check-architecture.mjs | Implements import scanning to enforce tool/lints/architecture-rules.json. |
| test/s3-storage.test.ts | Unit tests for S3 key building and adapter behavior via a mock client. |
| test/retention.test.ts | Tests retention planner and prune command (dry-run + execution safety). |
| test/postgres-docker.test.ts | Tests docker pg_dump argument construction, verification, and doctor checks. |
| test/notifications.test.ts | Tests secret-safe failure message formatting and Telegram adapter behavior. |
| test/encryption.test.ts | Tests encryption pipeline ordering, age adapter runner integration, and external-none guard. |
| test/config.test.ts | Tests config loading/validation, env reference resolution, redaction, and doctor behavior. |
| test/cli.test.ts | Expands CLI tests across help, selection, backup/list/verify/restore/prune, and notifications. |
| test/backup-verification.test.ts | Tests artifact verification including temp workspace cleanup and pg_restore checks. |
| src/storage/s3.ts | Adds S3-compatible storage adapter (AWS SDK v3) including upload/head/list/get/delete. |
| src/storage/local.ts | Adds local filesystem storage adapter with manifests/artifacts + key listing + deletion. |
| src/storage/.gitkeep | Preserves storage directory in VCS. |
| src/notifications/telegram.ts | Adds Telegram failure notifier adapter using curl via process runner. |
| src/notifications/policy.ts | Adds notification policy resolution and “should notify failure” logic. |
| src/notifications/message.ts | Adds failure message formatting + sensitive value redaction utilities. |
| src/notifications/.gitkeep | Preserves notifications directory in VCS. |
| src/encryption/policy.ts | Adds effective encryption config + guard against none-encryption on external storage. |
| src/encryption/none.ts | Adds a no-op encryption adapter. |
| src/encryption/age.ts | Adds age encryption adapter with injectable runner and env-based configuration. |
| src/encryption/.gitkeep | Preserves encryption directory in VCS. |
| src/dumpers/postgres-docker.ts | Adds PostgreSQL-in-Docker dumper + pg_restore validation + doctor checks. |
| src/dumpers/fake.ts | Adds a fake dumper for local/dev pipeline testing. |
| src/dumpers/.gitkeep | Preserves dumpers directory in VCS. |
| src/core/temp-workspace.ts | Adds temp workspace helper used by verification/backup flows. |
| src/core/retention.ts | Adds retention policy modeling and plan generation (keep/delete/unknown). |
| src/core/process-runner.ts | Adds a shared spawnSync-based runner abstraction for external commands. |
| src/core/ports.ts | Defines core ports (dumper/storage/encryption) and shared data contracts. |
| src/core/notifications.ts | Defines notification contracts (failure notifier/result/event). |
| src/core/manifest.ts | Defines the backup manifest schema/type with Zod. |
| src/core/backup-verification.ts | Adds artifact verification (checksum + restore + pg_restore list for Postgres dumps). |
| src/core/backup-job.ts | Implements local backup job: dump → gzip → encrypt → store → manifest. |
| src/core/artifact.ts | Adds backupId generation and sha256 helpers. |
| src/core/.gitkeep | Preserves core directory in VCS. |
| src/config/types.ts | Adds typed config aliases and load result modeling. |
| src/config/targets.ts | Adds target selection logic (all vs single id). |
| src/config/schema.ts | Adds full Zod schema for config (targets/defaults, adapters, retention, notifications). |
| src/config/redact.ts | Adds redaction for secret-shaped keys in config previews. |
| src/config/loader.ts | Adds YAML config loader + schema validation + error shaping. |
| src/config/env.ts | Adds env reference collection and runtime env validation helpers. |
| src/config/.gitkeep | Preserves config directory in VCS. |
| src/commands/doctor.ts | Implements doctor command (config validation + env + docker checks + external encryption policy). |
| src/commands/.gitkeep | Preserves commands directory in VCS. |
| src/cli.ts | Implements the CLI command dispatcher and command behaviors, plus JSON output mode. |
| README.md | Adds quickstart + commit format guidance + docs links. |
| pnpm-lock.yaml | Adds runtime dependencies (@aws-sdk/client-s3, yaml, zod) and lock updates. |
| package.json | Adds harness scripts, replaces verify with the new orchestrator, adds runtime deps. |
| eslint.config.mjs | Adds explicit process global for *.mjs linting. |
| docs/implementation-plan.md | Updates implementation plan with detailed phase evidence and status notes. |
| config/targets.example.yaml | Adds an example config with defaults, retention, and Telegram notification settings. |
| .github/workflows/ci.yml | Adds CI job to install deps, validate commit message, and run pnpm verify. |
| .github/pull_request_template.md | Adds a PR template focused on backup risk, evidence, and operational notes. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+29
to
+36
| const run = (command) => | ||
| new Promise((resolve) => { | ||
| const child = spawn(command[0], command.slice(1), { | ||
| shell: false, | ||
| stdio: "inherit", | ||
| }); | ||
| child.on("exit", (code) => resolve(code ?? 1)); | ||
| }); |
|
|
||
| ```bash | ||
| pnpm build | ||
| node dist/cli.js --version |
Comment on lines
+139
to
+146
| const issues = getTargetEnvReferences(config, target) | ||
| .filter((reference) => reference.requiredForEnabledTarget) | ||
| .filter((reference) => env[reference.name] === undefined) | ||
| .map((reference) => ({ | ||
| envName: reference.name, | ||
| owner: reference.owner, | ||
| message: `Missing required environment variable ${reference.name} for ${reference.owner}`, | ||
| })); |
| encryption.recipientEnv, | ||
| `${target.id}.encryption.recipientEnv`, | ||
| true | ||
| ); |
Comment on lines
+59
to
+60
| accessKeyIdEnv: envNameSchema.optional(), | ||
| secretAccessKeyEnv: envNameSchema.optional(), |
Comment on lines
+529
to
+533
| const manifests = selection.targets.flatMap((target) => { | ||
| const storage = getLocalStorageForTarget(configResult.config, target); | ||
| if (!storage.ok) return []; | ||
| return storage.storage.listManifests(target.id); | ||
| }); |
Comment on lines
+71
to
+78
| "Commands:", | ||
| " doctor Validate config and required runtime environment.", | ||
| " backup Run local fake backup pipeline or validate selection with --dry-run.", | ||
| " list List local backup manifests.", | ||
| " verify Verify local backup artifact integrity.", | ||
| " restore Restore a local backup artifact to a file.", | ||
| " prune Prune backups. Placeholder until retention exists.", | ||
| "", |
Comment on lines
+20
to
+41
| - name: Checkout | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 | ||
|
|
||
| - name: Setup pnpm | ||
| uses: pnpm/action-setup@v4 | ||
| with: | ||
| run_install: false | ||
|
|
||
| - name: Setup Node.js | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: 22 | ||
| cache: pnpm | ||
|
|
||
| - name: Install dependencies | ||
| run: pnpm install --frozen-lockfile | ||
|
|
||
| - name: Validate latest commit message | ||
| run: pnpm harness:commit | ||
|
|
Comment on lines
+697
to
+702
| const restoredBytes = restoreLocalBackupArtifact( | ||
| storage.storage.readArtifact(manifest), | ||
| getEncryptionForTarget(configResult.config, target) | ||
| ); | ||
| writeFileSync(outputPath, restoredBytes); | ||
|
|
Comment on lines
+353
to
+368
| telegram.botTokenEnv === undefined ? undefined : env[telegram.botTokenEnv]; | ||
| const chatId = | ||
| telegram.chatIdEnv === undefined ? undefined : env[telegram.chatIdEnv]; | ||
|
|
||
| if (botToken === undefined || chatId === undefined) { | ||
| return `Telegram failure notification skipped: missing Telegram runtime configuration for ${params.target.id}.`; | ||
| } | ||
|
|
||
| const notifier = createTelegramNotifier({ botToken, chatId }); | ||
| return notifier.notifyFailure({ | ||
| targetId: params.target.id, | ||
| stage: params.stage, | ||
| occurredAt: new Date(), | ||
| error: redactSensitiveText(params.error, [botToken, chatId]), | ||
| server: getServerName(), | ||
| }).message; |
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.
No description provided.