Skip to content

refactor(shell-quote): one implementation, reached through the runner host port - #2595

Merged
thymikee merged 1 commit into
mainfrom
refactor/shell-quote-dedupe-2525
Sep 14, 2026
Merged

thymikee merged 1 commit into
mainfrom
refactor/shell-quote-dedupe-2525

Conversation

@thymikee

Copy link
Copy Markdown
Member

Summary

Closes #2525. Two byte-identical shellQuote bodies went away, leaving
packages/host-kit/src/internal/shell-quote.ts as the only implementation. 5 files, no behavior
change and no new module.

  • platform-android/perf-native-artifacts.ts: the exported copy is deleted and both call sites,
    plus perf-native-simpleperf.ts, import shellQuote from @agent-device/host-kit/command
    the subpath three sibling Android modules already use for shellQuoteIfNeeded.
  • runner/runner-lease.ts: goes through the AppleRunnerHost port (runner/host.ts, bound in
    core/runner-host.ts), not a runner-local const, because R77 (apple-runner-host-port) forbids a
    value import of host-kit from runner/** and runner/ sits in the eager closure of seven Apple
    façades.

grep -rn "replaceAll(\"'\"" packages src now returns the canonical implementation plus
packages/xml/src/internal/entities.ts, which escapes ' for XML and was never a shell quoter.

Validation

Tested at 37b1eca498. pnpm check:affected --run passed (format, lint, typecheck, layering,
fallow, 420 related test files / 2,899 tests), so no device run applies. runner-session.test.ts
already pins the quoted hint text AGENT_DEVICE_STATE_DIR='/tmp/agent-device-owner' for the Apple
path, and it passes unchanged; shell-quote.test.ts pins the canonical output. GitHub stays
authoritative for coverage and provider integration.

First gate run failed on runner-client.test.ts :: ensureXctestrunArtifact aborts only the disconnected request build (5.0s timeout). Per docs/agents/testing.md it was host contention:
that test passes alone in 113ms at the same commit and the full rerun above is green.

… host port

Closes #2525. The Android perf copy is a direct import swap to
@agent-device/host-kit/command, which platform-android already uses for
shellQuoteIfNeeded. The apple runner copy goes through AppleRunnerHost
because R77 (apple-runner-host-port) forbids a value import of host-kit from
runner/**, and runner/ sits in the eager closure of seven Apple facades.
@github-actions

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
Installed (including dependencies) 4.53 MB 4.53 MB -43 B
Package (unpacked) 4.53 MB 4.53 MB -43 B
Package (download) 1.34 MB 1.34 MB -32 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 27.8 ms 28.0 ms +0.1 ms
CLI --help 78.7 ms 79.6 ms +1.0 ms

@thymikee

Copy link
Copy Markdown
Member Author

Looks good at 37b1eca. Both deleted copies match host-kit's shellQuote exactly, AppleRunnerHost has one implementer and it binds the new member, and nothing still imports the removed Android export.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Sep 14, 2026
@thymikee

Copy link
Copy Markdown
Member Author

The Smoke Tests failure at 37b1eca is the native runner test testAlertAcceptDoesNotActivateAReplacementWithASharedButton (job). This change does not touch the runner or alert handling, so it looks unrelated; a rerun should confirm.

@thymikee
thymikee merged commit 69667b1 into main Sep 14, 2026
17 of 18 checks passed
@thymikee
thymikee deleted the refactor/shell-quote-dedupe-2525 branch September 14, 2026 15:36
@github-actions

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-09-14 15:37 UTC

@thymikee

Copy link
Copy Markdown
Member Author

Rerun confirms it. The failed iOS job on the same head 37b1eca is green: Smoke Tests passed, testAlertAcceptDoesNotActivateAReplacementWithASharedButton among them, and every other Smoke Tests lane is green too. Nothing was changed for this.

Worth noting for whoever owns the runner suite: #2600, a TypeScript-only change, failed Smoke Tests in the same file one test over — testAlertCannotProveAnIdenticalReplacementAndDoesNotActivateIt at RunnerTests+AlertObservationTests.swift:75, twice with ("First actions: 0; replacement actions: 0") is not equal to ("First actions: 1; replacement actions: 0"). Two unrelated branches landing in adjacent tests of one alert-observation file, both reporting an action log that recorded nothing, reads like the fixture's own observation window rather than either change. Happy to file it as a flake issue if nobody else is on it.

thymikee added a commit that referenced this pull request Sep 14, 2026
…tle-on-exit-2522

* origin/main:
  perf(scroll-until): answer every candidate from one visibility index (#2596)
  refactor(daemon): resolve device interactors through a composed capability (#2593)
  refactor(daemon): stop the client value-importing daemon internals (#2559) (#2594)
  refactor(shell-quote): one implementation, reached through the runner host port (#2595)
  docs(agents): delete the file-size tiers nobody enforces (#2597)
  fix(daemon): key the implicit session by workspace and platform (#2587)
  refactor(cli-schema): orient the schema layer above commands (#2543 step 3) (#2590)
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.

refactor: one shellQuote implementation instead of three

1 participant