Skip to content

Commit 6e22e26

Browse files
authored
refactor(contracts): own the daemon HTTP wire contract so clients stop importing src/daemon (#2322)
* refactor(contracts): own the daemon HTTP wire contract so clients stop importing src/daemon Move the pure wire vocabulary (base path, header names, URL/auth/tenant builders, /health payload) from src/daemon into @agent-device/contracts as the daemon-http subpath, so src/remote and src/cli stop importing daemon server internals. buildDaemonHealthPayload takes the version its caller advertises (R18 keeps host mechanics out of contracts); both callers pass readVersion(). Wire-compat surface, mutation, and ledger references follow the package path. * chore(gates): pin the moved daemon HTTP wire surface and teach the released-baseline check file moves Exports map + snapshot gain the daemon-http subpath. The wire ledger re-keys the eight moved declarations (buildDaemonHealthPayload moves with its new caller-supplied version parameter, acked additive). The released-baseline comparison now classifies a baseline declaration that re-appears unchanged at exactly one new path as a move instead of a removal: a file move is not wire surface a released peer stopped sending. A move that changes shape is a change acked at the destination path, and a name still owned by the baseline stays a removal.
1 parent 0bebbe8 commit 6e22e26

28 files changed

Lines changed: 364 additions & 106 deletions

packages/contracts/package.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,10 @@
163163
"types": "./src/command-platform-execution.ts",
164164
"default": "./src/command-platform-execution.ts"
165165
},
166+
"./daemon-http": {
167+
"types": "./src/daemon-http.ts",
168+
"default": "./src/daemon-http.ts"
169+
},
166170
"./daemon-owner-cleanup": {
167171
"types": "./src/daemon-owner-cleanup.ts",
168172
"default": "./src/daemon-owner-cleanup.ts"

src/daemon/__tests__/http-contract.test.ts renamed to packages/contracts/src/daemon-http.test.ts

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
import { test } from 'vitest';
22
import assert from 'node:assert/strict';
33
import {
4+
buildDaemonHealthPayload,
45
buildDaemonHttpAuthHeaders,
56
buildDaemonHttpBaseUrl,
67
buildDaemonHttpTenantHeaders,
78
buildDaemonHttpUrl,
8-
} from '../http-contract.ts';
9+
DAEMON_RPC_PROTOCOL_VERSION,
10+
} from './daemon-http.ts';
911

