Uninstall manifest can lose recorded paths when two processes call recordOwnedConfigPath

Open
  • Is something wrong?
#8,093 1 comment 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
30/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
sqlite, typescript
Domain
tooling

Research direction

Start in src/lib/config-ownership.ts with recordOwnedConfigPath, which appends to a process-cached manifest and rewrites it, and compare it with removeOwnedConfigState, which reloads from disk. Then look at the config mutation lock and INITIAL_OWNED_PATHS to see how cross-process serialization is already done. Done means a test where two cached instances each record a different path and both paths survive in the on-disk manifest for uninstall.

Written by the indexing model from the issue text.

Description

bug install
Client or integration

Other

Area

Installation or packaging

Summary

recordOwnedConfigPath in src/lib/config-ownership.ts appends a path to the uninstall manifest from a process-local cached copy of the manifest and then rewrites the whole file. Two processes that share one config home can each hold an older cached manifest, so the later writer replaces the file without the path the earlier writer just recorded, while both calls return true. removeOwnedConfigState reloads the manifest from disk, so the dropped path is not removed by uninstall.

I expected a recorded path to stay in the on-disk manifest until uninstall, whichever process recorded it.

Today this affects every file registered after home creation, including spend-ledger-owner.sqlite (prepareOwnerPath), config-mutation.sqlite for homes created before it joined INITIAL_OWNED_PATHS, and every default atomicWriteFileNoFollow write. Paths listed in INITIAL_OWNED_PATHS are unaffected for homes created after they were added.

Suggested fix: serialize manifest updates across processes (for example under the config mutation lock) and reload the manifest from disk inside that critical section before appending.

Reproduction
  1. Start from an owned config home whose manifest lacks paths a and b.
  2. Process 1 and process 2 both load the manifest (it is now cached in each).
  3. Process 1 calls recordOwnedConfigPath(home, a); the manifest on disk lists a.
  4. Process 2 calls recordOwnedConfigPath(home, b); it writes its cached list plus b, so a is gone.
  5. removeOwnedConfigState(home) leaves a behind.
Version

2.97.0 (dev, commit 6f1d3e65c)

Operating system

All platforms (logic is platform independent).

Provider and model

No response

Logs or error output
No error: both recordOwnedConfigPath calls return true.
Screenshots and supporting files

No response

Redacted configuration
{}
Checks
  • I searched existing issues and documentation.
  • I removed secrets, tokens, account details, request credentials, and personal data.
Dominant language
TypeScript
Stars
16.9k
Forks
1.3k
Avg merge
16h 51m
Merged PRs (30d)
681

Getting set up

This project ships no dev container, Dockerfile or contributing guide, so setting up is up to you: start from its README, and see our first-contribution guide for the general steps.

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from lidge-jun/opencodex

All issues in lidge-jun/opencodex

Similar issues

More TypeScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.