Approval writers can race and package admission can miss granted authority #520
Labels
No labels
code-review
correctness
dos
performance
security
severity/high
severity/low
severity/medium
tech-debt
Kind/Breaking
Kind/Bug
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Security
Kind/Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
h-dv/code-index#520
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Scope (consolidated 2026-11-12)
Enablement, updates and reads can reach independent approval writers with separate ordering rules. A read-only query must not become an activation writer. Eligibility also needs to reflect the exact generation’s stored claims rather than mutable process state. Admission is trustworthy only when conformance was graded under all the authority being granted.
Priority: Medium · Risk: data or trust · Size: L
Merged into this ticket: #563 (Conformance under granted authority)
Done
project_mutation_lockincrates/daemon/src/offer.rsand bundled adoption gives partial serialization, not a single-writer architecture.crates/indexer/src/conform.rs(1790–1843); a narrower failed verdict produces conformance_not_run and repair flags. Bridge authority is explicitly not_graded.Remaining
One activation writer
Generation claims
Authority-matched conformance
Acceptance
Suggested order
Original report
Summary
Approval and activation state (
code_index_indexer::approval) is written directly from three processes:cli/src/plugin.rs,activation.rs:833,plugin_gc.rs:69,plugin_doctor.rs:69,languages.rs:173,main.rs:782server.rsfrom:269, plusmain.rs:1277offer.rs,reactivate.rs,lifecycle.rs:939The activation logic is likewise spread over
indexer/activation.rs,indexer/activation_offer.rs,daemon/activation.rsandcli/activation.rs. Every one of these writers has to get locking, ordering and generation promotion right independently. #448 already reports "daemon approval concurrency" from the field.Proposal
Make the daemon the single writer of approval and activation state. The CLI and the MCP server send commands over RPC, and only when no daemon is running do they fall back to one shared lock-and-write function. After that, the activation code needs exactly one home.
Acceptance
approval::write functions are called only from the daemon crate and from the single fallback.plugin enablewhile the daemon auto-applies an update, and approval state stays consistent.Related: #448. Found by an automated architecture review (2026-11-09).
Filed from a five-agent review of master @
e3ec6ead(bloat, robustness, agent usefulness, docs, architecture). Every finding was spot-checked in the main session before filing.Approval/activation state is written from three processes — make the daemon the single writerto Activation state: daemon as single writer of approvals, and claims persisted per generation so every reader answers eligibility from the databaseField case for "daemon as single writer of approvals" (codex-531 dogfood D7, 2026-11-09). A read-only
code-index query --db <existing index>triggered automatic plugin enablement in the CLI process, loggedplugin_mutation_busy, and answeredsymbol_not_found. Several concurrent read queries contended as plugin writers. A read path should never become an approval writer. Details are on #559.Activation state: daemon as single writer of approvals, and claims persisted per generation so every reader answers eligibility from the databaseto Approval writers can race and package admission can miss granted authorityplugin_mutation_busyunder load: attributed and fixed on the train branch after v0.41.52 (2026-11-13)Who held the lock. Every writer of a project's approval state takes the same project lease (
project_mutation_lock), and every one of them made a singletry_lockwith no wait. In the reproduction (the phase-2plugin addpath, four copies in parallel, twelve busy loops) there were 1,683 refused attempts in eight runs, and the holder was the same each time: the daemon's own backgroundplugin updatechild, which holds the lease while it fetches and admits packages; observed holder ages 26.7–44.3 s. So a user command, or the adoption at the start of a second process, was refused by useful work of the product itself. The promise in the refusal holds: a refused operation is stopped before it loads approvals, so it changes nothing.Fix (merged into
integration-1114). One wait in one place (crates/indexer/src/approval/mutation.rs): a writer waits for the lease up to 120 s, polling; the kernel's release on process exit stays authoritative and no lease is ever stolen. It says once that it is waiting and for whom. A refusal that still happens names the operation, the waiting time and the holder (pid, operation, acquisition time, kept in a small file beside the lock because Windows cannot read a locked file through another handle). MCPplugin_addwaits on the blocking pool so the holder's elicitation answer can still be delivered. Same load and counts after the fix: 0 refused attempts.Not reproduced. The single failure of
a_withdrawn_or_explicitly_owned_language_is_not_adoptedand the exhausted 180 s deadline of the scheduled run did not recur in 24 and 8 runs under load. No second writer was observed in that single-project test, so its holder is unknown; the refusal now prints it if it happens again.What this adds to the remaining list. The background updater holds the project lease across network fetches. Fetching before taking the lease would shorten the hold from tens of seconds to the write itself; that belongs to "One activation writer". First-start language enablement and owner/config writes use their own files without this lease.
Data point from the released v0.41.52, 2026-11-13, on the code-index checkout itself (45 installed packages):
code-index link add <name> <path>wrote the link entry, then answered "the link was written but could not be approved (plugin_mutation_busy…)".code-index plugin update --root <project>, started about 20 s earlier. It held the project lock for at least 150 s.code-index approve link:<name>was refused 19 times over about 2.5 minutes before it succeeded. The refusal names no holder in this release.So in the release an operator command can fail for minutes because of a background update the operator did not start and cannot see. The next train has the waiting writer and the holder in the refusal (lane r31, 120 s limit); this case would still have run into that limit. The remaining part is the one already listed here: the updater must not hold the project lock while it fetches.
The background update no longer holds the project lease while it prepares (lane r36, merged locally as
ff377ee9on the next-train branch; not released yet).plugin updatetook the project lease and the store's automatic lock for the whole run. After: it reads a snapshot of the project (approval record, manifest bytes, effective languages, automatic policy), does catalog work, installs, verification and conformance without the project lease, then takes the lease only to publish. Under the lease it compares the snapshot with the current state; if approvals, links, enablement or the policy changed meanwhile, the candidate is deferred and the next run starts from the new state.approveorlink add.approvecompletes while an update is in its slow phase; an approval withdrawn during the download is respected (the candidate is deferred); an update with nothing to do on a store with 45 packages takes no project lease. The worker reported each mutation red; in my run the 22 tests of the update suite pass.Why the update took 150 s here. Not reproduced by the lane, because its fixtures are small. The updater's own log explains it: with nothing to update, one run installs and conformance-checks each of the eleven catalog packages again, twice (22 verdicts for 11 digests). That is filed as #586 and a lane is running for it. With this change that work no longer blocks project writers; #586 removes the work itself.
Remaining on this ticket after the two changes: the items of the body that are not about the update lock.
The background update now prepares without the project plugin lock and takes it only to publish. Plugin writers wait up to 120 s for the lock, and a refusal names the holder, operation and time held.