Skip to content

Commit e86cf0d

Browse files
committed
fix(runtime): DELETE /packages/:id asks deletePackage before it withdraws the package or clears its disable record
The door ran registry.uninstallPackage and setPackageDisabled(..., false) before protocol.deletePackage, so a store that refused the sys_packages delete got a 500 while the running process had already dropped the package and its durable disable record. Existence is now read with getPackage, and the withdrawal and the disable clear follow a deletePackage that answered. Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 7d29e5c commit e86cf0d

1 file changed

Lines changed: 84 additions & 26 deletions

File tree

‎packages/runtime/src/domains/packages.ts‎

Lines changed: 84 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -450,6 +450,11 @@ function requireWritablePackage(
450450
* asks, so a mirror that exempted anyone would disagree with it. Returns a
451451
* refusal result to short-circuit on, or `null` to proceed. Callers MUST run
452452
* it before `uninstallPackage`, and only when `deletePackage` will run.
453+
*
454+
* [#21276] The ordering above is the one this refusal was written against.
455+
* Since then the door withdraws nothing before `deletePackage` answers: the
456+
* registry withdrawal and the disable-record clear both follow it, so a
457+
* refusal from the store leaves the running process untouched as well.
453458
*/
454459
function requireUninstallOrganizationScope(
455460
deps: DomainHandlerDeps,
@@ -2031,35 +2036,25 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin
20312036
// The protocol keeps its own refusal as the second line.
20322037
const protocol = await resolveProtocol(deps, _context);
20332038
const organizationId = await deps.resolveActiveOrganizationId(_context);
2034-
if (protocol && typeof protocol.deletePackage === 'function') {
2039+
const persists = Boolean(protocol && typeof protocol.deletePackage === 'function');
2040+
if (persists) {
20352041
const unscoped = requireUninstallOrganizationScope(deps, id, organizationId); if (unscoped) return unscoped;
20362042
}
2037-
const registryRemoved = registry.uninstallPackage(id);
20382043

2039-
// ⭐ [#18877 ruling item 3] A package that no longer exists has no
2040-
// lifecycle state — so the DURABLE disable record goes with the row,
2041-
// and the next install of this id is a FRESH install that lands at
2042-
// the declared default. The registry half of the same sentence is
2043-
// inside `uninstallPackage`, which forgets the id from the boot seed
2044-
// set; this is the half that outlives the process.
2045-
//
2046-
// Without it the record was immortal: `DELETE` removed the row and
2047-
// left the id listed on disk, the next boot seeded it back, and a
2048-
// reinstalled package came up disabled with nothing anywhere saying
2049-
// why — a disable the operator could no longer even see to undo,
2050-
// since the package it named was gone. Written only when the
2051-
// registry really removed the row, so a 404 changes no state.
2044+
// [#21276] Existence is READ here, never acted on. This line used to
2045+
// be `registry.uninstallPackage(id)`, and the disable-record clear
2046+
// below sat right after it — both BEFORE `deletePackage`. So when the
2047+
// store then refused the `sys_packages` delete, the door answered the
2048+
// failure while the running process had already dropped the package
2049+
// (`GET` 404 until a restart brought it back) and the disable record
2050+
// was already gone (a disabled package came back enabled). Measured
2051+
// at this door on SQLite with a trigger refusing the delete.
20522052
//
2053-
// Same best-effort try/catch as the install and PATCH arms above:
2054-
// the uninstall itself already happened, so a state-file failure
2055-
// must not turn it into a 500.
2056-
if (registryRemoved) {
2057-
try {
2058-
setPackageDisabled(_context?.environmentId, id, false);
2059-
} catch (err) {
2060-
console.warn('[handlePackages] failed to clear persisted disable state on delete', { id, error: (err as Error)?.message });
2061-
}
2062-
}
2053+
// Triage's ruling for #21276 — refuse before withdrawing, not undo:
2054+
// `deletePackage` deletes the stored row FIRST and refuses before it
2055+
// removes anything else, so the door asks it first and touches the
2056+
// running registry and the disable record only once it has answered.
2057+
const existed = registry.getPackage(id) !== undefined;
20632058

20642059
// Persisted removal (AI/runtime packages live in sys_metadata, not
20652060
// just the in-memory registry — the registry uninstall alone would
@@ -2102,10 +2097,70 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin
21022097
...(keepData ? { keepData: true } : {}),
21032098
});
21042099
} catch (e: any) {
2100+
// [#21276] Nothing was touched on this path: the registry
2101+
// still holds the package and its disable record is intact.
21052102
return { handled: true, response: deps.errorFromThrown(e, 500) };
21062103
}
21072104
}
21082105

