Skip to content

refactor: tidy three post-extraction seams - #1551

Merged
thymikee merged 3 commits into
mainfrom
claude/cleanup-extraction-seams-1b3ae8
Aug 2, 2026
Merged

refactor: tidy three post-extraction seams#1551
thymikee merged 3 commits into
mainfrom
claude/cleanup-extraction-seams-1b3ae8

Conversation

@thymikee

@thymikee thymikee commented Aug 2, 2026

Copy link
Copy Markdown
Member

Three small mechanical cleanups from a 2026-08-01 audit of the #1478 extraction seams. Behavior-preserving throughout: two are pure import re-pointing, the third names an existing predicate.

1. REPLAY_VAR_KEY_RE shim (5f1be5c0e)

src/replay/vars.ts re-exported the constant from @agent-device/ad-script for exactly one consumer, src/replay/recorded-input.ts. That consumer now imports from the package directly and the re-export line is gone (vars.ts keeps its own import; it uses the constant internally). This also makes packages/ad-script/src/internal/script.ts's doc comment true — it already claimed recorded-input.ts imports from the package.

2. Target-annotation shape imports (4e3e75c4b)

The annotation shape types live in @agent-device/contracts/replay; target-annotation-serde.ts re-exported them and the façade re-exported that, so 21 files reached the shape through the detour. Every consumer now imports from contracts directly — the 10 type-only production files from the audit, the two mixed value+type files (type half split out), root tests, and the package's own tests — and both re-exports are dropped.

Type-only, so nothing changes at runtime. The package.json exports map is untouched, so the R11 assertion in scripts/layering/package-boundaries.test.ts (which pins the subpath list, not the named-export list) still holds as written; pnpm check:layering confirms.

3. isAuthoringArmedSession (42b3cefd3)

kind === 'authoring' && status === 'armed' was spelled out at three handler sites asking the same question (session-close.ts, session-open.ts, session-script-publication.ts). It now has a name in session-script-publication-capability.ts next to isSessionScriptPublished, mirroring how isRepairArmedSession is housed in the repair projection, and the three sites route through it.

abortAuthoring's own guard keeps its inline check — that one is the transition's legality test, not a session-level read. The helper is read-only, so scripts/layering/session-state.ts's writer-ownership registry is unaffected (R7 still reports 22 writer-owned fields / 28 owner claims).

Verification

pnpm typecheck && pnpm lint && pnpm format:check && pnpm check:layering
npx vitest run scripts/layering src/daemon packages/ad-script src/replay

All green — 215 files / 1883 tests passed; layering guard OK.

…ectly

vars.ts re-exported the constant for a single consumer, recorded-input.ts.
Point that consumer at @agent-device/ad-script and drop the shim, which also
makes script.ts's doc comment ("recorded-input.ts imports it from this
package") true.
…s directly

The annotation shape types (TargetAncestryEntry, TargetAnnotationV1,
TargetScrollRegion, TargetVerification) live in @agent-device/contracts/replay;
the codec package re-exported them, and 21 files reached the shape through that
detour. Point every consumer — root src, root tests, and the package's own
tests — at contracts, then drop the re-export from the serde module and the
façade. Type-only, so nothing changes at runtime.

The package.json exports map is unchanged, so the R11 boundary assertion in
scripts/layering/package-boundaries.test.ts still holds as written.
`kind === 'authoring' && status === 'armed'` was spelled out at three handler
sites that all ask the same question. Give it a name next to
isSessionScriptPublished, mirroring how isRepairArmedSession is housed in the
repair projection, and route the three sites through it.

abortAuthoring's own guard keeps its inline check: that one is the transition's
legality test, not a session-level read.
@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 1.93 MB 1.92 MB -8.5 kB
JS gzip 617.9 kB 615.7 kB -2.3 kB
npm tarball 736.3 kB 733.8 kB -2.5 kB
npm unpacked 2.58 MB 2.57 MB -9.3 kB

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 22.6 ms 22.1 ms -0.6 ms
CLI --help 52.1 ms 46.2 ms -5.9 ms

Top changed chunks:

Chunk Raw diff Gzip diff
dist/src/selector-vocabulary.js -4.2 kB -1.3 kB
dist/src/session.js +1.9 kB +685 B
dist/src/screenshot-result.js -2.1 kB -631 B
dist/src/registry.js -2.6 kB -600 B
dist/src/cli.js -745 B -199 B

@thymikee

thymikee commented Aug 2, 2026

Copy link
Copy Markdown
Member Author

Reviewed exact head 42b3cefd3: no findings. The private type/shim removals align ownership with contracts/ad-script, and the authoring-armed predicate is semantically equivalent across publication states and consistent with ADR 0016. Code review is clean; completed exact-head gates are green, with iOS smoke/native-fingerprint resolution still in progress.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Aug 2, 2026
@thymikee
thymikee merged commit 2e4825e into main Aug 2, 2026
32 of 33 checks passed
@thymikee
thymikee deleted the claude/cleanup-extraction-seams-1b3ae8 branch August 2, 2026 06:41
@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-08-02 06:41 UTC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant