chore: remove dead code and tune fallow's dead-code rules#1363
Merged
Conversation
Evaluated knip (webpro-nl/knip) against the fallow setup already in the
repo, cleaned up everything it surfaced, then removed knip again: measured
head-to-head on the same tree, fallow is a strict superset once two
switches it already supports are flipped.
Dead code removed:
- `daemon/artifact-materialization.ts` (224 lines) had no production
caller, only its own test. Removing it exposed that
`downloadArtifactToTempDir` and the whole URL-fetch-with-redirects path
in `artifact-download.ts` were reachable only through it — the live
upload paths use the incoming-request helpers instead. That file goes
348 -> 123 lines. `readZipEntries` then fell out of `artifact-archive.ts`.
- Dead test-helper exports: 12 unused re-exports and 6 needlessly-exported
mocks in `session-test-harness.ts`, dead barrel entries in
`__tests__/test-utils/index.ts`, plus `withMockedXcrun`, `matchesSchema`,
`IOS_FRAME`, `IOS_TAB_FRAME`, `snapshotWithOffscreenContent`.
- `androidSnapshotHelperOutput` was duplicated byte-for-byte in
`provider-scenarios/android-world.ts`; it now imports the shared copy.
- 8 unreferenced type aliases, and 17 redundant type re-export lines in
`client/client-types.ts`. The published `.d.ts` is byte-identical before
and after all 19 files: those types already reach consumers through
`contracts/*` via `CommandResult<...>`, so this is not an API change.
Tooling:
- `.fallowrc.json` gains `includeEntryExports`,
`ignoreExportsUsedInFile: {type, interface}` and `unused-types: warn`.
That combination is what made the findings above visible; the previous
config was quiet mainly because of its own suppression list.
- Dropped 2 now-obsolete `ignoreExports` suppressions, added 3 documented
ones (published `sdk/*` surface, tool-config `default` exports, and the
`AssertTrue<...>` totality guards that exist only to satisfy
`noUnusedLocals`).
`unused-types` stays at `warn`: 72 pre-existing type re-export lines across
27 files remain, tracked separately. Every other fallow detector is at zero.
Size Report
Startup median (7 runs, lower is better):
Top changed chunks:
|
Member
Author
|
Reviewed |
thymikee
added
the
ready-for-human
|
thymikee
added a commit
that referenced
this pull request
Jul 22, 2026
Clears the 72 `unused-types` findings left by #1363 and flips `rules.unused-types` from `warn` to `error` so new ones cannot creep in. Every fallow detector is now at zero. 71 of the findings were re-export lines (`export type { X } from './y.ts'` with no importer of X through that path); one was a direct declaration (`HelpTopicName`, zero references anywhere). Removing them cascaded, which is the point: imports that existed only to feed a re-export became unused (8 of them, caught by `noUnusedLocals`), and clearing those exposed a further chain through `command-projection.ts` -> `batch/index.ts` -> `batch/projection.ts`. Also updated a comment in `batch-policy.ts` that described the `command-surface.ts` re-export path this removes, and dropped two `export type {}` husks left where every name in a block was dead. One finding was NOT what it looked like: fallow flagged `DebugSymbolsOptions` and `DebugSymbolsResult` in `apple/core/debug-symbols/types.ts`, but removing them broke `apple/core/debug-symbols.ts`, which re-exported them from there. Both ends were dead — every real consumer imports from `contracts/debug-symbols.ts` — so the fix was removing the intermediate re-export too, not restoring the leaf. Safety: the published type surface is unchanged. All 11 entry points in package.json#exports export exactly the same names before and after. Two chunk files differ only because `AndroidSnapshotBackendMetadata` relocated between internal code-splitting chunks, which is not a consumer-visible boundary.
thymikee
added a commit
that referenced
this pull request
Jul 22, 2026
…used-types (#1364) * chore: remove dead type re-exports and gate unused-types Clears the 72 `unused-types` findings left by #1363 and flips `rules.unused-types` from `warn` to `error` so new ones cannot creep in. Every fallow detector is now at zero. 71 of the findings were re-export lines (`export type { X } from './y.ts'` with no importer of X through that path); one was a direct declaration (`HelpTopicName`, zero references anywhere). Removing them cascaded, which is the point: imports that existed only to feed a re-export became unused (8 of them, caught by `noUnusedLocals`), and clearing those exposed a further chain through `command-projection.ts` -> `batch/index.ts` -> `batch/projection.ts`. Also updated a comment in `batch-policy.ts` that described the `command-surface.ts` re-export path this removes, and dropped two `export type {}` husks left where every name in a block was dead. One finding was NOT what it looked like: fallow flagged `DebugSymbolsOptions` and `DebugSymbolsResult` in `apple/core/debug-symbols/types.ts`, but removing them broke `apple/core/debug-symbols.ts`, which re-exported them from there. Both ends were dead — every real consumer imports from `contracts/debug-symbols.ts` — so the fix was removing the intermediate re-export too, not restoring the leaf. Safety: the published type surface is unchanged. All 11 entry points in package.json#exports export exactly the same names before and after. Two chunk files differ only because `AndroidSnapshotBackendMetadata` relocated between internal code-splitting chunks, which is not a consumer-visible boundary. * chore: prune fallow suppressions that no longer suppress anything The whole point of this arc was that fallow looked clean mainly because of its own ignore list, so the list itself deserved an audit. Emptying `ignoreExports` and re-running shows most entries no longer match a real finding: 20 blocks collapse to 5, with no change in what either fallow invocation reports. Removed 8 fully-stale blocks. They went stale for three different reasons: - code moved (`assertSafeDerivedCleanup` is exported from runner-cache.ts, not the runner-xctestrun.ts the suppression named), - the exports became genuinely used (apps.ts, runner-contract.ts, runner-session.ts, cloud-webdriver.ts, the test-utils fixtures), - and `installAndroidInstallablePath` was suppressed in two places at once. `app-lifecycle.ts` needed a code fix rather than a suppression: it re-exported three `parseAndroid*` helpers from app-parsers.ts that nothing imports through it, so the re-export is dropped and the block goes away. The two type re-exports on that statement ARE consumed and stay. The seven `src/daemon/handlers/*` blocks are KEPT, collapsed into one documented glob. They are not stale: those handlers are reached only through the dynamic `import()` table in request-handler-chain.ts, which the --production analysis behind `check:production-exports` cannot follow. A first pass removed them because the staleness probe only exercised the default config; the repo runs fallow twice, and `check:production-exports` caught it.
thymikee
added a commit
that referenced
this pull request
Jul 22, 2026
- drop the consumer-less AndroidFillVerificationNode re-export - reuse requireSnapshotSession in resolveSnapshotForRef instead of inlining the same authorized-frame resolution (fallow clone group); the helper's return type now guarantees the session it already throws for
thymikee
added a commit
that referenced
this pull request
Jul 23, 2026
…#1367) * chore: remove verified dead code and migration scaffolding Multi-agent audit of accumulated waste, every finding adversarially verified against call sites, git history, and the published surface before removal. Net -710 lines. - delete src/core/platform-descriptor/ (superseded ADR-0009 migration scaffold; parity tests now assert an inline table) - remove test-only seams: registry introspection exports, CommandFacet.extraDaemonWriters, MaestroEngineOptions.timing - remove dead flexibility: backend capability allow-list, screenshot-diff maxRegions, CloudWebDriverSupportLevel 'partial', clearFirst on the TS+Swift runner wire contract - remove dead deprecated surface: --session-locked / --session-lock-conflicts aliases (hard migration error now points at --session-lock), replay export --format single-value enum, unused Lease*Payload contract types, runtime-layer rotate duplicate - collapse pass-throughs/duplication: withRetry adapter, default-cloud-artifact-provider, connect-profile client-id hashing (3x sha256 impls -> one helper, byte-identical output), shared scripts walker, cloneValue -> structuredClone, fill-diagnostics moved into android/ BREAKING CHANGE: --session-locked and --session-lock-conflicts now fail with a migration error pointing at --session-lock; replay export --format is removed (Maestro was the only value); Lease*Payload types are dropped from the ./contracts subpath. * chore: satisfy fallow gates tightened by #1363/#1364 after rebase - drop the consumer-less AndroidFillVerificationNode re-export - reuse requireSnapshotSession in resolveSnapshotForRef instead of inlining the same authorized-frame resolution (fallow clone group); the helper's return type now guarantees the session it already throws for * chore: address review — keep cloud-webdriver partial capability metadata The partial/supported/unsupported levels and their notes are part of the lease-response capability contract for genuinely limited operations (Appium page-source snapshots, upload-then-install), not dead scaffolding. Restore them and the asserting tests unchanged from main. Also add the missing CHANGELOG entry for the Lease*Payload type removal from agent-device/contracts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Installed knip, used it for a full dead-code sweep, then removed it again and tuned
fallowinstead. Measured head-to-head on the same tree, fallow turned out to be a strict superset once two switches it already supports are flipped. All the cleanup knip surfaced is kept.Dead code removed
daemon/artifact-materialization.ts(224 lines) had no production caller — only its own test. Removing it exposed thatdownloadArtifactToTempDirand the entire URL-fetch-with-redirects path inartifact-download.tswere reachable only through it; the liveupload.ts/resumable-upload.tsuse the incoming-request helpers instead. That file drops 348 → 123 lines.readZipEntriesthen fell out ofartifact-archive.ts.Test-helper dead weight: 12 unused re-exports and 6 needlessly-exported mocks in
session-test-harness.ts, dead barrel entries in__tests__/test-utils/index.ts, pluswithMockedXcrun,matchesSchema,IOS_FRAME,IOS_TAB_FRAME,snapshotWithOffscreenContent.A duplicate:
androidSnapshotHelperOutputexisted byte-for-byte twice;provider-scenarios/android-world.tsnow imports the shared copy instead of redefining it.Types: 8 unreferenced aliases, plus 17 redundant type re-export lines in
client/client-types.ts.Note
The
client-types.tsremoval is not an API change. All 19 published.d.tsfiles are byte-identical before and after (index.d.ts= 55309 bytes both). Those types already reach consumers throughcontracts/*viaCommandResult<…>; the re-export lines were a redundant second path.Tooling
.fallowrc.jsongainsincludeEntryExports,ignoreExportsUsedInFile: {type, interface}, andunused-types: warn. That combination is what made the above visible — the previous config was quiet largely because of its own suppression list, not because the code was clean. Dropped 2 now-obsoleteignoreExportsentries and added 3 documented ones (publishedsdk/*surface, tool-configdefaultexports, and theAssertTrue<…>totality guards that exist only to satisfynoUnusedLocals).unused-typesstays atwarn: 72 pre-existing type re-export lines across 27 files remain and are tracked separately. Every other fallow detector is at zero.Why not knip
The two knip-only export findings weren't wins:
dispatchGenericCommandwas knip's false positive (fallow correctly silent), andtargetDagZonesits in a path fallow'signorePatternsexcludes by choice. Meanwhile fallow caught a deadexport { SessionStore }that knip missed — knip resolves a re-export back to its origin and marks it used. fallow also does dupes, complexity/health baselines, boundaries and changed-file audits, which knip has no equivalent for.Verification
pnpm check:tooling,pnpm check:fallow,pnpm format:check— all cleanpnpm build+ byte-comparison of all 19 published.d.ts— identicalWarning
The full parallel
pnpm test:unitrun shows rotating failures indaemon/__tests__/runtime-hints.test.tsandplatforms/apple/core/__tests__/index.test.ts. This is pre-existing and unrelated — verified by stashing this branch and reproducing the identical failures on the merge-base. Both files pass 114/114 when run serialized. These PATH-stub tests surface spawn timeouts as assertion failures under CPU load; tracked separately.