2106+
// [#21276] Only now — the persisted delete has answered, or this host
2107+
// has none — is the package withdrawn from the running registry.
2108+
// `deletePackage` withdraws it itself on the same registry (its own
2109+
// step after the stored rows), so this is usually a no-op and is
2110+
// skipped when the package is already gone. A host with no
2111+
// persisted half keeps exactly its old behaviour: the withdrawal is
2112+
// the uninstall, and its refusal (another package extends an object
2113+
// this one owns, ADR-0029) is the request's refusal.
2114+
//
2115+
// With a persisted half, that refusal now arrives AFTER the stored
2116+
// rows were deleted — the registry has no verb that answers it
2117+
// without performing it. It is reported as `registryRemoved: false`
2118+
// on the answer rather than as a failure the store does not bear
2119+
// out: the package leaves the running process at the next restart.
2120+
if (registry.getPackage(id) !== undefined) {
2121+
if (persists) {
2122+
try {
2123+
registry.uninstallPackage(id);
2124+
} catch (err) {
2125+
console.warn(
2126+
`[handlePackages] '${id}' was deleted from storage but the running registry refused to `
2127+
+ `withdraw it, so this process keeps serving it until it restarts: ${(err as Error)?.message}`,
2128+
);
2129+
}
2130+
} else {
2131+
registry.uninstallPackage(id);
2132+
}
2133+
}
2134+
const registryRemoved = existed && registry.getPackage(id) === undefined;
2135+
2136+
// ⭐ [#18877 ruling item 3] A package that no longer exists has no
2137+
// lifecycle state — so the DURABLE disable record goes with the row,
2138+
// and the next install of this id is a FRESH install that lands at
2139+
// the declared default. The registry half of the same sentence is
2140+
// inside `uninstallPackage`, which forgets the id from the boot seed
2141+
// set; this is the half that outlives the process.
2142+
//
2143+
// Without it the record was immortal: `DELETE` removed the row and
2144+
// left the id listed on disk, the next boot seeded it back, and a
2145+
// reinstalled package came up disabled with nothing anywhere saying
2146+
// why — a disable the operator could no longer even see to undo,
2147+
// since the package it named was gone. Written only for a package
2148+
// this request found, so a 404 changes no state; and [#21276] only
2149+
// once the persisted delete has answered — with a persisted half,
2150+
// whenever its stored row is gone, even if the running registry
2151+
// refused the withdrawal above.
2152+
//
2153+
// Same best-effort try/catch as the install and PATCH arms above:
2154+
// the uninstall itself already happened, so a state-file failure
2155+
// must not turn it into a 500.
2156+
if (persists ? existed : registryRemoved) {
2157+
try {
2158+
setPackageDisabled(_context?.environmentId, id, false);
2159+
} catch (err) {
2160+
console.warn('[handlePackages] failed to clear persisted disable state on delete', { id, error: (err as Error)?.message });
2161+
}
2162+
}
2163+
21092164
const deletedCount = persisted?.deletedCount ?? 0;
21102165
const failedCount = persisted?.failedCount ?? 0;
21112166

@@ -2149,7 +2204,10 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin
21492204
),
21502205
};
21512206
}
2152-
if (!registryRemoved && deletedCount === 0) {
2207+
// [#21276] `existed`, not `registryRemoved`: a package this request
2208+
// found whose withdrawal the registry refused after its stored row
2209+
// was deleted is not "not found".
2210+
if (!existed && deletedCount === 0) {
21532211
return { handled: true, response: deps.error(`Package '${id}' not found`, 404) };
21542212
}
21552213
// [#16781] `packageId` is REQUIRED by

0 commit comments

Comments
 (0)