Repository navigation
fix(admin,cli,scraping,testing): two admin actions could share a name and dispatch to the wrong one - #178
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 45 minutes Limit details: You’ve used the included review currently available. Your 72 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. You can run this review on demand instead of waiting. On-demand reviews are free until September 18, 2026. After that, they cost $0.25 per reviewed file.
How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (30)
Comment |
… and dispatch to the wrong one `AdminAction.name` addresses one handler in three places at once — the MCP tool name (`admin.action.<name>`), the default label key, and the key `callAdminTool` resolves a handler by. Two actions sharing it dispatched to whichever `.find()` reached first: a call that SUCCEEDS against the wrong action and reports nothing. Nothing refused it until `adminMcp()` was called, so an app that renders the dashboard and never wires MCP shipped the ambiguity silently. Refused at `defineAdmin` now (`X_ADMIN_ACTION_DUPLICATE`), by identity rather than by name — `defineAdmin` attaches `input.actions` to the entity each one names AND appends `resources[<entity>].actions`, so an author who spelled the same object in both meant one action. **`x g migration` has never been a generator**, and it shipped in two `@ultimat3/admin` `fix:` lines — so an operator whose admin list named a dropped column got `X_CLI_UNKNOWN_COMMAND` on top of the bug they already had. The real command is `x db gen "<description>"`. It reached the `errors` gate untouched because `x g` declares no subcommands and `fix-command.ts` returns `undefined` for an absent positional set; `g` now declares `positionalChoices: GENERATORS`, from the same constant `readKind` validates against. Third instance of this class (#131, then `x db branch <name>`). - `cli/fix-command`: the citation regex stopped a word at `:`, so `x g admin:page` — the CLI reference's one admin-page line — cited `x g admin` and was a standing false finding on a documented invocation that runs. A `:` now joins a word only when a letter follows it, so a sentence ending `x db migrate:` still cites `x db migrate`. - `cli/cmd-generate`: the conflict `fix:` was `x g --force`, which is `X_CLI_UNKNOWN_COMMAND` when run. A `fix:` is pasted verbatim, and the conflict path has the caller's own invocation in hand. - `scraping/robots-fetch`: new file. The production `/robots.txt` read — the one `scrape-run.ts` always takes, since it builds the gate with no `fetchText` — had no deadline, no size cap and no proxy. Every existing gate test injected one, which is how a read that could park a run forever stayed green. The gate caches one promise per origin, so a hung origin parked every later navigation to it, past `ctx.signal`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
d110e08 to
ce3c39e
Compare
1 participant
Tier 5 slice of a bug sweep.
One
AdminAction.name, two handlersAdminAction.nameaddresses one handler in three places at once — the MCP tool name(
admin.action.<name>), the default label key, andcallAdminTool's lookup. Two actions sharing itdispatched to whichever
.find()reached first: a call that succeeds against the wrong action andreports nothing.
Nothing refused it until
adminMcp()was called, so an app that renders the dashboard and neverwires MCP shipped the ambiguity silently. Refused at
defineAdminnow —X_ADMIN_ACTION_DUPLICATE,row added to
wiki/Error-Codes.md, manifest regenerated on this branch (453 codes).Deduplicated by identity, not name:
defineAdminattachesinput.actionsto the entity each onenames and appends
resources[<entity>].actions, so an author who spelled the same object in bothmeant one action, not two.
x g migrationhas never existedIt shipped in two
@ultimat3/adminfix:lines. An operator whose admin list named a droppedcolumn pasted it and got
X_CLI_UNKNOWN_COMMANDon top of the bug they already had. The migrationwriter is
x db gen "<description>".It passed the
errorsgate because a command with no subcommands must still declarepositionalChoices, or its second word is unjudged —fix-command.tsreturnsundefinedfor anabsent set.
cmd-generate.tsnow declarespositionalChoices: GENERATORS, from the same constantreadKindvalidates against, exactly ascmd-test.tsdeclaresTEST_TYPES.fix-command.test.tspins
x g migrationas a finding against the real catalog. Third instance of this class (#131, thenx db branch <name>); the rule is now written down inpackages/cli/CLAUDE.md.The rest
cli/fix-command.ts:, sox g admin:page— the CLI reference's one admin-page line — was read asx g adminand reported as a standing false finding on an invocation that runs. A:now joins a word only when a letter follows, sox db migrate: the release phasestill citesx db migratecli/cmd-generate.tsfix:wasx g --force, itselfX_CLI_UNKNOWN_COMMANDwhen run. Afix:is pasted verbatim, and the conflict path has the caller's own invocation in handscraping/robots-fetch.ts(new)/robots.txtread — the onescrape-run.tsalways takes, since it builds the gate with nofetchText— had no deadline, no size cap and no proxy. Every existing gate test injected afetchText, which is how a read that could park a run forever stayed green. The gate caches one promise per origin, so a hung origin parked every later navigation to that origin, pastctx.signal, the watchdog and the job timeoutscraping/robots.tspatternMatcheswalks the pattern instead of compiling it, so a wildcard-dense rule from a scraped site is linear rather than catastrophic backtracking on the worker's only threadadmin/widget-value.ts,admin/resource.ts,testing/matchers.ts,cli/i18n-audit.ts,cli/mcp-host.ts,cli/dev-runtime.tsGate
bun run verify— 14/17 green, 3 skipped as always.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.