Skip to content

fix(hooks): harden Pi/OMP shared-extension ownership lifecycle - #3875

Open
KuSh wants to merge 1 commit into
rtk-ai:developfrom
KuSh:fix/pi-omp-ownership-lifecycle
Open

KuSh wants to merge 1 commit into
rtk-ai:developfrom
KuSh:fix/pi-omp-ownership-lifecycle

Conversation

@KuSh

@KuSh KuSh commented Sep 5, 2026 •

Copy link
Copy Markdown
Collaborator

Follow-up to #3707, split out so it could merge without waiting on this.

#3707 introduced the .rtk-agents ownership sidecar to stop one agent's uninstall from deleting an extension the other agent shares. The mechanism is right, but it could not engage in the case it exists for, and several destructive paths had no way back. None of this is a regression against develop — OMP support did not exist there before #3707 — which is why it was kept out of that PR.

Ownership

The sidecar was never created for an extension that pre-dated it, so shared-file protection was inert for every upgrading user: install OMP over an existing Pi extension, uninstall OMP, and Pi's extension is deleted with no warning and exit 0.

The underlying problem was that record_managed_agent had two outcomes for three facts — "nobody else owns this", "someone else does", and "someone installed this and I cannot know who". The third now has a representation (prior_unknown), so a record can be created for a pre-existing extension while staying explicitly partial.

Also in this area:

  • Entries written by a newer RTK are preserved across rewrites, and recorded owners survive an entry this version cannot parse.
  • Records are merged, never reset: deleting the file does not unconfigure an agent that still resolves to that path.
  • The record is re-read immediately before writing, since a confirmation prompt sits between the caller's read and the write.
  • On uninstall, only the departing agent is dropped when the extension survives — removing a symlink leaves the target and its other owner.

Recovery

  • Uninstall gets the same Ask/Auto/Skip policy as install, so a hand-edited or fork-built extension stays removable with --auto-patch instead of being permanently stuck behind "remove the file manually".
  • Non-stock content is copied aside before being replaced or removed, never overwriting an earlier backup.
  • --dry-run describes what the real run does, including the backup.

The copy callers go through one adapter over the shared free_backup_slot, so the probe, the ceiling and the notion of "already preserved" exist once. The uninstall prompt matches the slot directly, because it is the only caller whose wording depends on why no copy will be made: content already preserved reads differently from a source RTK cannot read.

Symlinks

  • Aliases are detected at any path component, not just the last one.
  • A dangling link is written through to its target rather than replaced, so an alias set up before the target exists survives.
  • A link RTK cannot resolve is routed through the same confirmation as any other content the user put there.

Tests

test_global_uninstall_detects_shared_pi_omp_extension no longer creates a project-local OMP directory: global-scope share detection never reads one, and its relative path put an empty .omp/ in the checkout on every run. Its assertion now names what it checks. The test job's fetch-depth: 0 in ci.yml gets a comment naming the Pi extension history guard that depends on it.

Verification

cargo fmt --all --check, cargo clippy --all-targets and cargo test --all clean: 3974 unit tests and 28 OMP/Pi integration tests, 33 test binaries in all. Also type-checked for x86_64-pc-windows-gnu with --all-targets, since the Unix-gated tests are invisible to a Linux run.

🤖 Generated with Claude Code

Follow-up to rtk-ai#3707. The ownership sidecar could not protect the case it
exists for, and several destructive paths had no way back.

Ownership:
- Record ownership for an extension that pre-dates the sidecar, marking
  the record partial instead of claiming sole ownership. Without this the
  sidecar is never created for an upgrading user, so shared-file
  protection is inert for everyone who already had the extension.
- Preserve entries written by a newer RTK across rewrites, and keep the
  recorded owners when one entry cannot be parsed.
- Merge into an existing record rather than resetting it: deleting the
  file does not unconfigure an agent that still resolves to that path.
- Re-read the record immediately before writing, since a confirmation
  prompt sits between the caller's read and the write.
- On uninstall, drop only the departing agent when the extension itself
  survives (removing a symlink leaves the target and its other owner).

Recovery:
- Give uninstall the same Ask/Auto/Skip policy as install, so a
  hand-edited or fork-built extension stays removable with --auto-patch
  instead of being permanently stuck.
- Copy non-stock content aside before replacing or removing it, never
  overwriting an earlier backup.
- Make --dry-run describe what the real run does, including the backup.

Symlinks:
- Detect an alias at any path component, not just the last one.
- Write through a dangling link to its target instead of replacing the
  link.
- Route an unresolvable link through the same confirmation as any other
  content the user put there.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@KuSh
KuSh force-pushed the fix/pi-omp-ownership-lifecycle branch from 2dfbe3f to 58b5c56 Compare October 7, 2026 16:34
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.

1 participant