[🔥AUDIT🔥] review: split the roster concern out of dispatch.ts, which is over its line cap - #304
Merged
Merged
Conversation
… dispatch.ts, which is over its line cap main is red on lint, and neither PR that caused it could have seen it. `@khanacademy/eslint-config` sets max-lines to 1000. #302 took workflows/review/lib/dispatch.ts to exactly 1000 lines; the single line #299 added to SHED_RANKING took it to 1001. Each PR was green against its own base, so the violation existed only in the merge, which is invisible to a per-PR lint run. Every open PR in the repo inherits the failure, including #300. Fixed by extracting a concern rather than by reclaiming a line, so the next addition does not land in the same place, and rather than raising the cap, which lives in the shared Khan config and would be a house-rule deviation. DEFAULT_FINDERS, SHED_RANKING, Roster, RosterShed and computeRoster move to dispatch-roster.ts and are re-exported from dispatch.ts, which already advertises one import surface for the dispatch machinery. The moved code is byte-identical; dispatch.ts goes 1001 -> 920 lines and the new module is 108. Verified: 1573 tests pass unmodified, typecheck clean, and eslint over everything CI lints (actions, utils, workflows, minus the ignorePatterns paths) reports zero errors. Worth recording for next time: a plain local `pnpm run lint` cannot reproduce CI inside a git worktree under .claude, because eslint skips dot-directories by default and silently ignores the whole tree; `--resolve-plugins-relative-to . --no-ignore` scoped to the CI paths is what actually reproduces it.
🦋 Changeset detectedLatest commit: f33e674 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
khan-actions-bot
requested review from
a team,
jaredly and
somewhatabstract
and removed request for
a team
July 30, 2026 22:15
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
jwbron
added a commit
that referenced
this pull request
Jul 31, 2026
…ing out of dispatch.ts for the line cap The refusal-fallback additions pushed dispatch.ts to 1002 lines, one over the max-lines cap that drove the roster split in #304. parseAgentFile/loadAgents and the DispatchFs shape move to dispatch-agents.ts: self-contained, no dispatch state, and re-exported so callers keep one import surface. DispatchFs moves with them rather than staying behind, because leaving it in dispatch.ts made the two modules import each other and the cycle broke loadAgents at runtime while tsc stayed clean.
jwbron
added a commit
that referenced
this pull request
Jul 31, 2026
…ing out of dispatch.ts for the line cap The refusal-fallback additions pushed dispatch.ts to 1002 lines, one over the max-lines cap that drove the roster split in #304. parseAgentFile/loadAgents and the DispatchFs shape move to dispatch-agents.ts: self-contained, no dispatch state, and re-exported so callers keep one import surface. DispatchFs moves with them rather than staying behind, because leaving it in dispatch.ts made the two modules import each other and the cycle broke loadAgents at runtime while tsc stayed clean.
jwbron
added a commit
that referenced
this pull request
Jul 31, 2026
…ing out of dispatch.ts for the line cap The refusal-fallback additions pushed dispatch.ts to 1002 lines, one over the max-lines cap that drove the roster split in #304. parseAgentFile/loadAgents and the DispatchFs shape move to dispatch-agents.ts: self-contained, no dispatch state, and re-exported so callers keep one import surface. DispatchFs moves with them rather than staying behind, because leaving it in dispatch.ts made the two modules import each other and the cycle broke loadAgents at runtime while tsc stayed clean.
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.
🖍 This is an audit! 🖍
Summary:
main is red on lint, and neither PR that caused it could have seen it.
@khanacademy/eslint-configsets max-lines to 1000. #302 tookworkflows/review/lib/dispatch.ts to exactly 1000 lines; the single line #299
added to SHED_RANKING took it to 1001. Each PR was green against its own
base, so the violation existed only in the merge, which is invisible to a
per-PR lint run. Every open PR in the repo inherits the failure, including
#300.
Fixed by extracting a concern rather than by reclaiming a line, so the next
addition does not land in the same place, and rather than raising the cap,
which lives in the shared Khan config and would be a house-rule deviation.
DEFAULT_FINDERS, SHED_RANKING, Roster, RosterShed and computeRoster move to
dispatch-roster.ts and are re-exported from dispatch.ts, which already
advertises one import surface for the dispatch machinery. The moved code is
byte-identical; dispatch.ts goes 1001 -> 920 lines and the new module is 108.
Verified: 1573 tests pass unmodified, typecheck clean, and eslint over
everything CI lints (actions, utils, workflows, minus the ignorePatterns
paths) reports zero errors. Worth recording for next time: a plain local
pnpm run lintcannot reproduce CI inside a git worktree under .claude,because eslint skips dot-directories by default and silently ignores the
whole tree;
--resolve-plugins-relative-to . --no-ignorescoped to the CIpaths is what actually reproduces it.
Test plan:
pnpm exec vitest run: 1573 tests pass, none modified by this change.pnpm run typecheck: clean.pnpm exec eslint --resolve-plugins-relative-to . --no-ignore --ext .js,.mjs,.ts actions utils workflows, filtered to exclude theignorePatternscorpus trees, reports 0 errors. Before the change the same command reports themax-lineserror ondispatch.ts.git show --statconfirms the move is a move: no line ofcomputeRosteror the ranking changed.Unblocks every open PR in the repo, #300 included, since they all contain main's
dispatch.ts. #300 needs no changes of its own; it goes green once this lands.