Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
518 changes: 218 additions & 300 deletions AGENTS.md

Large diffs are not rendered by default.

351 changes: 241 additions & 110 deletions CONTEXT.md

Large diffs are not rendered by default.

42 changes: 42 additions & 0 deletions docs/agents/cli-flags.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# Adding a CLI Flag

A new flag touches only the layers that need to understand it. Stop at the layer where it stops
mattering — threading it further is the common failure, not stopping too early.

1. `src/contracts/cli-flags.ts`: add to `CliFlags`; add the definition to the matching
`src/commands/cli-grammar/flag-definitions-*.ts` owner and the relevant group in `flag-groups.ts`
(for example `SNAPSHOT_FLAGS`). Then update the command family metadata/schema that exposes the
flag; find the owner with
`rg -n "<command>|supportedFlags|allowedFlags" src/commands src/cli-schema src/cli/parser`. For
schema-only CLI commands (`cdp`, `auth`, `connect`, `proxy`, `react-devtools`, `web`) the owner is
`SCHEMA_ONLY_CLI_COMMAND_SCHEMAS` in `src/cli-schema/command-overrides.ts`.
2. `src/commands/cli-grammar/*`: read the CLI flag into command input.
3. `src/commands/command-projection.ts` and command-family projection helpers: write the input into
the daemon request only if the flag affects daemon execution.
4. `src/commands/*-command-contracts.ts`: add to the command input schema only if the option should
be available through Node.js or MCP as structured input.
5. `src/client/client-types.ts`: update the public typed client option only when the Node.js
interface exposes it.
6. `src/client/client-normalizers.ts`: update daemon flag normalization only when the request still
needs a public-to-internal translation.
7. `src/daemon/context.ts` and `src/core/dispatch-context.ts`: add the field only when it flows into
platform dispatch.
8. Handler/platform modules: thread the option only after the command surface, grammar, and
projection prove it belongs there.
9. `scripts/integration-progress-model.ts`: classify the flag (device-observable vs
intentionally-outside). The architecture-progress gate fails CI on unclassified public flags.
10. If the flag changes interaction semantics, revisit the affected cells in
`src/contracts/interaction-guarantees.ts` (scope with `appliesTo` when the flag exists only on
some commands).

Command-only flags (like `find --first`) that never reach the platform layer usually stop at
steps 1-3, plus step 9.

## Where CLI help and schema live

- Long help prose: `src/cli/parser/cli-help.ts`. Flag definitions: `src/commands/cli-grammar/`.
- Command-specific usage/flag metadata lives with the command family metadata that owns the command.
- Parser/help *rendering* stays in `src/cli/parser/`; command schema metadata is derived from command
metadata, family declarations, and the schema-only merge path in
`src/cli-schema/command-overrides.ts`. Keep the two separate.
- Locating an owner: `rg -n "helpDescription|summary|supportedFlags|allowedFlags" src/commands src/cli/parser src/cli-schema`.
53 changes: 53 additions & 0 deletions docs/agents/device-verification.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
# Manual Device Verification

Read this before running `agent-device` by hand against a simulator, emulator, or physical device.

## Before the run: defeat staleness

Dev-loop staleness has three layers, and each produces a convincing false negative.

- After changing runtime code reached through `bin/agent-device.mjs` or the daemon: `pnpm build`,
then `pnpm clean:daemon` — the daemon does not self-reload.
- Before any Android verification from source: `pnpm build`, `pnpm build:android`, `pnpm clean:daemon`.
`build:android` refreshes and verifies both bundled Android helper artifacts for the current
package version.
- `shutdown` deliberately HANDS OFF a healthy simulator runner. The adopted runner keeps serving the
old Swift binary until you kill its process or the source fingerprint changes, so "my change did
nothing" measured against an adopted runner is a classic false negative. If Swift runner code
changed, run `pnpm build:xcuitest`.

## Prove the path under test was actually active

- Android: capture `snapshot -i --json` and require `androidSnapshot.backend` to be `android-helper`
with `helperVersion` equal to `package.json`'s version. A stock UIAutomator fallback is not valid
verification unless the fallback itself is the behavior under test.
- For repo-owned `Agent Device Tester` work, `examples/test-app/README.md` is the source of truth for
simulator, physical-device, Metro/dev-client, and app-surface steps. An already-installed
`com.callstack.agentdevicelab` is not sufficient — the README's Metro/dev-build and `snapshot -i`
checks must prove the expected app surface is running.
- For Android RN/Expo/dev-client apps on any local Metro port, `adb reverse tcp:<port> tcp:<port>` is
harmless and should be run before opening the app or URL.

## Session hygiene

Every manually opened session is a resource that must be closed — including exploratory sessions and
failed verification attempts.

- Every `agent-device open` needs a matching `close` with the same `--session`, `--platform`,
`--udid`, and `--state-dir` before the agent finishes.
- Use a purpose-specific session name for experiments, and an isolated `--state-dir` under
`/private/tmp` when you need cleanup isolation beyond the current worktree's default daemon.
- Track opened sessions in working notes; close each one before the final response.
- If `close` is blocked by stale daemon metadata, inspect processes first with
`ps -ax | rg "agent-device|xcodebuild test-without-building"`. Stop only exact stale PIDs belonging
to this verification run, then `pnpm clean:daemon`.
- If cleanup cannot be completed, report the remaining session name, state dir, PIDs, and metadata
paths as a blocker.

## Sandboxed environments

Start the daemon outside the sandbox with escalation. The daemon binds localhost, and sandboxed runs
fail before any product code executes, with `listen EPERM: operation not permitted 127.0.0.1` or
repeated `Failed to start daemon`/metadata cleanup messages. Those are not agent-device regressions —
rerun with escalation. Unit tests, typecheck, lint, and build can stay sandboxed unless they need
devices or listener access.
16 changes: 7 additions & 9 deletions docs/agents/domain.md
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
# Domain Docs

This is a single-context repo.
Single-context repo. Before architecture, diagnosis, TDD, triage, PRD, or roadmap work, read
`CONTEXT.md` for domain vocabulary and the capture-reliability contract, plus the relevant ADRs in
`docs/adr/`.

Before architecture, diagnosis, TDD, triage, PRD, or roadmap work, read:
Use `CONTEXT.md` vocabulary in issue titles, refactor proposals, test names, and architecture notes.
If a proposed change contradicts an ADR, say so explicitly and explain why the decision should be
reopened.

- `CONTEXT.md` for domain vocabulary, test strategy terms, and architecture language.
- Relevant ADRs in `docs/adr/` for accepted architecture decisions.
- This `docs/agents/` directory for issue-tracker and triage-label conventions.
- `docs/agents/web-backend.md` before changing web automation backend setup or diagnostics.
- `docs/agents/testing.md` for maintainer-only test lane notes such as the live web smoke.

Use the vocabulary from `CONTEXT.md` in issue titles, refactor proposals, test names, and architecture notes. If a proposed change contradicts an ADR, call that out explicitly and explain why the decision should be reopened.
`AGENTS.md` routes to the rest of this directory by task type.
60 changes: 60 additions & 0 deletions docs/agents/pull-requests.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
# Pull Requests

## Readiness

- Static gates first: required checks pass, `pnpm check:fallow --base origin/main` is clean when
code-quality/dead-code risk is relevant, CI guards are green, and no conflict markers or unmerged
paths remain.
- A local unit-only run is not CI-green. Use `pnpm test:unit` for the repo unit bundle, or
`vitest run --project unit-core --project subprocess-stub` when invoking Vitest directly. The
**Integration Tests** and **Coverage** jobs run the `provider-integration` project — verify those
green on the actual PR head.
- Device-facing behavior is not merge-ready without real simulator/emulator/device evidence for the
changed path. Fixture-backed tests prove contracts; they do not replace a live run that creates or
observes the artifact/state the feature claims to handle. If live verification is blocked, state
the blocker and the exact command/device needed, and downgrade the PR to residual risk rather than
calling it ready.
- Command-surface changes preserve CLI, Node.js, daemon, MCP, help, docs, and SkillGym coverage
where that surface is affected, without duplicating command contracts across layers.
- Runtime output stays agent-friendly: compact defaults, top offenders first for diagnostics/perf,
bounded arrays in JSON, artifact paths for large raw data, progressive lookup for deeper detail.
- Close every manual `agent-device` session opened during verification
(`docs/agents/device-verification.md`) and report any cleanup that could not be completed.

## PR body

Conventional commit prefixes (`feat:`, `fix:`, `chore:`, `perf:`, `refactor:`, `docs:`, `test:`,
`build:`, `ci:`). No bracketed bot tags like `[codex]`. Ready-for-review by default; draft only when
asked or when the work is intentionally incomplete.

- `## Summary`: user/API behavior, not an implementation file tour. Lead with what changed for
operators, clients, command authors, or platform behavior. A compact before/after helps when it
clarifies the workflow or bug fix. For new or changed public APIs, include 1-3 concrete CLI/Node/MCP
examples a reviewer can scan. `Closes #123` when applicable.
- `## Validation`: meaningful evidence in concise prose — scenario names, manual device/browser
evidence, changed screenshots, CI status, notable failures/retries and their outcome. Avoid command
accounting for routine local gates; name an exact command only when it is unusual, manually
reproducible evidence, or needed to explain a residual risk. For docs-only changes, say why runtime
validation does not apply instead of writing a command checklist.
- Call out real tradeoffs, known gaps, and follow-ups; omit boilerplate when there are none.
- Note touched-file count and whether scope expanded beyond the initial command family.

## Reviewing

- Review against the linked issue, not only the diff. State the issue's motivating behavior and
verify the PR fixes *that*.
- Check relevant ADRs before reviewing architecture, routing, command-surface, platform-boundary,
diagnostics, or testing-strategy changes. An ADR conflict is a review finding unless the PR updates
or supersedes the ADR explicitly.
- Read dependency notes (`Blocked by: ...`, linked PRs, sibling branches) before judging correctness.
A base/sequence problem outranks detail review.
- Trace the real production route from command surface through daemon/request routing to the platform
backend. Tests that mock away the router, or exercise only a helper, do not prove the shipped path.
- For each key regression test, identify what deletion or revert would make it fail. If reverting the
implementation still passes, the test is vacuous.
- Check for hidden behavior changes separately from intended refactors: output shape,
warning/error propagation, artifact paths, fallback/retry tiers.
- Verify tests cover the issue's motivating failure, not just the new abstraction. Prefer
before/after evidence when an issue reports a concrete divergence.
- Green CI is necessary but insufficient for device-facing or routing-sensitive work.
- Check whether the tightening pass removed code/tests the change made obsolete.
55 changes: 53 additions & 2 deletions docs/agents/testing.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,52 @@
# Testing Notes

## Which gates a change needs

Default for code changes: `pnpm check:affected --base origin/main --run`. It derives the gate set
from repository sources of truth, so prefer it over interpreting the table below by hand. GitHub CI
stays authoritative.

The mapping it encodes, for when you need to run a gate directly or reason about coverage:

| Change | Gate |
| --- | --- |
| Any TypeScript | `pnpm typecheck` or `pnpm check:quick` |
| Daemon handler / shared module | `pnpm check:unit` |
| Tooling/config (`package.json`, `tsconfig*.json`, `.oxlintrc.json`, `.oxfmtrc.json`) | `pnpm check:tooling` |
| Platform/device response — anything emitting `platform`/`appleOs` on the wire, or shaping a daemon response | `pnpm test:integration:provider` **and** `pnpm test:coverage` |
| Cross-platform behavior | `pnpm test:integration` |
| iOS runner / Swift | `pnpm build:xcuitest` |
| 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` |
| SkillGym prompts/assertions | `pnpm test:skillgym:case <case-id>` (broad: `pnpm test:skillgym`, filter with `-- --tag fixture-smoke` or `-- --tag skill-guidance`) |
| Anything in `src/`, `test/`, `skills/` | `pnpm format` |

Two traps worth naming:

- The platform/device-response row is the one agents miss. `pnpm check:unit` does **not** exercise the
`provider-integration` project, and that project holds the apple-platform-output leak guard.
Internal `apple` must never reach a command response — project through `publicPlatformString`.
- Fallow CI failures reproduce with `pnpm check:fallow --base origin/main`. Do not estimate
complexity or dead-code impact by hand.

Docs/skills-only and non-TS changes with no behavior impact need no tests. Test-only DI seam CI
failures are enforced by the workflow — do not add optional `typeof` DI params to production code to
satisfy a test.

## Shared test utilities

Before writing a new test, inspect `src/__tests__/test-utils/index.ts`:
`rg -n "export .*make|export .*DEVICE|withMocked" src/__tests__/test-utils`. Import through the
barrel and prefer named shared fixtures over inlining new `DeviceInfo`, `SessionState`, snapshot,
store, or mocked-binary objects. If a helper is missing, add it near the concept it serves and export
it through the barrel.

Keep tests behavioral. Do not assert shapes or cases TypeScript already proves.

Test through public interfaces where practical, and do not add unrelated production exports solely
to make a test easier — widening the public surface for a test is a product change, and the exports
outlive the test that motivated them. If a seam is genuinely missing, add it as a real one rather
than as a test affordance (the workflow separately forbids test-only `typeof` DI params).

## Affected-check selector (`pnpm check:affected`)

`pnpm check:affected --base <ref>` derives which local checks a diff needs, so
Expand Down Expand Up @@ -44,8 +91,12 @@ sides of a rename are classified (a moved file cannot look docs-only by its
destination alone).

Anything the selector cannot classify — unknown, ambiguous, workflow/tooling, or
a change to the selector's own sources (including the `AGENTS.md` Testing
Matrix) — **fails open to the full check set**.
a change to the selector's own sources — **fails open to the full check set**.
That includes this file: the Testing Matrix above is the prose the ownership
rules mirror, so `docs/agents/testing.md` is selector-owning
(`SELECTOR_OWNING_DOCS` in `scripts/check-affected/model.ts`) and outranks the
docs-only short-circuit its path would otherwise take. If the matrix moves
again, move that entry with it.
The plan documents the rule and changed path behind every selected check.

Model and catalog live under `scripts/check-affected/`; the derivation is guarded
Expand Down
7 changes: 4 additions & 3 deletions scripts/check-affected/checks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,10 @@ export const CHECK_CATALOG: readonly CheckSpec[] = [
id: 'skillgym',
label: 'SkillGym command-planning suite',
kind: { type: 'script', script: 'test:skillgym' },
// No GitHub workflow runs SkillGym; per the AGENTS.md testing matrix it is
// a local-only gate (`pnpm test:skillgym`). Keep it locally runnable rather
// than claiming a CI job that does not exist and silently skipping it.
// No GitHub workflow runs SkillGym; per the Testing Matrix in
// docs/agents/testing.md it is a local-only gate (`pnpm test:skillgym`).
// Keep it locally runnable rather than claiming a CI job that does not
// exist and silently skipping it.
ciJobs: [],
localRunnable: true,
},
Expand Down
4 changes: 3 additions & 1 deletion scripts/check-affected/model.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,9 @@ test('workflow/tooling and selector-owning changes fail open', () => {
plan(['scripts/check-affected/model.ts']).failOpenReasons[0]?.rule,
'selector-owning',
);
assert.equal(plan(['AGENTS.md']).failOpenReasons[0]?.rule, 'selector-owning');
// The Testing Matrix lives here; a matrix edit must outrank the docs-only
// short-circuit that its `docs/` path would otherwise take.
assert.equal(plan(['docs/agents/testing.md']).failOpenReasons[0]?.rule, 'selector-owning');
});

test('a fail-open path in a mixed changeset forces the full set', () => {
Expand Down
13 changes: 11 additions & 2 deletions scripts/check-affected/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,17 @@ const ROOT_TOOLING = new Set([
'.npmrc',
]);

// Prose that specifies this selector's own behavior — the Testing Matrix these
// ownership rules mirror. It is docs by path, but editing it can invalidate the
// derivation below, and the selector cannot tell whether it did. Keep this in
// sync when the matrix moves; the docs short-circuit would otherwise treat it as
// inert Markdown.
const SELECTOR_OWNING_DOCS = new Set(['docs/agents/testing.md']);

function isSelectorOwning(file: string): boolean {
return (
file === 'AGENTS.md' || (file.startsWith('scripts/check-affected/') && !file.endsWith('.md'))
SELECTOR_OWNING_DOCS.has(file) ||
(file.startsWith('scripts/check-affected/') && !file.endsWith('.md'))
);
}

Expand Down Expand Up @@ -221,7 +229,8 @@ const nodeIntegrationOwnership: OwnershipRule = ({ file }) =>
: [];

// SkillGym validates skill guidance (`skills/`) and owns its harness
// (`test/skillgym/`); AGENTS.md routes skill-prompt/assertion changes here.
// (`test/skillgym/`); the Testing Matrix in docs/agents/testing.md routes
// skill-prompt/assertion changes here.
const skillgymOwnership: OwnershipRule = ({ file, underSkills }) =>
underSkills || file.startsWith('test/skillgym/')
? [
Expand Down
Loading