Skip to content

Commit 45fa091

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/review-issues-1431-1429-xpehmi
2 parents 4c3e3c1 + c2c8154 commit 45fa091

30 files changed

Lines changed: 2439 additions & 6 deletions

.github/workflows/ci.yml

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,30 @@ jobs:
257257
- name: Verify corpus entries against their released blobs
258258
run: pnpm check:replay-compat
259259

260+
released-surface-compat:
261+
# The daemon RPC wire ledger (#1432) is compared against the ledger as it
262+
# stood at the last RELEASED tag, which only a full-history checkout can
263+
# read. Same split as the replay-compat corpus above: the shallow unit lane
264+
# holds the ledger to its source, this job holds it to the last release.
265+
name: Released-Surface Compatibility
266+
runs-on: ubuntu-latest
267+
timeout-minutes: 10
268+
steps:
269+
- name: Checkout
270+
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
271+
with:
272+
fetch-depth: 0
273+
fetch-tags: true
274+
275+
- name: Setup toolchain
276+
uses: ./.github/actions/setup-node-pnpm
277+
278+
- name: Verify the wire-compat rules
279+
run: pnpm check:daemon-wire-compat:test
280+
281+
- name: Compare the daemon RPC wire surface against the last released tag
282+
run: pnpm check:daemon-wire-compat
283+
260284
coverage:
261285
# Runs the full unit + provider-integration suites under coverage with
262286
# thresholds, so a separate unit-tests job would rerun the same tests.

AGENTS.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,11 @@ prose in this repo, including this file.
3737
pre-fix code (revert, run, quote the failing number), a moved test against its gates (planted type
3838
error, discovered-count delta), a structural gate against a planted violation. Three vacuous
3939
regression tests shipped in one day before this rule; review caught all three.
40+
- Regression tests are evidence, not the default repair. First make the error class impossible at
41+
the owning interface with types, a registry, or one construction path; then add the smallest
42+
interface-level regression that proves the invariant. If a custom guard starts reconstructing a
43+
compiler, schema, or scope model—or needs a second omission patch—redesign it around the source of
44+
truth instead of adding another exception.
4045
- Typed signals over message sniffing: key on structured details (`details.timeoutMs`, reason codes),
4146
never on error text. Remaining sniffs are owned debt with in-code rationale — do not copy them.
4247
- Snapshot output is the token budget. Never add per-node bytes to the tree; response-level metadata
@@ -124,7 +129,6 @@ the new thing — never to suppress or allowlist it.
124129
redundant tests, stale helpers/fixtures, and duplication the change made unnecessary.
125130
- Name durable module concepts with `CONTEXT.md` vocabulary. Do not coin parallel names across docs,
126131
tests, and code.
127-
128132
Module size is about agent context safety, and the unit is questions, not lines: a file should answer
129133
one question so `rg` → read-whole-file stays one cheap bounded read.
130134

CONTRIBUTING.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,18 @@ when a running daemon needs to pick up a new TypeScript build.
7272
`pnpm package:npm` is a release guard, not a routine development command. Use the specific commands
7373
above while iterating.
7474

75+
### Released-surface baselines roll forward on publish
76+
77+
Compatibility gates baseline against the last **released tag**, not against `main`, so publishing is
78+
what advances them — there is no separate baseline-refresh step and no regenerate command. Tagging a
79+
release makes that commit's `test/wire-compat/ledger.json` the new baseline for
80+
`pnpm check:daemon-wire-compat`, and its `.ad` corpus tags the new ceiling for
81+
`pnpm check:replay-compat`. The practical consequence for a normal PR: wire churn *within* an
82+
unreleased branch is free, and only the net change since the last publish has to carry a
83+
`DAEMON_RPC_PROTOCOL_VERSION` bump or a `compatibleChanges` acknowledgment. After a release that
84+
bumped the protocol version, the acknowledgments accumulated against the previous one no longer
85+
match any current digest and are dropped — git history keeps the audit trail.
86+
7587
## Validate a change
7688

7789
Use the smallest trustworthy loop while editing:

docs/adr/0006-daemon-rpc-protocol-version.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,15 @@ Protocol-breaking changes must update `DAEMON_RPC_PROTOCOL_VERSION`, tests that
6161
metadata, and at least one remote-client regression test that proves mismatched protocols fail before
6262
command RPC.
6363

64+
The bump rules above are enforced rather than remembered (#1432). `test/wire-compat/surface.ts`
65+
declares the wire surface grouped by the bullets in this decision, and `test/wire-compat/ledger.json`
66+
records what each declaration hashes to and at which protocol version. The unit lane fails when a
67+
declaration's shape moves without the ledger following, and `pnpm check:daemon-wire-compat` compares
68+
the ledger against the last released tag: drift since that release must carry either a bump or a
69+
`compatibleChanges` acknowledgment invoking the additive list above. Where a bullet is only partly
70+
digestible — the `/health` and `/rpc` path literals inside `http-server.ts` — the manifest records
71+
the gap and its reason instead of implying coverage. See `test/wire-compat/README.md`.
72+
6473
Legacy remote daemons without `rpcProtocolVersion` remain reachable. This keeps the first release of
6574
the proxy compatible with older HTTP daemons, but it means absence of the marker is not proof of
6675
compatibility.

docs/agents/pull-requests.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ asked or when the work is intentionally incomplete.
3838
validation does not apply instead of writing a command checklist.
3939
- Call out real tradeoffs, known gaps, and follow-ups; omit boilerplate when there are none.
4040
- Note touched-file count and whether scope expanded beyond the initial command family.
41-
4241
## Reviewing
4342

4443
- Review against the linked issue, not only the diff. State the issue's motivating behavior and
@@ -52,6 +51,10 @@ asked or when the work is intentionally incomplete.
5251
backend. Tests that mock away the router, or exercise only a helper, do not prove the shipped path.
5352
- For each key regression test, identify what deletion or revert would make it fail. If reverting the
5453
implementation still passes, the test is vacuous.
54+
- For recurring failures, prefer a design that makes the class impossible at the owning interface;
55+
keep one small regression as evidence rather than enumerating examples. If a custom guard needs
56+
repeated exceptions or reconstructs compiler/schema behavior, move the invariant to its source of
57+
truth instead of extending the guard.
5558
- Check for hidden behavior changes separately from intended refactors: output shape,
5659
warning/error propagation, artifact paths, fallback/retry tiers.
5760
- Verify tests cover the issue's motivating failure, not just the new abstraction. Prefer

docs/agents/testing.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ The mapping it encodes, for when you need to run a gate directly or reason about
4040
| CLI help/guidance (`src/cli/parser/cli-help.ts`, `src/cli-schema/`) | `pnpm exec vitest run src/cli/parser/__tests__ src/cli-schema/command-schema-guards.test.ts scripts/__tests__` — the `scripts/__tests__` gates enforce help-topic benchmark coverage and pin the bench's quoted CLI samples to the real renderers |
4141
| Help benchmark cases (`scripts/help-conformance-*.mjs`) | `pnpm exec vitest run scripts/__tests__` (deterministic gates); model-backed: `pnpm bench:help-conformance` (paid LLM calls, local only) |
4242
| `.ad` grammar (`src/replay/script.ts`, gesture arity, replay vars) | `pnpm exec vitest run --project unit-core test/replay-compat` — the frozen replay-compat corpus asserts which released script surfaces still parse; a flipped verdict is edited in `test/replay-compat/manifest.ts`, never in the script. Adding or re-pinning a corpus entry also runs `pnpm check:replay-compat`, which re-derives each entry from its release tag in git history |
43+
| Daemon RPC wire surface (the declarations listed in `test/wire-compat/surface.ts` — JSON-RPC envelope, request/response/error/artifact/progress framing, `/health` payload, HTTP auth headers) | `pnpm exec vitest run --project unit-core test/wire-compat` holds the ledger to its source and prints the digest to paste; `pnpm check:daemon-wire-compat` compares it against the last released tag and requires a `DAEMON_RPC_PROTOCOL_VERSION` bump or a `compatibleChanges` ack for the drift. Read ADR 0006 to decide which; `test/wire-compat/README.md` walks both |
4344
| Anything in `src/`, `test/` | `pnpm format` (`skills/` is Markdown-only guidance: oxfmt ignores `**/*.md`, and the affected-check selector classifies it docs-only) |
4445
| Workspace package source (`packages/*/src/**`) | Root format/lint/typecheck plus layering (R11 package-boundaries); Vitest resolves affected tests through the module graph; package manifests/tsconfigs fail open to the full set |
4546
| A decision kernel or its tests (`packages/kernel/src/errors.ts`, `src/daemon/ref-frame.ts`, `src/commands/interaction/runtime/settle.ts`, `src/utils/scroll-edge-state.ts`, `packages/selectors/src/`) | `pnpm mutation:affected --base origin/main` (minutes; GitHub runs it per PR — see the mutation ratchet section) |

package.json

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,8 @@
140140
"check:package": "node --experimental-strip-types scripts/check-package.ts",
141141
"check:command-docs": "vitest run --project unit-core src/__tests__/command-doc-coverage.test.ts",
142142
"check:replay-compat": "node --experimental-strip-types scripts/check-replay-compat-provenance.ts",
143+
"check:daemon-wire-compat": "node --experimental-strip-types scripts/wire-compat/run.ts",
144+
"check:daemon-wire-compat:test": "node --experimental-strip-types scripts/node-test-tmpdir.ts --experimental-strip-types --test scripts/wire-compat/model.test.ts",
143145
"check:tmpdir-leaks": "node --experimental-strip-types scripts/check-tmpdir-leaks.ts",
144146
"check:tmpdir-leaks:test": "node --experimental-strip-types scripts/node-test-tmpdir.ts --experimental-strip-types --test scripts/check-tmpdir-leaks-model.test.ts scripts/vitest-tmpdir-global-setup.test.ts scripts/node-test-tmpdir.test.ts scripts/swift-toolchain-tmpdir.test.ts",
145147
"check:freerange": "fr",
@@ -196,7 +198,8 @@
196198
"!android/ime-helper/dist/*.idsig",
197199
"!android/ime-helper/README.md",
198200
"linux/atspi-dump.py",
199-
"skills",
201+
"skills/agent-device",
202+
"skills/dogfood",
200203
"server.json",
201204
"smithery.yaml",
202205
"README.md",
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
import assert from 'node:assert/strict';
2+
import { readFile } from 'node:fs/promises';
3+
import { join } from 'node:path';
4+
import { describe, test } from 'vitest';
5+
6+
const ROOT = join(import.meta.dirname, '..', '..');
7+
8+
type Contract = {
9+
id: string;
10+
requiredText: string;
11+
};
12+
13+
const SKILLS = [
14+
{
15+
name: 'iOS Simulator',
16+
path: join(ROOT, 'skills', 'ios-simulator', 'SKILL.md'),
17+
contracts: [
18+
{
19+
id: 'foreground platform open',
20+
requiredText: 'agent-device open <app-or-bundle-id> --platform ios --foreground',
21+
},
22+
{
23+
id: 'initial interactive snapshot',
24+
requiredText: '`open` returns the initial interactive snapshot.',
25+
},
26+
{ id: 'current ref or selector', requiredText: 'Use its current refs or a selector.' },
27+
{ id: 'settled planned actions', requiredText: 'agent-device press @eN --settle' },
28+
{ id: 'type settle exception', requiredText: '`type` never takes `--settle`;' },
29+
{
30+
id: 'end-state verification',
31+
requiredText: 'Verify the end state with a selector or exact text, then close:',
32+
},
33+
{ id: 'session close', requiredText: 'agent-device close' },
34+
] satisfies Contract[],
35+
},
36+
{
37+
name: 'Android Emulator',
38+
path: join(ROOT, 'skills', 'android-emulator', 'SKILL.md'),
39+
contracts: [
40+
{
41+
id: 'foreground platform open',
42+
requiredText: 'agent-device open <app-or-package-id> --platform android --foreground',
43+
},
44+
{
45+
id: 'initial interactive snapshot',
46+
requiredText: '`open` returns the initial interactive snapshot.',
47+
},
48+
{ id: 'current ref or selector', requiredText: 'Use its current refs or a selector.' },
49+
{ id: 'settled planned actions', requiredText: 'agent-device press @eN --settle' },
50+
{ id: 'type settle exception', requiredText: '`type` never takes `--settle`;' },
51+
{
52+
id: 'end-state verification',
53+
requiredText: 'Verify the end state with a selector or exact text, then close:',
54+
},
55+
{ id: 'session close', requiredText: 'agent-device close' },
56+
] satisfies Contract[],
57+
},
58+
] as const;
59+
60+
function assertSkillContract(content: string, contract: Contract): void {
61+
assert.ok(content.includes(contract.requiredText), `missing ${contract.id} guidance`);
62+
}
63+
64+
describe('simulator skill contracts', () => {
65+
for (const skill of SKILLS) {
66+
test(`${skill.name} keeps its required workflow guidance`, async () => {
67+
const content = await readFile(skill.path, 'utf8');
68+
for (const contract of skill.contracts) assertSkillContract(content, contract);
69+
});
70+
71+
for (const contract of skill.contracts) {
72+
test(`${skill.name} rejects a missing ${contract.id} contract`, async () => {
73+
const content = await readFile(skill.path, 'utf8');
74+
const broken = content.replace(contract.requiredText, '[removed]');
75+
assert.notEqual(broken, content, `test fixture must remove ${contract.id} guidance`);
76+
assert.throws(
77+
() => assertSkillContract(broken, contract),
78+
new RegExp(`missing ${contract.id} guidance`),
79+
);
80+
});
81+
}
82+
}
83+
});

scripts/check-affected/checks.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,15 @@ export const CHECK_CATALOG: readonly CheckSpec[] = [
167167
ciJobs: ['Replay-Compat Provenance'],
168168
localRunnable: true,
169169
},
170+
{
171+
id: 'daemon-wire-compat',
172+
label: 'Daemon RPC wire surface vs. last released tag',
173+
kind: { type: 'script', script: 'check:daemon-wire-compat' },
174+
// Same shape as replay-compat: the released ledger is only readable from a
175+
// full-history checkout, so this cannot live in the shallow unit lane.
176+
ciJobs: ['Released-Surface Compatibility'],
177+
localRunnable: true,
178+
},
170179
];
171180

172181
export function getCheckSpec(id: CheckId): CheckSpec {

scripts/check-affected/model.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,7 @@ test('every catalog command resolves against package scripts', () => {
224224
'build:macos-helper': 'x',
225225
'test:smoke:web': 'x',
226226
'check:replay-compat': 'x',
227+
'check:daemon-wire-compat': 'x',
227228
};
228229
for (const spec of CHECK_CATALOG) {
229230
const command = resolveCommand(spec, scripts, 'origin/main');

0 commit comments

Comments
 (0)