1012
test('buildDaemonHttpBaseUrl appends the public agent-device base path', () => {
1113
assert.equal(
@@ -43,3 +45,22 @@ test('buildDaemonHttpTenantHeaders omits blank tenant identities', () => {
4345
});
4446
assert.deepEqual(buildDaemonHttpTenantHeaders(''), {});
4547
});
48+
49+
test('buildDaemonHealthPayload takes the version from its caller and keeps the payload shape', () => {
50+
assert.deepEqual(buildDaemonHealthPayload('agent-device-daemon', '0.20.9'), {
51+
ok: true,
52+
service: 'agent-device-daemon',
53+
version: '0.20.9',
54+
rpcProtocolVersion: DAEMON_RPC_PROTOCOL_VERSION,
55+
});
56+
assert.deepEqual(
57+
buildDaemonHealthPayload('agent-device-proxy', '0.20.9', { upstream: { ok: true } }),
58+
{
59+
ok: true,
60+
service: 'agent-device-proxy',
61+
version: '0.20.9',
62+
rpcProtocolVersion: DAEMON_RPC_PROTOCOL_VERSION,
63+
upstream: { ok: true },
64+
},
65+
);
66+
});
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
// The daemon HTTP wire vocabulary shared by the daemon server, the remote
2+
// proxy, and every client that talks to them: the base path, the tenant and
3+
// network-access header names, the URL/auth/tenant header builders, and the
4+
// /health payload. Client and server must agree on all of it, so neither side
5+
// owns it (ADR 0006).
6+
export const DAEMON_HTTP_BASE_PATH = '/agent-device';
7+
export const DAEMON_HTTP_TENANT_HEADER = 'x-agent-device-tenant';
8+
export const DAEMON_HTTP_NETWORK_ACCESS_HEADER = 'x-agent-device-network-access';
9+
export const DAEMON_HTTP_PUBLIC_NETWORK_ACCESS = 'public-only';
10+
11+
export function buildDaemonHttpBaseUrl(baseUrl: string): string {
12+
return buildDaemonHttpUrl(baseUrl, DAEMON_HTTP_BASE_PATH);
13+
}
14+
15+
export function buildDaemonHttpUrl(baseUrl: string, route: string): string {
16+
const normalizedBase = baseUrl.endsWith('/') ? baseUrl : `${baseUrl}/`;
17+
return new URL(route.replace(/^\/+/, ''), normalizedBase).toString();
18+
}
19+
20+
export function buildDaemonHttpAuthHeaders(token: string | undefined): Record<string, string> {
21+
const normalizedToken = token?.trim();
22+
if (!normalizedToken) return {};
23+
return {
24+
authorization: `Bearer ${normalizedToken}`,
25+
'x-agent-device-token': normalizedToken,
26+
};
27+
}
28+
29+
export function buildDaemonHttpTenantHeaders(tenantId: string | undefined): Record<string, string> {
30+
const normalizedTenantId = tenantId?.trim();
31+
if (!normalizedTenantId) return {};
32+
return { [DAEMON_HTTP_TENANT_HEADER]: normalizedTenantId };
33+
}
34+
35+
// See docs/adr/0006-daemon-rpc-protocol-version.md before changing this value.
36+
// Enforced, not just documented: `test/wire-compat/` digests the declarations
37+
// that cross this boundary and fails when one changes shape without a bump or
38+
// an acknowledged-compatible entry (#1432).
39+
export const DAEMON_RPC_PROTOCOL_VERSION = 2;
40+
41+
export type DaemonHealthPayload = {
42+
ok: true;
43+
service: 'agent-device-daemon' | 'agent-device-proxy';
44+
version: string;
45+
rpcProtocolVersion: number;
46+
upstream?: unknown;
47+
};
48+
49+
export function buildDaemonHealthPayload(
50+
service: DaemonHealthPayload['service'],
51+
version: string,
52+
options: { upstream?: unknown } = {},
53+
): DaemonHealthPayload {
54+
return {
55+
ok: true,
56+
service,
57+
version,
58+
rpcProtocolVersion: DAEMON_RPC_PROTOCOL_VERSION,
59+
...(options.upstream !== undefined ? { upstream: options.upstream } : {}),
60+
};
61+
}

scripts/ios-snapshot-benchmark/proxy-client-support.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import fs from 'node:fs';
22
import { performance } from 'node:perf_hooks';
33
import path from 'node:path';
44
import { pathToFileURL } from 'node:url';
5-
import { buildDaemonHttpBaseUrl } from '../../src/daemon/http-contract.ts';
5+
import { buildDaemonHttpBaseUrl } from '@agent-device/contracts/daemon-http';
66
import {
77
BenchmarkCellAdmissionError,
88
BenchmarkContentionError,

scripts/layering/contracts-exports.snapshot.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
"@agent-device/contracts/clipboard-runtime",
3838
"@agent-device/contracts/command",
3939
"@agent-device/contracts/command-platform-execution",
40+
"@agent-device/contracts/daemon-http",
4041
"@agent-device/contracts/daemon-owner-cleanup",
4142
"@agent-device/contracts/device",
4243
"@agent-device/contracts/device-readiness-runtime",

scripts/wire-compat/model.test.ts

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,131 @@ test('a removed wire declaration passes with a bump', () => {
112112
assert.deepEqual(result.failures, []);
113113
});
114114

115+
// #2318 moved the daemon HTTP wire contract from src/daemon to
116+
// packages/contracts. A declaration that left its path but was re-declared
117+
// unchanged is a file move, not a removal: a released peer still parses it,
118+
// so it must not force a protocol bump.
119+
test('a declaration moved to a new file unchanged is a move, not a removal', () => {
120+
const movedFrom = 'src/daemon/http-contract.ts#buildDaemonHttpUrl';
121+
const movedTo = 'packages/contracts/src/daemon-http.ts#buildDaemonHttpUrl';
122+
const released = ledger({
123+
declarations: { [movedFrom]: 'sha256:ddd', [META]: 'sha256:bbb' },
124+
});
125+
const current = ledger({ declarations: { [movedTo]: 'sha256:ddd', [META]: 'sha256:bbb' } });
126+
const result = compareWireLedgers({
127+
baselineTag: 'v0.20.6',
128+
released,
129+
current,
130+
digests: new Map(Object.entries({ [movedTo]: 'sha256:ddd', [META]: 'sha256:bbb' })),
131+
});
132+
assert.deepEqual(result.moved, [movedFrom]);
133+
assert.deepEqual(result.removed, []);
134+
assert.deepEqual(result.added, []);
135+
assert.deepEqual(result.failures, []);
136+
});
137+
138+
// A baseline key that leaves and a same-named key that arrives with a moved
139+
// digest are textually indistinguishable, so the gate reads the pair as a
140+
// CHANGE at the destination (ackable at the new digest) rather than the
141+
// bump-only removal the key-pair alone would suggest.
142+
test('a move that changes the shape is a change, acked at the path it moved to', () => {
143+
const movedFrom = 'src/daemon/http-health.ts#buildDaemonHealthPayload';
144+
const movedTo = 'packages/contracts/src/daemon-http.ts#buildDaemonHealthPayload';
145+
const released = ledger({ declarations: { [movedFrom]: 'sha256:old' } });
146+
const current = ledger({
147+
declarations: { [movedTo]: 'sha256:new' },
148+
compatibleChanges: [
149+
{
150+
declaration: movedTo,
151+
digest: 'sha256:new',
152+
rationale: 'The added parameter is caller-local; the wire payload is unchanged.',
153+
},
154+
],
155+
});
156+
const digests = new Map(Object.entries({ [movedTo]: 'sha256:new' }));
157+
const withoutAck = compareWireLedgers({
158+
baselineTag: 'v0.20.6',
159+
released,
160+
current: ledger({ declarations: { [movedTo]: 'sha256:new' } }),
161+
digests,
162+
});
163+
assert.deepEqual(withoutAck.changed, [movedTo]);
164+
assert.deepEqual(withoutAck.removed, []);
165+
assert.equal(withoutAck.failures.length, 1);
166+
const withAck = compareWireLedgers({ baselineTag: 'v0.20.6', released, current, digests });
167+
assert.deepEqual(withAck.failures, []);
168+
});
169+
170+
// Names are not unique across files: while a baseline declaration still owns
171+
// the name at its own path, a same-named declaration elsewhere cannot be
172+
// identified as a move of this one, so the baseline key stays a removal.
173+
test('a name still owned by the baseline is not a move, so it remains a removal', () => {
174+
const serverSendJson = 'src/daemon/server/http-server.ts#sendJson';
175+
const uploadSendJson = 'src/daemon/upload-http.ts#sendJson';
176+
const released = ledger({
177+
declarations: { [serverSendJson]: 'sha256:old', [uploadSendJson]: 'sha256:eee' },
178+
});
179+
const current = ledger({ declarations: { [uploadSendJson]: 'sha256:eee' } });
180+
const result = compareWireLedgers({
181+
baselineTag: 'v0.20.6',
182+
released,
183+
current,
184+
digests: new Map(Object.entries({ [uploadSendJson]: 'sha256:eee' })),
185+
});
186+
assert.deepEqual(result.removed, [serverSendJson]);
187+
assert.deepEqual(result.moved, []);
188+
assert.equal(result.failures.length, 1);
189+
});
190+
191+
// One destination cannot be two declarations' move. When two same-name
192+
// baseline declarations leave their paths and one same-name path arrives, one
193+
// of them is a real removal, and a removal is bump-only.
194+
test('two same-name removals cannot share one move destination, so both stay removals', () => {
195+
const first = 'src/a.ts#sendJson';
196+
const second = 'src/b.ts#sendJson';
197+
const destination = 'src/c.ts#sendJson';
198+
const released = ledger({ declarations: { [first]: 'sha256:ddd', [second]: 'sha256:ddd' } });
199+
const current = ledger({ declarations: { [destination]: 'sha256:ddd' } });
200+
const result = compareWireLedgers({
201+
baselineTag: 'v0.20.6',
202+
released,
203+
current,
204+
digests: new Map(Object.entries({ [destination]: 'sha256:ddd' })),
205+
});
206+
assert.deepEqual(result.removed, [first, second]);
207+
assert.deepEqual(result.moved, []);
208+
// The destination is a key the baseline never had, so it is still reported
209+
// as added alongside the removal that fails the gate.
210+
assert.deepEqual(result.added, [destination]);
211+
assert.equal(result.failures.length, 1);
212+
});
213+
214+
// The contested-destination rule holds even when the arriving digest matches
215+
// one of the sources and an ack sits at the destination: the other source's
216+
// removal still cannot be covered.
217+
test('a contested destination fails even when acked at the destination', () => {
218+
const first = 'src/a.ts#sendJson';
219+
const second = 'src/b.ts#sendJson';
220+
const destination = 'src/c.ts#sendJson';
221+
const released = ledger({ declarations: { [first]: 'sha256:aaa', [second]: 'sha256:old' } });
222+
const current = ledger({
223+
declarations: { [destination]: 'sha256:new' },
224+
compatibleChanges: [
225+
{ declaration: destination, digest: 'sha256:new', rationale: 'One of them moved here.' },
226+
],
227+
});
228+
const result = compareWireLedgers({
229+
baselineTag: 'v0.20.6',
230+
released,
231+
current,
232+
digests: new Map(Object.entries({ [destination]: 'sha256:new' })),
233+
});
234+
assert.deepEqual(result.removed, [first, second]);
235+
assert.deepEqual(result.moved, []);
236+
assert.equal(result.failures.length, 1);
237+
assert.match(result.failures[0]!, /an ack cannot cover it/);
238+
});
239+
115240
test('a newly added wire declaration is additive and needs nothing', () => {
116241
const added = 'packages/kernel/src/contracts.ts#NewEnvelope';
117242
const current = ledger({

scripts/wire-compat/model.ts

Lines changed: 102 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,16 @@ export type WireComparison = {
2929
changed: readonly string[];
3030
/** Declarations the baseline had and the current wire surface does not. */
3131
removed: readonly string[];
32-
/** Declarations added since the baseline; additive, so never a failure. */
32+
/**
33+
* Baseline declarations that left their path but were re-declared unchanged
34+
* (same name, same digest) at a single new path that no other left
35+
* declaration also claims: a file move, never a failure.
36+
*/
37+
moved: readonly string[];
38+
/**
39+
* Current keys the baseline never had (pure-move destinations excepted —
40+
* their source is reported in `moved` instead). Additive, so never a failure.
41+
*/
3342
added: readonly string[];
3443
/** Whether the protocol version advanced since the baseline. */
3544
bumped: boolean;
@@ -43,12 +52,20 @@ export function compareWireLedgers(input: WireComparisonInput): WireComparison {
4352

4453
const changed: string[] = [];
4554
const removed: string[] = [];
55+
const moved: string[] = [];
56+
const movedDestinations = new Set<string>();
57+
const moveDestination = soleMoveDestinations(released.declarations, digests);
4658
for (const [key, releasedDigest] of Object.entries(released.declarations)) {
47-
const digest = digests.get(key);
48-
if (digest === undefined) removed.push(key);
49-
else if (digest !== releasedDigest) changed.push(key);
59+
const fate = baselineKeyFate(key, releasedDigest, digests, moveDestination);
60+
if (fate.kind === 'changed') changed.push(fate.reportKey);
61+
else if (fate.kind === 'moved') {
62+
moved.push(key);
63+
movedDestinations.add(fate.destination);
64+
} else if (fate.kind === 'removed') removed.push(key);
5065
}
51-
const added = Object.keys(current.declarations).filter((key) => !(key in released.declarations));
66+
const added = Object.keys(current.declarations).filter(
67+
(key) => !(key in released.declarations) && !movedDestinations.has(key),
68+
);
5269

5370
const failures: string[] = [];
5471
const stillAt = `still ${current.protocolVersion}`;
@@ -85,5 +102,84 @@ export function compareWireLedgers(input: WireComparisonInput): WireComparison {
85102
}
86103
}
87104

88-
return { changed, removed, added, bumped, failures };
105+
return { changed, removed, moved, added, bumped, failures };
106+
}
107+
108+
type BaselineFate =
109+
| { kind: 'unchanged' }
110+
| { kind: 'changed'; reportKey: string }
111+
| { kind: 'moved'; destination: string }
112+
| { kind: 'removed' };
113+
114+
/** What a baseline declaration became in the current surface. */
115+
function baselineKeyFate(
116+
key: string,
117+
releasedDigest: string,
118+
digests: ReadonlyMap<string, string>,
119+
moveDestination: ReadonlyMap<string, string>,
120+
): BaselineFate {
121+
const digest = digests.get(key);
122+
if (digest !== undefined) {
123+
return digest === releasedDigest ? { kind: 'unchanged' } : { kind: 'changed', reportKey: key };
124+
}
125+
const destination = moveDestination.get(key);
126+
if (destination === undefined) return { kind: 'removed' };
127+
return digests.get(destination) === releasedDigest
128+
? { kind: 'moved', destination }
129+
: { kind: 'changed', reportKey: destination };
130+
}
131+
132+
/**
133+
* Displaced baseline declarations mapped to the single new path they may have
134+
* moved to — or nothing when the move cannot be identified.
135+
*
136+
* A same-name re-declaration at exactly one new path is a file move, which a
137+
* released peer still parses. One destination cannot be two declarations'
138+
* moves, though: when two same-name baseline declarations left their paths and
139+
* only one same-name new path exists, the other declaration's loss is real and
140+
* only a bump covers it, so the contested destination resolves to removals. A
141+
* same-name re-declaration whose digest MOVED is a change at the destination,
142+
* ackable (digest-pinned, rationale required) rather than bump-forcing:
143+
* textually it is indistinguishable from a removal plus a new same-named
144+
* declaration, and that reading gets the ack escape hatch. Candidate paths are
145+
* limited to ones absent from the baseline: names are not unique across files
146+
* (two files both declare `sendJson`), and a name a baseline declaration still
147+
* owns at its own path cannot identify a move.
148+
*/
149+
function soleMoveDestinations(
150+
releasedDeclarations: Record<string, string>,
151+
digests: ReadonlyMap<string, string>,
152+
): ReadonlyMap<string, string> {
153+
const releasedKeys = new Set(Object.keys(releasedDeclarations));
154+
const candidates = new Map<string, readonly string[]>();
155+
for (const key of releasedKeys) {
156+
if (digests.has(key)) continue;
157+
const name = declarationName(key);
158+
candidates.set(
159+
key,
160+
[...digests.keys()].filter(
161+
(candidate) => declarationName(candidate) === name && !releasedKeys.has(candidate),
162+
),
163+
);
164+
}
165+
const claimCount = new Map<string, number>();
166+
for (const matches of candidates.values()) {
167+
if (matches.length === 1) {
168+
const destination = matches[0]!;
169+
claimCount.set(destination, (claimCount.get(destination) ?? 0) + 1);
170+
}
171+
}
172+
const sole = new Map<string, string>();
173+
for (const [key, matches] of candidates) {
174+
if (matches.length === 1 && claimCount.get(matches[0]!) === 1) {
175+
sole.set(key, matches[0]!);
176+
}
177+
}
178+
return sole;
179+
}
180+
181+
/** The declaration name in a `<file>#<name>` key. */
182+
function declarationName(key: string): string {
183+
const separator = key.lastIndexOf('#');
184+
return separator >= 0 ? key.slice(separator + 1) : key;
89185
}

scripts/wire-compat/run.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,5 +88,5 @@ process.stdout.write(
8888
`Daemon RPC wire surface checked against ${baseline.tag} ` +
8989
`(protocol ${result.bumped ? 'bumped' : 'unchanged'}): ${WIRE_DECLARATIONS.length} ` +
9090
`declarations, ${result.changed.length} changed, ${result.removed.length} removed, ` +
91-
`${result.added.length} added.\n`,
91+
`${result.added.length} added, ${result.moved.length} moved.\n`,
9292
);

0 commit comments

Comments
 (0)