Skip to content

chore(layering): runner modules reach host-kit only through the runner host port - #2470

Merged
thymikee merged 1 commit into
mainfrom
claude/runner-host-port-rule
Sep 10, 2026
Merged

chore(layering): runner modules reach host-kit only through the runner host port#2470
thymikee merged 1 commit into
mainfrom
claude/runner-host-port-rule

Conversation

@thymikee

Copy link
Copy Markdown
Member

Rule

packages/platform-apple/src/runner/** may not VALUE-import @agent-device/host-kit/* directly. The host-capability port at runner/host.ts is the only door: a host-kit symbol the runner needs is added to AppleRunnerHost there and bound to the real implementation in core/runner-host.ts. A type-only import (import type … from '@agent-device/host-kit/…') is exempt everywhere, since it evaluates nothing. A pure constant that both the runner and another package need does not go in host-kit either — it belongs in a runner module already inside every façade closure (e.g. runner/apple-runner-platform.ts), imported directly from there.

This lands as:

  • One paragraph added to the header doc of runner/host.ts stating the rule and its reason.
  • A new layering rule, scripts/layering/apple-runner-host-port-policy.ts (R77 apple-runner-host-port), registered in scripts/layering/check.ts. Violation message: "imports '<spec>' directly. Reach host-kit through the runner host port (runner/host.ts, bound in core/runner-host.ts); a direct import grows the Apple façade eager closures (eager-closure-budgets)."
  • scripts/layering/apple-runner-host-port-policy.test.ts: a planted violation (every import form — named, namespace, dynamic, re-export, side-effect) goes red; a type-only import (in the runner subtree and on host.ts itself) stays green; imports outside the runner subtree and non-host-kit runner imports are untouched.

Reason

runner/** sits in the eager import closure of seven Apple façade entries (app-lifecycle-facade.ts, app-resolution-facade.ts, doctor-facade.ts, perf-facade.ts, physical-device-facade.ts, runner-operations-facade.ts, runner/index.ts), each held at a fixed size (no growth against the merge-base) by scripts/__tests__/eager-closure-budgets.ts. A direct @agent-device/host-kit/* value import from a runner module adds every module on its own import path to all seven closures at once.

Measured on #2423: a candidate direct @agent-device/host-kit/command import from runner-cache-metadata.ts added 5 modules to runner/index.ts's eager closure — 13 → 18 — per the PR's own accounting ("a static edge from that file to host-kit adds five modules to the packages/platform-apple/src/runner/index.ts eager closure, which scripts/__tests__/eager-closure-budgets.test.ts holds at 13"). The PR spent two review rounds rediscovering the port requirement before the constant was routed back through runner/host.ts / core/runner-host.ts (round 1: a dependency-free module holding the constant was still +1 on all seven entries; the accepted fix in the final structural round moved the constant into runner/apple-runner-platform.ts, a module already inside every one of the seven closures, specifically because a new host-kit-owned module was unlandable under the gate). Nothing before this PR wrote the rule down, so nothing enforced it — this closes that gap.

Planted-violation output

Appending import { runCmdSync } from '@agent-device/host-kit/command'; to packages/platform-apple/src/runner/apple-runner-platform.ts and running pnpm check:layering:

Layering guard: 1 violation(s)

  [R77 apple-runner-host-port] 1 violation(s):
    packages/platform-apple/src/runner/apple-runner-platform.ts:205 — imports '@agent-device/host-kit/command' directly. Reach host-kit through the runner host port (runner/host.ts, bound in core/runner-host.ts); a direct import grows the Apple facade eager closures (eager-closure-budgets).

Reverted before committing; the rule ran clean against main first (0 violations across the tracked runner subtree) before this PR wired it into check:layering, so nothing pre-existing needed a baseline table.

Verification

  • pnpm check:layering — OK, 226/226 gate tests pass (includes the two new rule tests below).
  • pnpm typecheck — fails with a pre-existing, unrelated error in src/commands/cli-grammar/flag-definitions-action.ts (projectConfig missing on the until flag) that reproduces identically on origin/main at this branch's base commit (6d08de4609, #2436), before any change in this PR. Not touched here.
  • pnpm lint — clean.
  • node --experimental-strip-types --test scripts/layering/apple-runner-host-port-policy.test.ts — 5/5 pass.
  • vitest run scripts/__tests__/eager-closure-budgets.test.ts — unchanged, 581/581 pass.

Motivated by the review rounds on #2423.

@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
Installed (including dependencies) 4.54 MB 4.54 MB 0 B
Package (unpacked) 4.54 MB 4.54 MB 0 B
Package (download) 1.35 MB 1.35 MB -7 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 27.7 ms 29.0 ms +1.3 ms
CLI --help 79.9 ms 81.0 ms +1.1 ms

@thymikee

Copy link
Copy Markdown
Member Author

No code findings at 098f795. The rule reuses the existing import parser, covers the supported value-import forms, and keeps type-only imports exempt. The planted violation demonstrates enforcement through the real layering gate. The failed typecheck, repo guards and integration checks point to the inherited until flag definition, now fixed on main by #2472. Marking ready for human review; CI still needs a run with that fix included.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Sep 10, 2026
…r host port

R77 apple-runner-host-port bans a direct @agent-device/host-kit/* value
import from packages/platform-apple/src/runner/**; the port at runner/host.ts,
bound in core/runner-host.ts, is the only door. runner/** sits in the eager
closure of seven Apple facade entries eager-closure-budgets.ts holds at a
fixed size, so a direct import grows all seven at once (#2423 measured one
candidate import adding 5 modules to runner/index.ts's closure, 13 -> 18,
after two review rounds spent rediscovering this).
@thymikee
thymikee force-pushed the claude/runner-host-port-rule branch from 098f795 to eae5f7a Compare September 10, 2026 16:15
@thymikee
thymikee merged commit fea7ca8 into main Sep 10, 2026
18 checks passed
@thymikee
thymikee deleted the claude/runner-host-port-rule branch September 10, 2026 16:44
@github-actions

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-09-10 16:44 UTC

@thymikee

Copy link
Copy Markdown
Member Author

The reviewed patch is unchanged at eae5f7a after the rebase, and all checks now pass. No code findings or conflicts; ready to merge.

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