Add owner-directed archive removal and durable exclusions - #217
hannesrudolph wants to merge 17 commits into
Conversation
…backfill # Conflicts: # CHANGELOG.md # docs/commands.md # internal/github/history.go # internal/github/history_test.go # internal/syncer/graphql_history_test.go # internal/syncer/syncer.go
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
clawsweeper
Bot
added
P0
|
Codex review: blocked before merge. Reviewed September 29, 2026, 11:09 PM ET / September 30, 2026, 03:09 UTC (Revision 5). ClawSweeper reviewWhat this changesAdds a Regression provenancePossible regression — suspected (reviewed change). No predecessor PR is attributed. Merge readiness⛔ Blocked before merge - 6 items remain Keep this draft open. Current main contains the merged analytics base from #216, but not the owner-directed purge command. A source-proven archive-loss defect still blocks this branch. Priority: P0 Review scores
Verification
How this fits togetherGitcrawl collects GitHub conversations into a local SQLite archive. The new command changes that archive and its collection policy; portable export later reads the same database to create a publishable snapshot. flowchart LR
A[Owner selects conversations] --> B[Purge plan and apply]
B --> C[Local archive and exclusions]
D[GitHub conversations] --> E[Collector checks exclusions]
E --> C
C --> F[Portable export checks policy]
F --> G[Snapshot or refusal]
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Reject owner exclusions while the source is still protected by the consume-source ownership lock, preserve its bytes on refusal, and verify upgrade of a released-schema archive. Do we have a high-confidence way to reproduce the issue? Yes. Source order gives a high-confidence path: create an archive with an owner exclusion and export it with Is this the best way to solve the issue? No. Refusing publication of local policy is reasonable, but the refusal must happen before the destructive handoff and be checked with a source-preservation regression. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: not found in the target repository. Codex review notes: model internal, reasoning high; reviewed against 84ed753d157f. LabelsLabel changes: No label changes. Label justifications:
EvidenceAcceptance criteria:
What I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (4 earlier review cycles)
|
Add purge-threads preview/apply for an exact owner-selected local archive set. Bind apply to a stable hash of the selected stored rows, then atomically remove content, revisions, derived rows and retry work while recording durable collection exclusions. Preserve shared actors and unrelated threads; refuse ambiguous identities, unsafe native-ID reuse and unsupported shared/blob/portable/cloud targets. Adapted and narrowed from #217 by Hannes Rudolph. Remove portable-mirror mutation and refresh ownership, arbitrary request labels, and idempotent reapply plumbing. Keep owner exclusions separate from provider deletion and successful recovery. Validation: synthetic selection, stale-plan, multi-target rollback, collection and unsafe-target regression tests; full make check on one AWS Crabbox lease with 85.6% coverage and six-platform snapshots; isolated Codex review clean through P2; exact-head GitHub CI verified before merge. Prepare 0.15.0 release notes with all changelog bullets and empty Unreleased. Release dispatch remains blocked on Apple's required agreement; no tag is created here. Co-authored-by: Hannes Rudolph <49103247+hannesrudolph@users.noreply.github.com>
|
Thanks, Hannes, for the owner-removal and durable-exclusion design and the careful safety groundwork. We’ve landed the focused implementation in #227, with your co-author credit and changelog thanks preserved. The landed command keeps atomic removal, collection exclusions, shared-actor preservation, and unsafe-target refusals, and adds a content-bound plan ID checked at apply time. We narrowed this first version to native local archives, leaving portable-mirror ownership/refresh out of scope. Synthetic rollback and normal-crawl regression tests, the full remote gate, and exact-head CI passed. Closing this stacked draft in favor of the merged PR. The 0.15.0 notes are prepared; tagging is waiting for Apple’s notarization agreement to be accepted. |
2 participants
Local hiding leaves conversation bodies, revisions and retry work in an archive. Add an explicit
purge-threadsplan/apply command that atomically removes an exact owner-selected set and persists exclusions respected by normal collection. Owner removal is reported separately from provider deletion or successful recovery; core coverage remains independent.The command preserves shared actors, rejects unsafe native ID reuse, and refuses blob-backed targets, retained workflow dependencies and shared cluster state, and supports an existing managed portable mirror through its current ownership lock and local-write preservation policy. Failed mirror applies retain the original bytes and refresh eligibility. Local exclusion metadata cannot be published through portable export.
The exclusion namespace follows the explicit collector target; changing that target requires carrying its policy. Repository rename/transfer migration is outside this command.
Stacked on #216. No merge requested; GitHub Actions are unchanged.
Validation:
make test-coverage: 85.2%, with the unchanged 85% gate.make tidy-check fmt lint smoke test-release docs snapshot.Exact pushed head
6c9c1bea7d094397dcdfecfb50e693460592a5aapassed Linux/macOS Go coverage, Windows portable filesystem, Docs, Docker and secret-scanning checks. Signed local activation and two advancing ordinary core checkpoints passed; owner exclusions remain distinct from review-provider completeness.