Skip to content

Commit eb3fc5b

Browse files
thymikeeclaude
andauthored
chore: scan packages/** with fallow instead of ignoring it (#1591)
`ignorePatterns: ["packages/**"]` landed in #1494 W0 with the recorded reason "its resolver cannot follow workspace specifiers". That was either wrong at the time or never re-checked: the fallow version has not moved (^2.95.0 then and now) and it resolves @agent-device/* through each package's exports map today. packages/kernel alone exposes 8 subpaths and ~110 exports reachable only via workspace specifiers, and scanning it reports zero findings — a resolver that could not follow the specifier would report all of them. The cost of the ignore is that every package extraction silently removes its code from dead-code analysis. #1589 moved the selector engine into packages/selectors/ and shipped a façade with 15 zero-consumer exports, including `selectorUsesKey`, written in that PR and never called. A follow-up commit removed them by hand; nothing would have caught them. Removing the pattern surfaced 43 findings, driven to zero by deleting the dead code rather than by baselining or excluding it (fallow-baselines/*.json are empty on purpose — the posture is fix-or-document-the-exemption, so a first baseline entry would be a policy change): - 38 are deleted. 24 façade type re-exports whose only claim was that a consumer might one day want to name them — typecheck is green without every one, so the claim was theoretical; 5 façade value re-exports; 9 `export` keywords on symbols used only inside their own file. Every deleted façade symbol comes off scripts/layering/facade-symbols.ts (and ad-replay's inline pin in package-boundaries.test.ts) in the same change, so R11 is narrowed with the façade, never weakened around it. - 4 stale suppressions in src/provider-limrun-runtime.ts existed only because packages/ was invisible. - 5 have consumers analysis genuinely cannot see, and get an `ignoreExports` entry naming the consumer per the existing `comment` convention: four test-tree importers that --production does not walk, and `LimrunIosCommandExecution`, which src/sdk/limrun.ts republishes as agent-device/limrun — its only importer compiles in a temp checkout, so no static edge reaches it. test/integration/limrun-public-types.test.ts is the standing proof that one is real API. Three doc comments named types their façade no longer exports and are corrected rather than left asserting something false — including #1555's claim in session-replay-target-verification.ts that the daemon imports `AdReplayVerifiedTargetGuard` directly. It does not; it reaches that shape through `AdReplayTargetClassification`/`AdReplayDispatchGuard`, which is why the name read as dead. `scripts/maestro-conformance/**` was ignored wholesale to cover its corpus data. Narrowed to `corpus/**`, which un-hides the tooling beside it and turned up one more file-local export (`buildManifest`); regenerate.mjs's importer of `fixtureContentHash` becomes visible, so that needs no exemption at all. scripts/check-affected/model.ts deliberately did not select the `fallow` check for packages/*/src/**, carrying the same stale rationale as a comment. Without that selection the new scope would never run in the affected-driven lane, so the ignore removal would have bought nothing. model.test.ts now pins the selection. Verified: check:fallow and check:production-exports green with packages in scope; full-repo `fallow dead-code` back to its one pre-existing finding; typecheck, layering (R11), lint, format, build, check:package, and the limrun published-types integration test all pass. Probed by adding a fresh zero-consumer export to the xml façade — check:production-exports reports it, so the #1589 case now fails the gate. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 8d526f2 commit eb3fc5b

21 files changed

Lines changed: 60 additions & 102 deletions

File tree

.fallowrc.json

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,10 @@
3434
"website/rspress.config.ts"
3535
],
3636
"ignorePatterns": [
37-
"packages/**",
3837
"examples/test-app/**",
3938
"scripts/perf/**",
4039
"scripts/layering/**",
41-
"scripts/maestro-conformance/**",
40+
"scripts/maestro-conformance/corpus/**",
4241
"apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests.xctestplan",
4342
"scripts/write-xcuitest-cache-metadata.mjs",
4443
"scripts/help-conformance-sample-outputs.d.mts"
@@ -119,6 +118,26 @@
119118
"comment": "#1458: getSessionCommandKind's former production consumer (the session.ts if-chain) was replaced by SESSION_COMMAND_HANDLER_IMPLS, which encodes command-kind groupings directly via shared handler references instead of a kind lookup. The accessor and its SessionCommandKind classification remain covered by daemon-command-registry.test.ts, a test-tree file outside --production analysis.",
120119
"file": "src/daemon/daemon-command-registry.ts",
121120
"exports": ["getSessionCommandKind"]
121+
},
122+
{
123+
"comment": "Workspace-package symbols whose only consumers are test-tree files, which --production analysis does not see. Each import is the package specifier, so the façade entry is load-bearing: parseTargetAnnotationV1Payload/TARGET_ANNOTATION_MAX_FIELD_BYTES from src/daemon/__tests__/session-target-evidence.test.ts, MAESTRO_COMPAT_* from src/cli/parser/__tests__/maestro-support-matrix.test.ts.",
124+
"file": "packages/{ad-script,maestro}/src/index.ts",
125+
"exports": [
126+
"parseTargetAnnotationV1Payload",
127+
"TARGET_ANNOTATION_MAX_FIELD_BYTES",
128+
"MAESTRO_COMPAT_LIMITATIONS",
129+
"MAESTRO_COMPAT_SUPPORTED_CAPABILITIES"
130+
]
131+
},
132+
{
133+
"comment": "Deliberately kept off the @agent-device/maestro façade (index.test.ts asserts its absence) and consumed only by the conformance harness under packages/maestro/test/.",
134+
"file": "packages/maestro/src/internal/program-ir-command-parser.ts",
135+
"exports": ["SUPPORTED_MAESTRO_COMMAND_NAMES"]
136+
},
137+
{
138+
"comment": "Published type surface: re-exported by src/sdk/limrun.ts to agent-device/limrun, where test/integration/limrun-public-types.test.ts pins that consumers can import it. Its only importer compiles in a temp checkout, so no static in-repo edge reaches it — the same reason src/sdk/*.ts is exempt above.",
139+
"file": "packages/provider-limrun/src/index.ts",
140+
"exports": ["LimrunIosCommandExecution"]
122141
}
123142
],
124143
"usedClassMembers": ["name", "listActiveLeases", "delete", "values", "elapsedMs", "isExpired"],

packages/ad-replay/src/index.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,8 @@
6464
* and the classification/guard/binding-evidence/verification-routing shapes
6565
* `verifyAndDispatchStep` exchanges with the daemon's `AdReplayStepRuntime`
6666
* implementation (`AdReplayVerificationEntry`, `AdReplayTargetClassification`,
67-
* `AdReplayTargetBindingEvidence`, `AdReplayVerifiedTargetGuard`,
68-
* `AdReplayDispatchGuard`, `AdReplayDispatchOutcome`) ARE named here: the
67+
* `AdReplayTargetBindingEvidence`, `AdReplayDispatchGuard`,
68+
* `AdReplayDispatchOutcome`) ARE named here: the
6969
* daemon builds/reads real values of these shapes directly now (routing in
7070
* `session-replay-target-verification.ts`, wire-narrowing in
7171
* `session-replay-runtime-engine-adapter.ts`) rather than re-declaring a
@@ -99,7 +99,6 @@ export type {
9999
AdReplayTargetClassification,
100100
AdReplayVarSources,
101101
AdReplayVerificationEntry,
102-
AdReplayVerifiedTargetGuard,
103102
} from './internal/runtime-port-types.ts';
104103

105104
export type {

packages/ad-replay/src/internal/step-loop.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -224,9 +224,7 @@ function resolveActionLoc(
224224
* lifecycle-skipped, never dispatched or expanded (native `.ad` has no
225225
* include grammar).
226226
*/
227-
export function isExecutableReplayAction(
228-
action: SessionAction | undefined,
229-
): action is SessionAction {
227+
function isExecutableReplayAction(action: SessionAction | undefined): action is SessionAction {
230228
return Boolean(action && action.command !== 'replay');
231229
}
232230

@@ -249,7 +247,7 @@ export function isExecutableReplayAction(
249247
* nor included in the successful `replayed` count, exactly like the `replay`
250248
* pseudo-command just above it in the loop.
251249
*/
252-
export function resolveSuppressedTerminalCloseIndex(
250+
function resolveSuppressedTerminalCloseIndex(
253251
actions: readonly SessionAction[],
254252
): number | undefined {
255253
for (let index = actions.length - 1; index >= 0; index -= 1) {

packages/ad-replay/src/internal/target-verification.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ export type ReplayPostDispatchMismatchEvidence = {
177177
};
178178

179179
/** A `position:` mismatch line from the guard's structural denotations, when both are present and differ. */
180-
export function describeStructuralMismatch(
180+
function describeStructuralMismatch(
181181
expected: AdReplayTargetStructuralDenotation | undefined,
182182
observed: AdReplayTargetStructuralDenotation | undefined,
183183
): string | undefined {

packages/ad-script/src/index.ts

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,8 @@ export {
6666
} from './internal/script-formatting.ts';
6767

6868
export {
69-
normalizeIdentifierField,
70-
normalizeLabelField,
71-
normalizeRoleField,
7269
parseTargetAnnotationV1Payload,
7370
serializeTargetAnnotationV1,
74-
truncateToUtf8Bytes,
7571
utf8ByteLength,
7672
TARGET_ANNOTATION_MAX_ANCESTRY,
7773
TARGET_ANNOTATION_MAX_FIELD_BYTES,
@@ -92,16 +88,9 @@ export {
9288
siblingOrdinal,
9389
structuralDenotationsEqual,
9490
} from './internal/target-annotation-identity.ts';
95-
export type {
96-
LocalIdentity,
97-
NodeStructuralDenotation,
98-
} from './internal/target-annotation-identity.ts';
91+
export type { LocalIdentity } from './internal/target-annotation-identity.ts';
9992

10093
export { classifyTargetBindingMatch } from './internal/target-annotation-classification.ts';
101-
export type {
102-
TargetBindingClassification,
103-
TargetBindingClassificationInput,
104-
} from './internal/target-annotation-classification.ts';
10594

10695
export {
10796
buildReplayVarScope,
@@ -112,4 +101,3 @@ export {
112101
readReplayShellEnvSource,
113102
resolveReplayAction,
114103
} from './internal/vars.ts';
115-
export type { ReplayVarScope } from './internal/vars.ts';

packages/maestro/src/internal/conformance-normalize.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ function numLike(value: unknown): number | string | undefined {
325325
return undefined;
326326
}
327327

328-
export function dropUndefined<T extends Record<string, unknown>>(value: T): T {
328+
function dropUndefined<T extends Record<string, unknown>>(value: T): T {
329329
for (const key of Object.keys(value)) {
330330
if (value[key] === undefined) delete value[key];
331331
}

packages/maestro/test/conformance/verify.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ function readJson<T>(file: string): T {
6464
return JSON.parse(fs.readFileSync(file, 'utf8')) as T;
6565
}
6666

67-
export const FIXTURE_FILES = ['layer1-parser.json', 'layer2-semantics.json'] as const;
67+
const FIXTURE_FILES = ['layer1-parser.json', 'layer2-semantics.json'] as const;
6868

6969
export function loadLayer1(): Layer1Fixture {
7070
return readJson(path.join(FIXTURES_DIR, 'layer1-parser.json'));
@@ -198,7 +198,7 @@ export function checkLayer2(): Layer2Result[] {
198198
export type CoverageResult = { command: string; covered: boolean; unverified: boolean };
199199

200200
/** Which native agent-device command kinds each corpus flow parses to. */
201-
export function agentKindsByCorpus(): Set<string> {
201+
function agentKindsByCorpus(): Set<string> {
202202
const kinds = new Set<string>();
203203
for (const flow of loadLayer1().flows) {
204204
// Rejected flows contribute nothing to coverage; a crash still throws.

packages/provider-limrun/src/index.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,6 @@
11
export { LIMRUN_PROVIDER } from './device.ts';
22
export { createLimrunRuntime, type LimrunRuntime, type LimrunRuntimeOptions } from './runtime.ts';
3-
export {
4-
verifyLimrunConnection,
5-
type LimrunConnectionVerification,
6-
type LimrunConnectionVerificationOptions,
7-
} from './connection-verification.ts';
3+
export { verifyLimrunConnection } from './connection-verification.ts';
84

95
export type { LimrunRuntimeDependencies } from './runtime-dependencies.ts';
106

packages/provider-webdriver/src/aws-device-farm.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ export function createAwsDeviceFarmPrepareSession(
247247
};
248248
}
249249

250-
export function selectAwsDeviceFarmWebDriverEndpoint(
250+
function selectAwsDeviceFarmWebDriverEndpoint(
251251
session: AwsDeviceFarmRemoteAccessSession,
252252
): string | undefined {
253253
const endpointValues =

packages/provider-webdriver/src/browserstack-device-features.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,9 +128,7 @@ export function buildBrowserStackDeviceFeatureCapabilities(
128128
* and then silently dropped — the session runs with provider defaults and nothing says why.
129129
* Callers use this to reject them at the point the provider is known.
130130
*/
131-
export function browserStackOnlyDeviceFeatureFlags(
132-
flags: Record<string, unknown> | undefined,
133-
): string[] {
131+
function browserStackOnlyDeviceFeatureFlags(flags: Record<string, unknown> | undefined): string[] {
134132
return BROWSERSTACK_DEVICE_FEATURE_SPECS.filter((spec) => {
135133
const value = flags?.[spec.field];
136134
return value !== undefined && value !== false && value !== '';

0 commit comments

Comments
 (0)