Repository navigation
fix(backup): reconcile legacy profiles and ignore Finder metadata - #248
Conversation
|
🦞👀 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. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f8a7f5c2c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| provenLegacyProfileIds: new Set([profileId]), | ||
| preserveCanonicalMutableStateForLegacyProfileIds: | ||
| preserveCanonicalMutableState ? new Set([profileId]) : undefined, |
There was a problem hiding this comment.
Free reserved source handles when merging into an existing stub
When the destination already contains a canonical stub for user 4242 and another profile owns friend, importing a raw-proven legacy profile for 4242 with handle friend still fails with UNIQUE constraint failed: profiles.handle. Reconciliation first assigns the legacy row a birdclaw_stale_* handle, then this deferred merge calls mergeCurrentProfile, whose placeholder-target branch adopts that reserved source handle. Its call to allocateReservedProfileHandle at line 827 returns the same handle because it excludes the source row from collision checks, so the source never releases it before the target update. Exclude the source's current handle in the existing-target branch as well, as the new missing-target branch already does. This failure rolls back the entire import and blocks backup sync for this combination.
Useful? React with 👍 / 👎.
|
Codex review: blocked before merge. What this changesThis PR reconciles proven legacy profile identities during backup imports and excludes regular Finder metadata from backup validation and commits. Example: A backup contains profile_friend with numeric identity 4242, while the destination already has profile_user_4242 owning friend.
Review scores
ProductKind: Bug fix · Worth it: Yes · Fix scope: Complete Merge readiness⛔ Blocked before merge - 2 items remain Keep open: this maintainer-authored PR addresses a real backup failure, but the expanded merge path still aborts for an existing canonical stub. Priority: P2 Before merge
Findings
Tests
Agent review detailsHow this fits togetherBackup commands validate Git-backed JSONL, import portable rows into SQLite, reconcile profile identities and references, then export and publish the merged store. flowchart LR
A[Backup CLI] --> B[Git checkout and inventory validation]
B --> C[JSONL import transaction]
C --> D[Profile identity reconciliation]
D --> E[SQLite profiles and references]
E --> F[Validated export and Git publication]
Technical reviewBest possible solution: Use the existing reserved-handle allocator to free the source handle before either canonical-target branch adopts it. Do we have a high-confidence way to reproduce the issue? Main only defers profile_me reconciliation, leaving other legacy handle collisions actionable; the PR's additional existing-stub failure is traceable through reservation, allocation, and the unique constraint without executing target code. Is this the best way to solve the issue? Extending deferred reconciliation in the existing identity owner is appropriate, but handle transfer must free the reserved source handle in both target branches. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: not found in the target repository. Codex review notes: model internal, reasoning medium; reviewed against 40e2e23af737. Provenance checked
TestingProof path: in-process harness. SecurityNone. EvidenceAcceptance criteria:
What I checked:
Likely related people:
Review metrics
LabelsLabel changes:
Label justifications:
Rating scale6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior. WorkflowClawSweeper edits this one comment on every review. Comment Reviewed October 24, 2026, 6:18 PM ET / 22:18 UTC. |
Backup sync can stop indefinitely when an older backup still contains a legacy profile whose handle is now owned by its canonical numeric profile. Finder-created
data/.DS_Storefiles can independently block validation and sync.The profile import path previously reconciled only
profile_meand canonical numeric IDs, leaving other legacy rows to collide on the unique handle constraint. Reuse the deferred merge plans to reserve colliding handles and fold raw-proven numeric identities into canonical profiles after every reference has been imported. Unproven or conflicting identities remain separate, and identity vetoes survive repeated imports even when an intervening backup supplies no numeric identity.Evaluate each alias independently. Preserve current canonical state unless matching imported snapshots establish a newer state, including when its handle is reserved. Apply approved newer mutable state coherently, protect more recent handle owners, and keep the original observation times so historical imports cannot acquire artificial freshness or roll newer data back. Refresh derived identity data after recovered handles are assigned.
Ignore only regular files named exactly
.DS_Storeunderdata/during inventory validation and checkout-cleanliness checks. Exclude them explicitly from Git staging and remove previously tracked metadata from the next backup commit. Unexpected files, directories, and symlinks remain errors. Update the backup documentation and unreleased changelog.Validation:
UNIQUE constraint failed: profiles.handlefailure before the fix using synthetic profiles../scripts/bun-canary.sh ./scripts/run-vitest.mjs run src/lib/backup.test.ts src/lib/x-profile.test.ts— 2 files, 125 tests passed../scripts/bun-canary.sh ./scripts/run-vitest.mjs run src/lib— 115 files, 1,441 tests passed../scripts/bun-canary.sh run --bun check— formatting, lint, and type checks passed.No Playwright or end-to-end tests were run.