Skip to content

v0.26.5 feat: destructive operation guard end-to-end (sources + pages + autopilot purge)#600

Merged
garrytan merged 7 commits into
masterfrom
garrytan/pr-595-v0.26.5
May 4, 2026
Merged

v0.26.5 feat: destructive operation guard end-to-end (sources + pages + autopilot purge)#600
garrytan merged 7 commits into
masterfrom
garrytan/pr-595-v0.26.5

Conversation

@garrytan

@garrytan garrytan commented May 3, 2026

Copy link
Copy Markdown
Owner

Summary

Closes the destructive-guard posture across every gbrain destructive surface. Supersedes #595 (cherry-picked as the foundation; this PR expands to the full posture from the CEO + Eng review).

  • Source-level guard (from feat(v0.26.5): Destructive operation guard — impact preview, confirmation gate, soft-delete #595): impact preview + --confirm-destructive flag for gbrain sources remove; archive/restore/archived/purge subcommands; soft-delete with 72h TTL
  • Page-level soft-delete (new): MCP delete_page op rewires from hard-delete to soft-delete; new restore_page (scope:write) + purge_deleted_pages (scope:admin, localOnly:true); include_deleted flag on get_page/list_pages
  • Schema migration v33: pages.deleted_at + partial purge index; sources.archived BOOLEAN promoted from JSONB key (closes Issue 5 reserved-key footgun); JSONB→column backfill; forward-reference bootstrap extended
  • Search visibility: buildVisibilityClause filter applied across searchKeyword/searchKeywordChunks/searchVector for both engines (column-based, no JSONB containment in hot path; not bypassed by detail=high)
  • Autopilot purge phase (9th in ALL_PHASES): calls purgeExpiredSources + engine.purgeDeletedPages(72) so the 72h TTL is real, not honor-system; manual escape hatch via new gbrain pages purge-deleted CLI

Verdict on the cherry-pick alone (PR #595)

The CEO + Eng review flagged three internal correctness gaps in the as-cherry-picked branch:

  1. "Removed from search" was false: softDeleteSource flipped config.federated=false but no search code filtered archived:true. Closed by the visibility-clause sweep.
  2. 72h TTL was honor-system: purgeExpiredSources was never called by autopilot. Closed by the new cycle phase.
  3. Zero tests for 334 LOC of safety-critical code: closed by 30+ new tests across 4 files.

Plus one scope-too-narrow finding: the motivating incident was an agent. delete_page over MCP was wide open. Closed by the page-level soft-delete + restore_page op.

Test Coverage

  • New unit + integration tests across test/destructive-guard.test.ts, test/pages-soft-delete.test.ts, test/sql-ranking.test.ts, test/schema-bootstrap-coverage.test.ts. 90 pass / 0 fail on the new surface.
  • Full bun test: 16 pre-existing failures inherited from v0.26.2 (sync-parallel, queue-child-done, eval-capture macOS WASM OOM, etc — all in TODOS.md under "Fix 22 pre-existing test failures unrelated to OAuth"). Zero new failures introduced by v0.26.5.

Q3 contract (eng-review IRON RULE)

get_page(slug) returns null for soft-deleted pages by default. get_page(slug, {include_deleted: true}) surfaces them with deleted_at populated. Same flag for list_pages. Locked in via regression test in test/pages-soft-delete.test.ts.

OAuth scope contract

  • delete_page: scope: 'write' (unchanged) — agents can still call it, soft-delete makes it safe
  • restore_page: scope: 'write' — agents can self-correct mistakes within the 72h window
  • purge_deleted_pages: scope: 'admin', localOnly: true — operators only, never reachable over gbrain serve --http

Pre-Landing Review

CEO + Eng reviews ran in plan mode (see ~/.claude/plans/take-a-look-and-gentle-pine.md). All 10 findings either escalated to user (Q3=A, Issue 5=a) or defaulted with reasoning. No new findings added in /ship.

TODOS

Three follow-ups filed under "destructive-guard (v0.26.5 follow-up)":

  • Adjacent 2: storage objects orphan on hard purge (P2, v0.26.6 candidate)
  • Adjacent 3: sources remove/purge doesn't acquire SYNC_LOCK_ID (P3)
  • Auth revoke-client should adopt the impact-preview pattern (P3)

Documentation

  • CLAUDE.md: extended Key Files (destructive-guard.ts, new BrainEngine methods, new ops, purge phase, partial-index strategy note) and Commands sections
  • CHANGELOG.md: rewrote v0.26.5 entry head-to-toe to reflect the full posture
  • llms-full.txt regenerated via bun run build:llms

Test plan

  • bun run typecheck — PASS
  • bun run build:schema — regenerated schema-embedded.ts
  • bun run build:llms — regenerated llms-full.txt
  • Targeted test run: 90 pass / 0 fail (destructive-guard, pages-soft-delete, sql-ranking, schema-bootstrap-coverage, build-llms)
  • Full bun test: 16 pre-existing failures match TODOS.md inventory; zero new failures
  • Spin up real Postgres + run bun run test:e2e for the Postgres-specific paths (HNSW two-stage CTE, CONCURRENTLY index)
  • gbrain upgrade smoke test: verify v33 migration runs cleanly on a v0.26.2 brain
  • MCP scope check: register read,write OAuth client → call delete_page → assert soft-delete + 200; call purge_deleted_pages → assert 403 (admin-scoped, localOnly)

Closes

Supersedes #595 — same source-level posture plus page-level + autopilot + tests + docs.

🤖 Generated with Claude Code


View in Codesmith
Need help on this PR? Tag @codesmith with what you need.

  • Let Codesmith autofix CI failures and bot reviews

garrytan-agents and others added 7 commits May 3, 2026 13:35
…tion gate, soft-delete

Three-layer protection against accidental data loss:

1. **Impact preview**: Every destructive operation (sources remove, purge)
   now shows a formatted preview of exactly what will be destroyed —
   page count, chunk count, embedding count, file count — BEFORE acting.

2. **--confirm-destructive flag**: `--yes` alone is no longer sufficient
   when a source has data. Must pass `--confirm-destructive` to proceed
   with permanent deletion. Prevents scripted/reflexive destroys.

3. **Soft-delete with 72h TTL**: New `gbrain sources archive <id>`
   hides a source from search and federation without destroying any data.
   Data preserved for 72 hours. Restorable via `gbrain sources restore <id>`.
   Expired archives purged via `gbrain sources purge`.

New subcommands:
  - `gbrain sources archive <id>` — soft-delete (hide, preserve 72h)
  - `gbrain sources restore <id>` — un-archive, re-federate
  - `gbrain sources archived` — list soft-deleted sources + TTL
  - `gbrain sources purge [<id>] [--confirm-destructive]` — permanent delete

Behavioral changes:
  - `sources remove` with data now requires `--confirm-destructive` (not just `--yes`)
  - `sources remove --dry-run` shows full impact preview without side effects
  - Impact box format shows source name, id, and all cascade counts

New files:
  - src/core/destructive-guard.ts — impact assessment, confirmation gate,
    soft-delete/restore/purge logic, display formatters
Bump VERSION + package.json to 0.26.5 and add the v0.26.5 CHANGELOG entry
on top of the destructive-guard feature commit cherry-picked from PR #595.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…bility

Closes the destructive-guard posture across every gbrain destructive surface.
PR #595 cherry-pick covered the CLI source-remove path; this commit closes
the higher-velocity MCP `delete_page` agent footgun and the three internal
correctness gaps the CEO+Eng review surfaced:

- Gap 1: archived sources were not actually filtered from search. Now they
  are, via `buildVisibilityClause` in `searchKeyword`/`searchKeywordChunks`/
  `searchVector` for both engines.
- Gap 2: 72h TTL was honor-system. Now wired into a new autopilot `purge`
  phase (9th in ALL_PHASES) that calls `purgeExpiredSources` + `engine.
  purgeDeletedPages(72)`. Manual escape hatch: `gbrain pages purge-deleted`.
- Gap 3: zero tests for safety-critical code. ~30 cases now in
  `test/destructive-guard.test.ts`, `test/pages-soft-delete.test.ts`, and
  `test/sql-ranking.test.ts` covering the boundary truth table, JSONB→column
  migration, soft-delete/restore/purge round-trip, multi-source isolation,
  cascade verification, and the Q3 IRON-rule contract test.

Schema migration v33 (`destructive_guard_columns`): adds `pages.deleted_at`
+ partial purge index, promotes `archived` from `sources.config` JSONB to
real columns (`sources.archived BOOLEAN`, `archived_at`, `archive_expires_at`),
backfills any pre-v0.26.5 JSONB shape. Engine-aware: Postgres uses CREATE
INDEX CONCURRENTLY, PGLite uses plain CREATE INDEX. Forward-reference
bootstrap extended in both engines so pre-v0.26.5 brains don't crash on the
embedded-schema replay.

BrainEngine surface: new `softDeletePage` / `restorePage` /
`purgeDeletedPages` methods + `includeDeleted` flag on `getPage`/`listPages`.
MCP ops: `delete_page` rewired to soft-delete (description string updated);
new `restore_page` (scope: write) + `purge_deleted_pages` (scope: admin,
localOnly: true).

Q3 contract (eng-review lynchpin): `get_page(slug)` returns null for
soft-deleted by default; `get_page(slug, {include_deleted: true})` surfaces
the row with `deleted_at` populated. Same flag for `list_pages`. Mirrors
the search-filter contract end-to-end.

Issue 5 (eng-review): `archived` is now a real column on `sources`, not a
JSONB key. No reserved-key footgun. Faster filter. Visibility clause
compiles to a column lookup, not JSONB containment.

Verification:
- bun run typecheck: PASS
- bun run build:schema + bun run build:llms: regenerated
- targeted test runs: 90 pass / 0 fail across destructive-guard,
  pages-soft-delete, sql-ranking, schema-bootstrap-coverage, build-llms
- full bun test: 16 pre-existing failures inherited from v0.26.2 (sync,
  sync-parallel, queue-child-done, etc — already filed in TODOS.md as
  "Fix 22 pre-existing test failures unrelated to OAuth")

CHANGELOG, CLAUDE.md (Key Files + Commands), TODOS.md updated. The plan
file at ~/.claude/plans/take-a-look-and-gentle-pine.md captures the full
review trail (CEO=C, Eng-Q3=A, Eng-Issue5=a, 8 defaults applied).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…--confirm-destructive

Two CI failures from the v0.26.5 ship:

1. **Tier 1 (Postgres E2E):** `E2E: Page CRUD > delete_page removes page and
   others survive` failed because `delete_page` now soft-deletes (sets
   deleted_at) but `getStats.page_count` was still counting all rows. The
   test seeds 16 pages, deletes one, and asserts page_count is 15. Fix:
   `getStats` now filters `WHERE deleted_at IS NULL` for page_count in both
   engines. This matches the visibility-filter contract — soft-deleted pages
   are hidden everywhere the user looks (search, get_page, list_pages, stats).
   Chunks and links stay raw because they still occupy storage until the
   autopilot purge phase runs.

2. **Test 2 (PGLite unit):** `multi-source-integration.test.ts:184` and
   `e2e/multi-source.test.ts:274` called `runSources(engine, ['remove', X,
   '--yes'])` against populated sources. v0.26.5's destructive guard rejects
   `--yes` alone on populated sources and calls `process.exit(5)`, which
   killed the bun test runner mid-suite (CI exit 5). Both test sites now
   pass `--confirm-destructive` per the v0.26.5 contract.

Verification: 115/0 pass across destructive-guard, pages-soft-delete,
sql-ranking, schema-bootstrap-coverage, sources, repos-alias, and
multi-source-integration test files. typecheck PASS.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…26.5

# Conflicts:
#	CHANGELOG.md
#	VERSION
#	package.json
#	src/core/migrate.ts
CI failure: `runCycle — yieldBetweenPhases hook` tests asserted exactly 8
phases. v0.26.5 added the autopilot `purge` phase as the 9th, so:

- `test/core/cycle.test.ts:381` — `hookCalls` is now 9 (one yield per phase)
- `test/core/cycle.test.ts:392` — `report.phases.length` is now 9
- `test/e2e/cycle.test.ts:101` — same update for the dry-run E2E

The `purge` phase invocation was already visible in the failing log output:
the cycle ran 9 phases end-to-end; the test assertions hadn't been updated.

Verification: bun run typecheck PASS. cycle.test.ts: 28/0 pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…26.5

# Conflicts:
#	CHANGELOG.md
#	TODOS.md
#	VERSION
#	package.json
@garrytan garrytan merged commit 0de9eb6 into master May 4, 2026
7 checks passed
garrytan added a commit that referenced this pull request May 4, 2026
Master added two more commits:
- d97f159 v0.26.4 test: parallel unit-test loop (12x speedup, #605)
- 0de9eb6 v0.26.5 feat: destructive operation guard end-to-end (#600)

Resolved three conflicts (all version bookkeeping):

- VERSION: kept 0.26.6 (this branch's version, ahead of master's 0.26.5)
- package.json: kept 0.26.6
- CHANGELOG.md: my v0.26.6 entry on top, then master's new v0.26.5 +
  v0.26.4 blocks below. Final order: 0.26.6 → 0.26.5 → 0.26.4 →
  0.26.3 → 0.26.2 → 0.26.1 → 0.26.0 (top to bottom, contiguous).

Schema-drift gate sanity check post-merge:
- Master's v0.26.5 destructive-guard work added pages.deleted_at
  (with partial index pages_deleted_at_purge_idx) and three columns
  on sources (archived, archived_at, archive_expires_at). All four
  are present in BOTH src/schema.sql AND src/core/pglite-schema.ts —
  master kept them in lockstep, so the gate is satisfied automatically.
- access_tokens.id is still UUID DEFAULT gen_random_uuid() in both
  engines (my v0.26.3 D6 fix preserved across the merge).
- Typecheck clean, schema-diff unit tests 17/17 pass, privacy script
  clean (master's v0.26.4 work fixed the Wintermute references in
  mounts-cache.ts that I had patched earlier — converged independently).

Regenerated llms-full.txt to match the merged CHANGELOG.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants