diff --git a/server/lib/README.md b/server/lib/README.md index cd8284e1e3..692009dd5a 100644 --- a/server/lib/README.md +++ b/server/lib/README.md @@ -218,7 +218,7 @@ The barrel `server/lib/index.js` is a machine-checkable enumeration of every pub | `collectionStore.js` | Per-type, per-record JSON storage with explicit type-level `schemaVersion` stamping. Use for collections that have outgrown a monolithic JSON file. `createCollectionStore({ dir, type, schemaVersion, sanitizeRecord })` returns `loadOne` / `saveOne` / `saveOneNow` / `listIds` / `loadAll` / `loadAllResult` / `deleteOne` / `loadTypeIndex` / `saveTypeIndex` / `verifySchemaVersion`. `loadAll` silently drops corrupt records; `loadAllResult()` → `{ records, failedIds }` keeps the "which ids failed to load" signal so a caller can tell a partial set from a complete one. Per-id write queue means writes to different records don't serialize; `saveOneNow` is for callers already inside a collection write queue. Boot-time `verifyCollectionVersions([store, ...])` logs schema-version mismatches. **Type-index `config` slot** holds cross-record state (see the `TypeIndexConfig` typedef + header convention): `{ runs?: [], featureFlags?: {}, lockPolicies?: {} }` — `runs` is the shipped slot (universeBuilder's capped history log), the other two are reserved names; consumers may add their own keys but should reuse a reserved name when it fits and document the shape next to the consumer. `saveTypeIndex({ config })` shallow-merges `config` one level deep (a patched `runs` replaces the whole array), so a read-modify-write of a slot must load → mutate a copy → write inside `queueTypeIndexWrite(fn)`. | | `conflictJournal.js` | Non-blocking edit-conflict journal for cross-install LWW merges. `maybeJournalBeforeOverwrite({kind,id,local,remote,source})` (call right before a merge overwrite) archives the losing local version when a true 3-way divergence is detected (`detectConflict` via per-record `syncBaseHash` + `contentHashForRecord`), then advances the base hash; `flushBaseHashes()` persists the batched base-hash side store; `withBaseHashFlushBatch(fn)` defers every interior flush so an await-separated multi-record loop — the peer:online convergence push and every base-hash-evicting `pruneTombstoned*` loop — collapses N `sync_base_hashes.json` rewrites into one terminal write (depth-counted, so concurrent batches merge too; flushes in `finally`). `deleteSyncBaseHash(kind,id)` evicts a record's base hash when its tombstone is hard-pruned, so those paths don't let the side store grow without bound; `pruneOrphanedBaseHashes(resolves)` is the backstop sweep (`resolves(kind,id) => bool`; unknown kinds kept) that drops keys whose record no longer resolves, wired into the tombstone GC sweep. `conflictJournalStore()` is the `pending`/`resolved` entry store (discard resolves an entry; DELETE hard-removes it — there is no `dismissed` status). Local-only — never crosses the wire. | | `schemaVersions.js` | Cross-instance sync version contract. `PORTOS_SCHEMA_VERSIONS` (frozen map of `{ category: layoutVersion }`), `RECORD_KIND_SCHEMA_CATEGORIES` (frozen map of federated record kind → the schema categories it writes), `buildPortosMeta()` (envelope for every outbound sync payload), `compareSchemaVersions(sender, receiver)` returning `{ ahead, behind, compatible }`, `scopeVersionDiff(diff, categories)` (restrict that diff to the categories a specific transfer touches), and `formatVersionGap()` for UI/log lines. Receivers gate `applyIncomingPush` / share-bucket import / snapshot apply per-category on the scoped comparator result so an upgraded sender can't corrupt a downstream peer — and a bump to one category doesn't sever sync of the others. | -| `dataRoot.js` | Data-root resolution + worktree-checkout detection (#1947). `resolveInstallRoot(fallbackRoot)` prefers the `PORTOS_DATA_ROOT` env var (pinned at real launch in `ecosystem.config.cjs`) over an `import.meta.url`-derived fallback, so a process booted from inside a CoS agent git worktree still resolves `data/`/`data.reference/` to the real install instead of the worktree's empty tree. `isWorktreeRoot(rootDir)` is the boot-migration backstop — true when `rootDir` lives under `data/cos/worktrees/` (keyed on the path segment only, so a fresh install's empty `data/` isn't a false positive). `DATA_ROOT_ENV` is the env-var name constant. Consumed by `fileUtils.js` (`PATHS`), `server/index.js`, and `scripts/run-migrations.js`. | +| `dataRoot.js` | Data-root resolution + worktree-checkout detection (#1947). `resolveInstallRoot(fallbackRoot)` prefers the `PORTOS_DATA_ROOT` env var (pinned at real launch in `ecosystem.config.cjs`) over an `import.meta.url`-derived fallback, so a process booted from inside a CoS agent git worktree still resolves `data/`/`data.reference/` to the real install instead of the worktree's empty tree. `isWorktreeRoot(rootDir)` is the boot-migration backstop — true when `rootDir` lives under `data/cos/worktrees/` (keyed on the path segment only, so a fresh install's empty `data/` isn't a false positive). `resolveCodeRootForModule(moduleUrl)` is the single source of truth for the "two directories above this file" depth assumption — `paths.js`'s `CODE_ROOT` and `services/userActions.js`'s data-root guard both derive through it so they cannot silently drift apart. `DATA_ROOT_ENV` is the env-var name constant. Consumed by `fileUtils.js` (`PATHS`), `server/index.js`, and `scripts/run-migrations.js`. | | `agentInstructionsFile.js` | The `AGENTS.md` + bridge `CLAUDE.md` pair a repo carries (#4852). `writeAgentInstructions(repoPath, content)` writes the body to `AGENTS.md` and the one-line `@AGENTS.md` import beside it — use it in scaffolders instead of a bare `writeFile(join(repoPath, 'CLAUDE.md'), …)`, since a generated repo carrying only one name is unreadable to half the CLIs PortOS can point at it. Constants: `AGENT_INSTRUCTIONS_FILENAME`, `CLAUDE_BRIDGE_FILENAME`, `AGENT_INSTRUCTIONS_IMPORT`. | | `fileCore.js` | Cross-cutting filesystem primitives (`atomicWrite`, directory helpers, bounded tail reads/watchers), time/format helpers, directory sizing, and SHA-256 helpers. | | `fileUtils.js` | Backward-compatible facade re-exporting the focused file utility modules so existing deep imports need no caller changes. | diff --git a/server/services/userActions.js b/server/services/userActions.js index 6e68fb3147..9bd9e5533e 100644 --- a/server/services/userActions.js +++ b/server/services/userActions.js @@ -71,20 +71,27 @@ const REAL_REPO_DATA_DIR = join(resolveInstallRoot(resolveCodeRootForModule(impo * `user-action-events.json` into the developer's live `data/` tree the next * time such a route gets exercised — #5594 patched three known offenders * one at a time, which is a per-suite fix, not a guard against the next one. - * Fires only under the test runner, and only at the moment a write is - * actually attempted, so a suite that merely reads (or never triggers a - * `recordUserAction` call) is unaffected either way. + * Fires only under the test runner, and only at the moment the file backend + * actually touches the ledger, so a suite that never reaches `recordUserAction` + * / `listUserActions` is unaffected either way. + * + * Reads are guarded as well as writes: the live ledger holds machine-local + * operator records (ADR docs/decisions/2026-08-08-privacy-records-machine-local.md), + * so an untethered suite must not pull them into the test process either. + * + * @param {string} attempted what the file backend was about to do, e.g. + * `'recordUserAction attempted a write of'` */ -function assertTestDataRootRedirected() { +function assertTestDataRootRedirected(attempted) { if (!isTestRunner() || PATHS.data !== REAL_REPO_DATA_DIR) return; throw new Error( - 'recordUserAction attempted a file-backend write of user-action-events.json ' + - "into the repo's real data/ tree. This suite exercises a route wired to " + - 'recordUserAction but never redirected PATHS.data to a temp root - mock ' + - "`../lib/fileUtils.js` with lib/mockPathsDataRoot.js's makePathsProxy/" + - 'createTempDataRoot (the same fix #5594 applied to cos.test.js / ' + - 'cosTaskRoutes.test.js / cosAgentFeedback.test.js) rather than letting the ' + - 'write land here.', + `${attempted} user-action-events.json in the repo's real data/ tree. ` + + 'This suite exercises the user-action ledger but never redirected ' + + 'PATHS.data to a temp root - mock `../lib/fileUtils.js` with ' + + "lib/mockPathsDataRoot.js's makePathsProxy/createTempDataRoot (the same " + + 'fix #5594 applied to cos.test.js / cosTaskRoutes.test.js / ' + + 'cosAgentFeedback.test.js) rather than letting the file backend touch the ' + + 'real tree.', ); } @@ -320,7 +327,7 @@ function makeFileBackend() { // BEFORE this guard ever ran, silently no-op'ing past it with no throw — // and by then loadFileEvents() had already read the real ledger into the // test process regardless. Running the guard first closes both holes. - assertTestDataRootRedirected(); + assertTestDataRootRedirected('recordUserAction attempted a file-backend write of'); const events = await loadFileEvents(); if (events.some((row) => row.type === event.type && row.dedupeKey === event.dedupeKey)) return null; await ensureDir(PATHS.data); @@ -328,6 +335,10 @@ function makeFileBackend() { return event; }), list: async (filters) => { + // Same guard as the write path: an un-redirected suite must not get to READ + // the developer's live ledger either — those rows are machine-local operator + // records (docs/decisions/2026-08-08-privacy-records-machine-local.md). + assertTestDataRootRedirected('listUserActions attempted a file-backend read of'); const events = await loadFileEvents(); return events .filter((event) => matchesFilters(event, filters)) diff --git a/server/services/userActionsDataRootGuard.test.js b/server/services/userActionsDataRootGuard.test.js index be961d4824..5ea4a51cb3 100644 --- a/server/services/userActionsDataRootGuard.test.js +++ b/server/services/userActionsDataRootGuard.test.js @@ -11,46 +11,54 @@ * would otherwise be the test that writes a real file into the repo's data/ * tree, so it also asserts that never happens. */ -import { describe, it, expect } from 'vitest'; -import { existsSync, readFileSync, rmSync } from 'node:fs'; +import { describe, it, expect, afterEach } from 'vitest'; +import { existsSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { join } from 'node:path'; import { PATHS } from '../lib/fileUtils.js'; -import { recordUserAction } from './userActions.js'; +import { listUserActions, recordUserAction } from './userActions.js'; const REAL_EVENTS_FILE = join(PATHS.data, 'user-action-events.json'); -describe('recordUserAction — data-root guard (#5605)', () => { - it('throws instead of writing user-action-events.json into the real data/ tree', async () => { - // Snapshot whatever is already there BEFORE exercising the guard. An - // install using the documented MEMORY_BACKEND=file escape hatch may - // legitimately already have this file as its real ledger — this test - // must prove the guard leaves it untouched, not assert it's absent (the - // previous version's `expect(existsSync(...)).toBe(false)` failed - // spuriously on exactly that install shape, and its unconditional - // afterEach `rmSync` then deleted the developer's real ledger). - const existedBefore = existsSync(REAL_EVENTS_FILE); - const contentBefore = existedBefore ? readFileSync(REAL_EVENTS_FILE, 'utf8') : null; +// Snapshot ONCE, before any case runs. An install using the documented +// MEMORY_BACKEND=file escape hatch may legitimately already have this file as +// its real ledger, so every case must prove the guard leaves it untouched +// rather than assert it is absent — and `afterEach` must put back exactly what +// it found, including when a regression mutated the bytes. +const EXISTED_BEFORE = existsSync(REAL_EVENTS_FILE); +const CONTENT_BEFORE = EXISTED_BEFORE ? readFileSync(REAL_EVENTS_FILE, 'utf8') : null; - try { - await expect(recordUserAction({ - type: 'cos.task.create', - summary: 'Guard regression probe', - dedupeKey: `guard-probe-${Math.random().toString(36).slice(2)}`, - })).rejects.toThrow(/real data\/ tree/); +/** The real tree must come back exactly as this file found it — present and byte-identical, or still absent. */ +function expectRealTreeUntouched() { + expect(existsSync(REAL_EVENTS_FILE)).toBe(EXISTED_BEFORE); + if (EXISTED_BEFORE) expect(readFileSync(REAL_EVENTS_FILE, 'utf8')).toBe(CONTENT_BEFORE); +} - // The guard must leave the real tree exactly as it found it — present - // and unchanged, or still absent — never newly created or modified. - expect(existsSync(REAL_EVENTS_FILE)).toBe(existedBefore); - if (existedBefore) { - expect(readFileSync(REAL_EVENTS_FILE, 'utf8')).toBe(contentBefore); - } - } finally { - // Only clean up a file THIS run's own guard regression created — never - // touch one that was already there before it ran (which could be a - // developer's real MEMORY_BACKEND=file ledger). - if (!existedBefore && existsSync(REAL_EVENTS_FILE)) { - rmSync(REAL_EVENTS_FILE, { force: true }); - } - } +afterEach(() => { + // Repair anything a GUARD REGRESSION did, never anything the developer had: + // delete only a file this run leaked, and restore only bytes this run changed. + if (!EXISTED_BEFORE) { + if (existsSync(REAL_EVENTS_FILE)) rmSync(REAL_EVENTS_FILE, { force: true }); + return; + } + if (readFileSync(REAL_EVENTS_FILE, 'utf8') !== CONTENT_BEFORE) { + writeFileSync(REAL_EVENTS_FILE, CONTENT_BEFORE); + } +}); + +describe('user-action ledger — data-root guard (#5605)', () => { + it('recordUserAction throws instead of writing into the real data/ tree', async () => { + await expect(recordUserAction({ + type: 'cos.task.create', + summary: 'Guard regression probe', + dedupeKey: `guard-probe-${Math.random().toString(36).slice(2)}`, + })).rejects.toThrow(/real data\/ tree/); + expectRealTreeUntouched(); + }); + + it('listUserActions throws instead of reading the real ledger', async () => { + // The read path matters too: those rows are machine-local operator records + // (privacy ADR), so an untethered suite must not pull them into the process. + await expect(listUserActions()).rejects.toThrow(/real data\/ tree/); + expectRealTreeUntouched(); }); });