Files
callstack__agent-device/scripts/layering/daemon-modularity.ts
Michał Pierzchała 62001cf210 refactor(record): derive session recording from the publication lifecycle (#1719)
* refactor(record): derive session recording from the publication lifecycle

`SessionState.recordSession` stored an answer the script-publication
aggregate already contained. Every writer set both, but nothing made them
agree, and #1533 was the consequence: a `--save-script` ingress re-armed
the flag behind an ABORTED status, and a bare `close` published a
recording the caller had been told was aborted.

That fix routed every write through one rule, which made the two agree
without making disagreement unrepresentable. The field remained a second
source of truth, and its doc comments had to carry the invariant that a
type could enforce.

Remove the field and derive the answer. `isRecordingPublication` reads
recording off the lifecycle: ordinary authoring records only while ARMED;
a repair transaction records for its whole lifetime, terminal statuses
included. That last clause is deliberately exact rather than merely safe —
`armRepairStep` armed the old flag and neither `abortRepair` nor
`commitRepair` ever cleared it, so narrowing it would silently stop
evidence capture for a committed repair. Whether it should is a real
question, and a behavior change, so it is left alone here.

What this buys, beyond one less field:

- `buildNextOpenSession` and `finalizeOrdinaryCloseScript` make no
  recording decision at all now, so no surface can arm recording without
  moving the lifecycle that authorizes it.
- The writer's publication gate is answered entirely by the aggregate. Its
  separate ABORTED check is gone: a terminal authoring lifecycle is
  already not recording, so one question replaces two that could disagree.
- The R7 ownership ratchet drops from 23 writer-owned fields / 29 owner
  claims to 22 / 26, and the layering manifest loses the entry whose
  comment documented the smell ("deliberately set on its own by paths that
  record without arming a publication").

Behavior-preserving: the derivation reproduces what the flag held at every
transition. The test fixtures that armed `recordSession` with no
publication state described a shape production stopped producing at #1478;
they now carry the lifecycle that causes recording.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PFW9gJqz1wEHoowkdd1nFW

* test(close-script): flush queued event-log writes before removing the tmp root

CI failed the Coverage lane with ENOTEMPTY removing the test's tmp root,
in `afterEach` rather than in an assertion.

`SessionStore.recordAction` QUEUES its event-log append
(`queueEventLogWrite`) instead of writing it, and every close path in this
file records an action. Nothing awaited that write, so `fs.rmSync(root,
{recursive: true})` could race it: the pending append recreates
`<root>/sessions/<name>/` while rmSync is walking, and the final rmdir
fails ENOTEMPTY. It needs CI's parallel load to lose the race — the file
passes 12/12 in isolation locally.

Await `flushSessionEventLogWrites()` before removing. The hazard is latent
in any test that records actions and then removes its tmp root; this fixes
the file that failed rather than sweeping the pattern, which deserves its
own change.

Not added to the #1419 contention-retry list: that list requires a
concrete spawn/wait mechanism named per entry, and this file has none. The
race was a real teardown bug, not lane contention.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PFW9gJqz1wEHoowkdd1nFW

* docs: correct ADR 0016 on recording vs publication for repair

Review caught a real overstatement. The amendment claimed evidence capture
and publication authorization are "the same question asked of the same
state". That holds for ordinary authoring — ARMED both records and
publishes, ABORTED and PUBLISHED do neither — but not for repair:
`isRecordingPublication` is true for every repair status including
`committed` and `aborted`, while the writer additionally applies
`isRepairArmedWriteBlocked`, refusing a committed transaction and one that
is not yet committable.

State it as it is: both decisions derive from the same aggregate, but they
remain distinct predicates, and collapsing them would republish a committed
repair or commit an incomplete prefix.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PFW9gJqz1wEHoowkdd1nFW

---------

Co-authored-by: Claude <noreply@anthropic.com>
2026-08-11 10:58:03 +02:00

297 lines
11 KiB
TypeScript

import path from 'node:path';
import { targetDagZone, type LayeringViolation, type ResolvedImportEdge } from './model.ts';
import { SESSION_STATE_FIELD_OWNERS } from './session-state.ts';
const LARGEST_TYPE_CYCLE_ZONE_CEILINGS: Readonly<Record<string, number>> = {
'(root)': 3,
client: 1,
commands: 14,
core: 10,
'daemon-server': 17,
platforms: 2,
};
export const DAEMON_MODULARITY_BASELINE = {
sessionState: {
writerOwnedFields: 21,
ownerFileClaims: 25,
},
largestTypeCycle: {
zoneMembers: LARGEST_TYPE_CYCLE_ZONE_CEILINGS,
},
externalDaemonTypesImporters: [
'src/client/client-normalizers.ts',
'src/remote/daemon-artifacts.ts',
],
} as const;
export const TYPE_CYCLE_BASELINE = Object.values(LARGEST_TYPE_CYCLE_ZONE_CEILINGS).reduce(
(sum, count) => sum + count,
0,
);
type LogicalModulePolicy = {
name: string;
roots: readonly string[];
forbiddenTargetRoots: readonly string[];
/**
* Imports that already violate `forbiddenTargetRoots` on the day the rule was written, recorded
* as `source -> target`. The rule enforces immediately for everything else, so a new violation
* cannot be added while the module waits for its extraction PR; each recorded edge must be
* deleted from this list by the change that removes the import, and re-adding one is a diff a
* reviewer sees.
*/
recordedMigrationImports?: readonly string[];
};
/**
* Zero-count targets for the accepted daemon modularity design. A root may be absent today:
* the policy starts enforcing as soon as the first file is added, without scaffolding an empty
* façade or package merely to make the gate concrete.
*/
export const LOGICAL_MODULE_POLICIES: readonly LogicalModulePolicy[] = [
{
name: 'ad-replay',
roots: ['packages/ad-replay/src/'],
forbiddenTargetRoots: [
'src/daemon/',
'src/platforms/',
'src/providers/',
'src/compat/',
'packages/maestro/',
],
},
{
name: 'maestro',
roots: ['packages/maestro/src/'],
forbiddenTargetRoots: [
'src/daemon/',
'src/platforms/',
'src/providers/',
'packages/ad-replay/',
],
},
{
// Replay-test schedules and reports; it must stay format-neutral. `src/request/` is
// request-global daemon plumbing (progress sinks, cancellation, AsyncLocalStorage), and the
// remaining roots are engine internals — reaching into either is how a scheduler quietly
// acquires daemon authority or an engine-specific value shape.
name: 'replay-test',
roots: ['packages/replay-test/src/'],
forbiddenTargetRoots: [
'src/daemon/',
'src/platforms/',
'src/providers/',
'src/request/',
'src/replay/',
'src/compat/',
'packages/maestro/',
'packages/ad-replay/',
],
},
];
const ENGINE_FILE_PREFIXES = [
'packages/ad-replay/src/',
'packages/maestro/src/',
'src/replay/',
'src/daemon/handlers/session-replay',
'packages/replay-test/src/',
] as const;
export function checkDaemonModularityRatchets(
edges: readonly ResolvedImportEdge[],
largestTypeCycleMembers: readonly string[],
): LayeringViolation[] {
return [
...checkSessionStateBaseline(),
...checkTypeCycleBaseline(largestTypeCycleMembers),
...checkDaemonTypesImporters(edges),
...checkLogicalModuleImports(edges),
];
}
function checkSessionStateBaseline(): LayeringViolation[] {
const actual = {
writerOwnedFields: Object.keys(SESSION_STATE_FIELD_OWNERS).length,
ownerFileClaims: Object.values(SESSION_STATE_FIELD_OWNERS).reduce(
(sum, owners) => sum + owners.length,
0,
),
};
const violations: LayeringViolation[] = [];
for (const metric of ['writerOwnedFields', 'ownerFileClaims'] as const) {
const baseline = DAEMON_MODULARITY_BASELINE.sessionState[metric];
if (actual[metric] === baseline) continue;
violations.push({
rule: 'R10 daemon-modularity',
file: 'scripts/layering/daemon-modularity.ts',
line: 1,
message:
actual[metric] > baseline
? `R7 ${metric} grew to ${actual[metric]} (baseline ${baseline}). Route the new write through an existing owner instead.`
: `R7 ${metric} dropped to ${actual[metric]} — lower the daemon modularity baseline in the same capability move so it cannot regrow.`,
});
}
return violations;
}
function checkTypeCycleBaseline(members: readonly string[]): LayeringViolation[] {
const violations: LayeringViolation[] = [];
const baseline = DAEMON_MODULARITY_BASELINE.largestTypeCycle;
if (members.length > TYPE_CYCLE_BASELINE) {
violations.push({
rule: 'R9 type-cycle-growth',
file: 'scripts/layering/daemon-modularity.ts',
line: 1,
message:
`the largest type-level import cycle grew to ${members.length} files (baseline ` +
`${TYPE_CYCLE_BASELINE}). A type-only import that closes a loop makes every file in the ` +
`loop unreadable in isolation. Declare the shared type below both modules, or if the growth ` +
`is genuinely warranted, raise the zone ceilings in the same commit and say why.`,
});
}
const zoneCounts = countBy(members, targetDagZone);
for (const [zone, count] of zoneCounts) {
const allowed = baseline.zoneMembers[zone] ?? 0;
if (count <= allowed) continue;
violations.push({
rule: 'R10 daemon-modularity',
file: members.find((member) => targetDagZone(member) === zone) ?? 'scripts/layering/check.ts',
line: 1,
message: `the largest type cycle now contains ${count} ${zone} file(s) (baseline ${allowed}); extraction must not trade one zone's locality for another's.`,
});
}
for (const member of members) {
if (!ENGINE_FILE_PREFIXES.some((prefix) => member.startsWith(prefix))) continue;
violations.push({
rule: 'R10 daemon-modularity',
file: member,
line: 1,
message:
'an engine file entered the largest type cycle. Keep engine contracts neutral and adapters outside the engine so extraction does not worsen R9.',
});
}
return violations;
}
function checkDaemonTypesImporters(edges: readonly ResolvedImportEdge[]): LayeringViolation[] {
const allowed = new Set<string>(DAEMON_MODULARITY_BASELINE.externalDaemonTypesImporters);
const importers = new Map<string, ResolvedImportEdge>();
for (const edge of edges) {
if (edge.target !== 'src/daemon/types.ts' || edge.file.startsWith('src/daemon/')) continue;
importers.set(edge.file, edge);
}
const violations = [...importers]
.filter(([file]) => !allowed.has(file))
.map(([file, edge]) => ({
rule: 'R10 daemon-modularity',
file,
line: edge.line,
message:
`external production imports of daemon/types.ts may only shrink from the recorded ${allowed.size}. ` +
'Use an existing neutral contract; do not move DaemonRequest into contracts to satisfy this gate.',
}));
for (const file of allowed) {
if (importers.has(file)) continue;
violations.push({
rule: 'R10 daemon-modularity',
file: 'scripts/layering/daemon-modularity.ts',
line: 1,
message: `${file} no longer imports daemon/types.ts — delete it from externalDaemonTypesImporters in the same change so the dependency cannot return.`,
});
}
return violations;
}
function checkLogicalModuleImports(edges: readonly ResolvedImportEdge[]): LayeringViolation[] {
const violations: LayeringViolation[] = [];
const observedMigrationImports = new Set<string>();
for (const edge of edges) {
const sourceModule = moduleForFile(edge.file);
const targetModule = moduleForFile(edge.target);
if (
targetModule &&
sourceModule !== targetModule &&
isInsideInternalTree(edge.target, targetModule.roots)
) {
violations.push({
rule: 'R10 daemon-modularity',
file: edge.file,
line: edge.line,
message: `${edge.file} must not import ${targetModule.name}'s internal tree (${edge.target}); use that module's façade.`,
});
continue;
}
if (!sourceModule) continue;
// A module's own files are never a forbidden target: `replay-test` sits inside the wider
// `src/replay/` engine root it may not import from.
if (sourceModule.roots.some((root) => edge.target.startsWith(root))) continue;
if (!sourceModule.forbiddenTargetRoots.some((root) => edge.target.startsWith(root))) continue;
const migrationImport = `${edge.file} -> ${edge.target}`;
if (sourceModule.recordedMigrationImports?.includes(migrationImport)) {
observedMigrationImports.add(migrationImport);
continue;
}
violations.push({
rule: 'R10 daemon-modularity',
file: edge.file,
line: edge.line,
message: `${sourceModule.name} must not import ${edge.target}; communicate through its façade and a narrow port with two real adapters.`,
});
}
return [...violations, ...checkRecordedMigrationImports(observedMigrationImports)];
}
function checkRecordedMigrationImports(observed: ReadonlySet<string>): LayeringViolation[] {
const violations: LayeringViolation[] = [];
for (const module of LOGICAL_MODULE_POLICIES) {
for (const migrationImport of module.recordedMigrationImports ?? []) {
if (observed.has(migrationImport)) continue;
violations.push({
rule: 'R10 daemon-modularity',
file: 'scripts/layering/daemon-modularity.ts',
line: 1,
message: `${migrationImport} no longer exists — delete it from ${module.name}'s recordedMigrationImports in the same change so the import cannot return.`,
});
}
}
return violations;
}
function moduleForFile(file: string): LogicalModulePolicy | undefined {
return LOGICAL_MODULE_POLICIES.find((module) =>
module.roots.some((root) => file.startsWith(root)),
);
}
function isInsideInternalTree(file: string, roots: readonly string[]): boolean {
return roots.some((root) => file.startsWith(path.posix.join(root, 'internal/')));
}
function countBy(values: readonly string[], keyOf: (value: string) => string): Map<string, number> {
const counts = new Map<string, number>();
for (const value of values) {
const key = keyOf(value);
counts.set(key, (counts.get(key) ?? 0) + 1);
}
return counts;
}
export function daemonModularitySummary(): string {
const session = DAEMON_MODULARITY_BASELINE.sessionState;
const recordedMigrationImports = LOGICAL_MODULE_POLICIES.reduce(
(sum, module) => sum + (module.recordedMigrationImports?.length ?? 0),
0,
);
return (
`R10 pins R7 at ${session.writerOwnedFields} writer-owned fields / ` +
`${session.ownerFileClaims} owner claims, R9 at ${TYPE_CYCLE_BASELINE} files with zone ceilings, ` +
`${DAEMON_MODULARITY_BASELINE.externalDaemonTypesImporters.length} external daemon/types.ts importers, ` +
`and zero forbidden logical-module imports beyond ${recordedMigrationImports} recorded migration import(s)`
);
}