Skip to content

Commit f2f6684

Browse files
claude[bot]claude
andauthored
fix(runtime): key package lifecycle state by project, not by environment alone (#16572)
* test(runtime): drive the two-project package-state collision Pins the isolation the store does not yet have: two real project roots, one OS_HOME, one environment id. Red on this commit by design -- it records the leak (B's disable reaches A's boot read) and the clobber (B's enable erases A's disable) before the repair. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YFY46JydE1gMxQG1TqBcMZ * fix(runtime): key package lifecycle state by project, not by environment alone The disable list lived at `<OS_HOME>/package-state/<environmentId>.json`, and both halves of that path are machine-global -- `resolveObjectStackHome()` takes no arguments, and an environment id is not a project identity -- so two projects on one machine shared one file. Driven, not reasoned: project B's disable reached project A's boot read, and project B's enable erased project A's disable. The name now carries the project as well, using the same convention the runtime state file settled on: a sanitised basename plus a 12-hex digest of the resolved root, joined to the environment id with a dot. A pre-existing `<environmentId>.json` is still read while a project has no file of its own and is never deleted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YFY46JydE1gMxQG1TqBcMZ * style(runtime): keep the packages.ts path comment on one line The reflow shifted an elevation read site by one line and rotted a system-context census anchor; keeping the comment on one line leaves the census page untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YFY46JydE1gMxQG1TqBcMZ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3e560da commit f2f6684

6 files changed

Lines changed: 444 additions & 14 deletions

File tree

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
"@objectstack/runtime": minor
3+
---
4+
5+
Package lifecycle state is keyed by the PROJECT as well as the environment id, so two projects on one machine stop sharing which packages an operator has disabled.
6+
7+
<!-- adr-0087: not-required (no-migration-prescription) Nothing authorable moves: no spec key, export, config field or stored metadata changes spelling or shape, `packages/spec` is untouched, and `objectstack migrate meta` has nothing to rewrite. What changes is the NAME of an operational state file the runtime writes under the ObjectStack home — a path on disk, not a metadata surface the ledger can project into `spec-changes.json` or the generated upgrade guide. The runtime reads the old name itself while no per-project file exists, so no consumer rewrites anything and there is no authored artifact a metadata upgrader could reach. -->
8+
9+
**BREAKING** for a machine that relied on one environment id meaning one shared disable list. Shipped as `minor` under the launch-window convention: while the whole workspace versions in lockstep the bump level carries no breaking-ness, so this banner and the ADR-0087 disposition above are the carriers.
10+
11+
`packages/runtime/src/package-state-store.ts` is the only durable record of which packages an operator has disabled, and `AppPlugin.start()` replays it at boot. It was stored at `<OS_HOME>/package-state/<environmentId>.json`, and both halves of where that lived were machine-global: `resolveObjectStackHome()` takes no arguments (it reads `OS_HOME`, else `~/.objectstack`), and an environment id is not a project identity. Two different projects on one machine, both in the ordinary `env_local` environment, therefore wrote one file.
12+
13+
Driven with two real project roots, one home and one environment id, that produced two failures with one cause:
14+
15+
- project B disabling `com.acme.billing` made project A's **boot read** answer `{ com.acme.billing, com.acme.reporting }` — A had never installed, seen or disabled that package, and the disable takes it out of A's running system;
16+
- project B enabling `com.acme.reporting` erased project A's disable of it, so one project's operator action silently undid another project's operator intent.
17+
18+
The file is now `<OS_HOME>/package-state/<environmentId>.<project>.json`, where the project component is a sanitised basename plus a short digest of the resolved project root — the same naming convention `os serve`'s runtime state file settled on, rather than a second spelling of one idea. The payload is unchanged.
19+
20+
**An existing `<environmentId>.json` keeps working and is not deleted.** While a project has no per-project file of its own the runtime still reads the old name, and that project's first write lands under the new one. The old file is never written and never removed, so a machine that rolls back to the previous release still finds its operator's disables where that release looks for them. Disables made after the upgrade live under the new name only.
21+
22+
**Which project the key is taken from:** the runtime's working directory, the base every path in a boot with no served-app anchor already resolves against. Two boundaries follow, stated rather than fixed. `os serve` anchors host resolution at the config file's own directory when that directory carries a `package.json`, so serving a config from elsewhere keys this file by the working directory while the CLI's supervision file keys by the config's directory; and the key is the resolved path rather than the realpath, so two symlinked spellings of one project key two files, each internally consistent. Two boots of the same project from the same directory still share one file, which is the same-project case and unchanged here.

packages/runtime/src/domains/packages-readonly-gate.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* One API call took platform functionality out of a live deployment. `DELETE`
1515
* came back on restart (the packages are code-loaded, so nothing is permanently
1616
* destroyed); `disable` did NOT — `setPackageDisabled` persists the choice to
17-
* `<OS_HOME>/package-state/<env>.json` and the registry replays it at boot.
17+
* `<OS_HOME>/package-state/<env>.<project>.json` and the registry replays it at boot.
1818
*
1919
* ## Two axes, not one
2020
*

packages/runtime/src/domains/packages.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ function requireReadCapability(deps: DomainHandlerDeps, context: HttpProtocolCon
303303
* running registry listing. One API call took platform functionality out of a
304304
* live deployment; `DELETE` came back on restart (the packages are code-loaded),
305305
* `disable` did NOT — {@link setPackageDisabled} persists the disable to
306-
* `<OS_HOME>/package-state/<env>.json`, which the registry re-reads at boot, so
306+
* `<OS_HOME>/package-state/<env>.<project>.json`, which the registry re-reads at boot, so
307307
* a disabled platform package stays disabled across restarts.
308308
*
309309
* The predicate is {@link isWritablePackage} from `@objectstack/metadata-protocol`
Lines changed: 303 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,303 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #15969 — package lifecycle state is keyed by the PROJECT as well as the
5+
* environment id, so two projects on one machine stop sharing one disable list.
6+
*
7+
* ## The collision, DRIVEN before it was repaired
8+
*
9+
* `package-state-store` is the only durable record of which packages an
10+
* operator has disabled, and `AppPlugin.start()` replays it at boot. Both
11+
* halves of where that record lived were machine-global:
12+
* {@link resolveObjectStackHome} takes NO arguments — `OS_HOME`, else
13+
* `~/.objectstack` — and an environment id is not a project identity. So two
14+
* different projects on one machine, both in the ordinary `env_local`
15+
* environment, addressed ONE file:
16+
*
17+
* ```text
18+
* <OS_HOME>/package-state/env_local.json ← written by BOTH projects
19+
* ```
20+
*
21+
* Driven here rather than reasoned, with two real project directories, one
22+
* home and one environment id. Two failures came out of that one file:
23+
*
24+
* ```text
25+
* LEAK project B disables com.acme.billing
26+
* → project A's BOOT READ now returns com.acme.billing as disabled,
27+
* and A never installed, saw or disabled that package.
28+
* CLOBBER project A disables com.acme.reporting; project B enables it
29+
* → A's disable is gone. A's operator intent was erased by an
30+
* operator action taken in a different project.
31+
* ```
32+
*
33+
* ⛔ The second is the literal "second write clobbers the first". The first is
34+
* the one that makes this a shared BEHAVIOUR rather than a shared report:
35+
* `loadDisabledPackageIds` is read at boot (`app-plugin.ts`), so a disable in
36+
* project A takes a package out of project B's running system.
37+
*
38+
* ## Why the CWD, and why real directories
39+
*
40+
* The project identity this store can see is the process's working directory —
41+
* the same base every other path in a boot with no served-app anchor resolves
42+
* against. The two projects below are therefore two real directories the test
43+
* process actually stands in, one at a time: a `cwd` that is merely *named*
44+
* would prove nothing about a store that reads `process.cwd()` itself.
45+
*
46+
* ⛔ No boot, no server, no plugin chain. What is under test is which FILE a
47+
* disable lands in; a boot would add minutes and a plugin graph to a question
48+
* that has neither.
49+
*/
50+
51+
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
52+
import { createHash } from 'node:crypto';
53+
import {
54+
existsSync,
55+
mkdirSync,
56+
mkdtempSync,
57+
readFileSync,
58+
readdirSync,
59+
realpathSync,
60+
rmSync,
61+
writeFileSync,
62+
} from 'node:fs';
63+
import { tmpdir } from 'node:os';
64+
import { basename, join } from 'node:path';
65+
66+
import { loadDisabledPackageIds, packageStateFileName, setPackageDisabled } from './package-state-store.js';
67+
68+
const ENVIRONMENT_ID = 'env_local';
69+
70+
/** The ONE machine-global home both projects resolve to. */
71+
let home: string;
72+
/** Two projects on that one machine. Real directories — the test stands in them. */
73+
let projectA: string;
74+
let projectB: string;
75+
let sandbox: string;
76+
77+
const originalCwd = process.cwd();
78+
const envSnapshot = { OS_HOME: process.env.OS_HOME, OS_ENVIRONMENT_ID: process.env.OS_ENVIRONMENT_ID };
79+
80+
/** Counters, printed by the assertions below: a drive that never ran is vacuous. */
81+
let chdirsPerformed = 0;
82+
83+
/** Do one project's work while the process really stands in that project. */
84+
function inProject<T>(root: string, work: () => T): T {
85+
process.chdir(root);
86+
chdirsPerformed += 1;
87+
try {
88+
return work();
89+
} finally {
90+
process.chdir(originalCwd);
91+
}
92+
}
93+
94+
/** Every state file the shared home currently holds. */
95+
function stateFiles(): string[] {
96+
const dir = join(home, 'package-state');
97+
return existsSync(dir) ? readdirSync(dir).sort() : [];
98+
}
99+
100+
beforeEach(() => {
101+
sandbox = realpathSync(mkdtempSync(join(tmpdir(), 'os-15969-')));
102+
home = join(sandbox, 'home');
103+
projectA = join(sandbox, 'alpha');
104+
projectB = join(sandbox, 'beta');
105+
mkdirSync(home, { recursive: true });
106+
mkdirSync(projectA, { recursive: true });
107+
mkdirSync(projectB, { recursive: true });
108+
process.env.OS_HOME = home;
109+
delete process.env.OS_ENVIRONMENT_ID;
110+
});
111+
112+
afterEach(() => {
113+
process.chdir(originalCwd);
114+
rmSync(sandbox, { recursive: true, force: true });
115+
if (envSnapshot.OS_HOME === undefined) delete process.env.OS_HOME;
116+
else process.env.OS_HOME = envSnapshot.OS_HOME;
117+
if (envSnapshot.OS_ENVIRONMENT_ID === undefined) delete process.env.OS_ENVIRONMENT_ID;
118+
else process.env.OS_ENVIRONMENT_ID = envSnapshot.OS_ENVIRONMENT_ID;
119+
});
120+
121+
describe('package state is keyed per project (#15969)', () => {
122+
// The control for every assertion below: the two projects really are two
123+
// different working directories, and the process really moves between them.
124+
it('stands in two distinct project roots', () => {
125+
const seen = [inProject(projectA, () => process.cwd()), inProject(projectB, () => process.cwd())];
126+
expect(seen[0]).toBe(projectA);
127+
expect(seen[1]).toBe(projectB);
128+
expect(seen[0]).not.toBe(seen[1]);
129+
expect(chdirsPerformed).toBeGreaterThanOrEqual(2);
130+
});
131+
132+
// LEAK — the half that makes this a shared BEHAVIOUR: `loadDisabledPackageIds`
133+
// is the boot read, so a package B disabled must never come back disabled in A.
134+
it('does not leak project B\'s disable into project A\'s boot read', () => {
135+
inProject(projectA, () => setPackageDisabled(ENVIRONMENT_ID, 'com.acme.reporting', true));
136+
inProject(projectB, () => setPackageDisabled(ENVIRONMENT_ID, 'com.acme.billing', true));
137+
138+
expect(inProject(projectA, () => loadDisabledPackageIds(ENVIRONMENT_ID)))
139+
.toEqual(new Set(['com.acme.reporting']));
140+
expect(inProject(projectB, () => loadDisabledPackageIds(ENVIRONMENT_ID)))
141+
.toEqual(new Set(['com.acme.billing']));
142+
});
143+
144+
// CLOBBER — the literal "the second write must not clobber the first".
145+
it('does not let project B\'s enable erase project A\'s disable', () => {
146+
inProject(projectA, () => setPackageDisabled(ENVIRONMENT_ID, 'com.acme.reporting', true));
147+
inProject(projectB, () => setPackageDisabled(ENVIRONMENT_ID, 'com.acme.reporting', false));
148+
149+
expect(inProject(projectA, () => loadDisabledPackageIds(ENVIRONMENT_ID)))
150+
.toEqual(new Set(['com.acme.reporting']));
151+
expect(inProject(projectB, () => loadDisabledPackageIds(ENVIRONMENT_ID)))
152+
.toEqual(new Set());
153+
});
154+
155+
// The cause, stated as a file fact: one shared name is what produced both
156+
// failures above, so the repair has to be visible in the home directory.
157+
it('writes two files, one per project, for one environment id', () => {
158+
inProject(projectA, () => setPackageDisabled(ENVIRONMENT_ID, 'com.acme.reporting', true));
159+
inProject(projectB, () => setPackageDisabled(ENVIRONMENT_ID, 'com.acme.billing', true));
160+
161+
const files = stateFiles();
162+
expect(files).toHaveLength(2);
163+
expect(files).not.toContain(`${ENVIRONMENT_ID}.json`);
164+
expect(new Set(files).size).toBe(2);
165+
});
166+
167+
// The environment id keeps doing its own job: same project, two environments,
168+
// still two files. Neither identity is sufficient alone.
169+
it('still separates two environments within one project', () => {
170+
inProject(projectA, () => {
171+
setPackageDisabled('env_staging', 'com.acme.reporting', true);
172+
expect(loadDisabledPackageIds('env_staging')).toEqual(new Set(['com.acme.reporting']));
173+
expect(loadDisabledPackageIds('env_production')).toEqual(new Set());
174+
});
175+
});
176+
});
177+
178+
/**
179+
* The naming convention, recomputed here rather than asked of the store.
180+
*
181+
* ⛔ This is the point of the block: `#15733` / PR #15968 settled a spelling for
182+
* "one project root, folded into a filename component" — a sanitised basename,
183+
* a `-`, and 12 hex of the sha256 of the RESOLVED root — and joined it to the
184+
* environment id with a `.`. Asking `packageStateFileName` what it produces
185+
* would pin nothing; an independent second computation is what makes a drift
186+
* away from that convention go red.
187+
*/
188+
function expectedProjectKey(root: string): string {
189+
const digest = createHash('sha256').update(root).digest('hex').slice(0, 12);
190+
const slug = basename(root).toLowerCase().replace(/[^a-z0-9]+/g, '-').replace(/^-+|-+$/g, '').slice(0, 24);
191+
return slug.length > 0 ? `${slug}-${digest}` : digest;
192+
}
193+
194+
/** The name every project shared before this card. */
195+
function legacyFile(environmentId: string): string {
196+
return join(home, 'package-state', `${environmentId}.json`);
197+
}
198+
199+
function writeLegacy(environmentId: string, disabled: string[]): string {
200+
mkdirSync(join(home, 'package-state'), { recursive: true });
201+
const file = legacyFile(environmentId);
202+
writeFileSync(file, `${JSON.stringify({ disabled }, null, 2)}\n`, 'utf8');
203+
return file;
204+
}
205+
206+
describe('the file name follows PR #15968\'s convention (#15969)', () => {
207+
it('is <environment>.<slug>-<digest>.json', () => {
208+
const name = packageStateFileName(ENVIRONMENT_ID, projectA);
209+
210+
expect(name).toBe(`${ENVIRONMENT_ID}.${expectedProjectKey(projectA)}.json`);
211+
// The shape, spelled out: a `.` between the two identities, and the
212+
// project half is a slug, a `-`, and 12 lowercase hex.
213+
expect(name).toMatch(/^env_local\.[a-z0-9-]*[a-z0-9]-[0-9a-f]{12}\.json$/);
214+
expect(name.startsWith(`${ENVIRONMENT_ID}.`)).toBe(true);
215+
});
216+
217+
it('separates two projects and two environments independently', () => {
218+
expect(packageStateFileName(ENVIRONMENT_ID, projectA))
219+
.not.toBe(packageStateFileName(ENVIRONMENT_ID, projectB));
220+
expect(packageStateFileName(ENVIRONMENT_ID, projectA))
221+
.not.toBe(packageStateFileName('env_staging', projectA));
222+
});
223+
224+
it('resolves the root, so two spellings of one project key one file', () => {
225+
expect(packageStateFileName(ENVIRONMENT_ID, `${projectA}/`))
226+
.toBe(packageStateFileName(ENVIRONMENT_ID, projectA));
227+
expect(packageStateFileName(ENVIRONMENT_ID, join(projectB, '..', 'alpha')))
228+
.toBe(packageStateFileName(ENVIRONMENT_ID, projectA));
229+
});
230+
231+
it('still keys a root whose basename sanitises away to nothing', () => {
232+
const odd = join(sandbox, '+++');
233+
mkdirSync(odd, { recursive: true });
234+
const name = packageStateFileName(ENVIRONMENT_ID, odd);
235+
236+
expect(name).toMatch(/^env_local\.[0-9a-f]{12}\.json$/);
237+
expect(name).not.toBe(packageStateFileName(ENVIRONMENT_ID, projectA));
238+
});
239+
240+
it('keeps the environment id sanitised, so it cannot escape the directory', () => {
241+
expect(packageStateFileName('../../etc/evil', projectA))
242+
.toBe(`.._.._etc_evil.${expectedProjectKey(projectA)}.json`);
243+
});
244+
245+
it('names the file the store actually writes', () => {
246+
inProject(projectA, () => setPackageDisabled(ENVIRONMENT_ID, 'com.acme.reporting', true));
247+
248+
expect(stateFiles()).toEqual([packageStateFileName(ENVIRONMENT_ID, projectA)]);
249+
});
250+
});
251+
252+
describe('legacy <environment>.json migration is READ-ONCE (#15969)', () => {
253+
it('reads the legacy file while this project has no file of its own', () => {
254+
writeLegacy(ENVIRONMENT_ID, ['com.acme.reporting']);
255+
256+
expect(inProject(projectA, () => loadDisabledPackageIds(ENVIRONMENT_ID)))
257+
.toEqual(new Set(['com.acme.reporting']));
258+
});
259+
260+
it('writes the new key and LEAVES THE LEGACY FILE IN PLACE', () => {
261+
const legacy = writeLegacy(ENVIRONMENT_ID, ['com.acme.reporting']);
262+
const before = readFileSync(legacy, 'utf8');
263+
264+
inProject(projectA, () => setPackageDisabled(ENVIRONMENT_ID, 'com.acme.billing', true));
265+
266+
// The legacy content came across, under the new name...
267+
const own = join(home, 'package-state', packageStateFileName(ENVIRONMENT_ID, projectA));
268+
expect(JSON.parse(readFileSync(own, 'utf8'))).toEqual({
269+
disabled: ['com.acme.billing', 'com.acme.reporting'],
270+
});
271+
// ...and the legacy file is untouched, byte for byte. ⛔ A release that
272+
// deletes it strands an operator who rolls back.
273+
expect(existsSync(legacy)).toBe(true);
274+
expect(readFileSync(legacy, 'utf8')).toBe(before);
275+
});
276+
277+
it('stops consulting the legacy file once this project has written one', () => {
278+
const legacy = writeLegacy(ENVIRONMENT_ID, ['com.acme.reporting']);
279+
280+
inProject(projectA, () => setPackageDisabled(ENVIRONMENT_ID, 'com.acme.reporting', false));
281+
// The legacy file still says disabled; this project's own file does not.
282+
expect(JSON.parse(readFileSync(legacy, 'utf8'))).toEqual({ disabled: ['com.acme.reporting'] });
283+
expect(inProject(projectA, () => loadDisabledPackageIds(ENVIRONMENT_ID))).toEqual(new Set());
284+
});
285+
286+
it('does not let one project\'s migration clobber another\'s legacy state', () => {
287+
writeLegacy(ENVIRONMENT_ID, ['com.acme.reporting']);
288+
289+
inProject(projectA, () => setPackageDisabled(ENVIRONMENT_ID, 'com.acme.reporting', false));
290+
291+
// B has not migrated yet, so B still reads the legacy record — unchanged.
292+
expect(inProject(projectB, () => loadDisabledPackageIds(ENVIRONMENT_ID)))
293+
.toEqual(new Set(['com.acme.reporting']));
294+
});
295+
296+
it('is per environment: a legacy file for another environment is not read', () => {
297+
writeLegacy('env_staging', ['com.acme.reporting']);
298+
299+
expect(inProject(projectA, () => loadDisabledPackageIds(ENVIRONMENT_ID))).toEqual(new Set());
300+
expect(inProject(projectA, () => loadDisabledPackageIds('env_staging')))
301+
.toEqual(new Set(['com.acme.reporting']));
302+
});
303+
});

packages/runtime/src/package-state-store.test.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,21 @@ import { mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'nod
1818
import { tmpdir } from 'node:os';
1919
import { join } from 'node:path';
2020

21-
import { loadDisabledPackageIds, setPackageDisabled } from './package-state-store.js';
21+
import { loadDisabledPackageIds, packageStateFileName, setPackageDisabled } from './package-state-store.js';
2222

2323
let home: string;
2424
const envSnapshot = { OS_HOME: process.env.OS_HOME, OS_ENVIRONMENT_ID: process.env.OS_ENVIRONMENT_ID };
2525

26-
/** Absolute path of the state file the store is expected to use. */
26+
/**
27+
* Absolute path of the state file the store is expected to use.
28+
*
29+
* The name carries the PROJECT as well as the environment id (#15969), and the
30+
* project this suite runs as is its own working directory — so it is asked for
31+
* by the same function the store names files with, rather than spelled out.
32+
* The naming convention itself is pinned in `package-state-project-key.test.ts`.
33+
*/
2734
function stateFile(environmentId: string): string {
28-
return join(home, 'package-state', `${environmentId}.json`);
35+
return join(home, 'package-state', packageStateFileName(environmentId, process.cwd()));
2936
}
3037

3138
beforeEach(() => {

0 commit comments

Comments
 (0)