Files
orca/src/shared/skills-cli-agent-keys.test.ts
T
Neil 231e805b1e fix(lint): enable anti-slop/no-shape-in-symbol-names (#20785)
Flip `anti-slop/no-shape-in-symbol-names` from "off" to "error" and clear
every violation under src, config, tests and mobile.

What the rule bans
------------------
The case-insensitive substring "shape" in any JS/TS identifier: variables,
functions, parameters, types, type parameters, class members, private names,
object-literal keys and JSX identifiers. The one exemption is a statically
accessed member read owned by another value (`zodObject.shape` is fine), so
third-party APIs stay readable without a suppression.

"Shape" names a value's structure rather than its domain role. `UserShape`,
`validateArgShape` and `errorShape` all tell you the symbol is "an object
with some fields" -- which is already what a type says -- while saying
nothing about what the value is for or who owns it. The rule forces the
name to carry the domain instead.

Violations fixed
----------------
689 violations across 109 files at baseline (verified by re-running the
audit against the pre-change tree with the rule set to "error").

Fix pattern
-----------
Rename for the domain role, not the structure:

  -type FieldShape = 'list' | 'map' | 'whole'
  -const FIELD_SHAPES = { ... } satisfies Record<keyof Observation, FieldShape>
  +type FieldEncoding = 'list' | 'map' | 'whole'
  +const FIELD_ENCODINGS = { ... } satisfies Record<keyof Observation, FieldEncoding>

  -function assertGitPushTargetShape(target: unknown): void
  +function assertValidGitPushTarget(target: unknown): void

  -function describeReadDirPathShape(p: string): ReadDirPathKind
  +function classifyReadDirPath(p: string): ReadDirPathKind

Predicates became statements about the value (`isDeltaShapedProviderFrameKind`
-> `isDeltaProviderFrameKind`, `isDeleteShapedDiscardEntry` ->
`discardDeletesEntryFile`, `isSkillsCliAgentKeyShaped` ->
`isUsableSkillsCliAgentKey`). Type aliases dropped the suffix where the
remaining name was already unambiguous (`GhGraphqlErrorShape` ->
`GhGraphqlError`).

No wire-visible name was renamed: no IPC or RPC channel, stream opcode,
request/response param, persisted field, or i18n key. The `--shape=symlink|copy`
CLI flag read by .github/workflows/skill-update-roundtrip.yml is unchanged --
only the local variable holding it was renamed.

Exemptions
----------
They are file-scoped entries in config/oxlint-anti-slop.json, not inline
`oxlint-disable` comments. An inline directive naming an anti-slop rule reads
back as an UNUSED directive under the root lint scan, which does not load this
plugin -- the changed-code quality gate counts that warning, so the comment form
cannot be used for a rule that lives only in this config.

* src/renderer/src/components/browser-pane/annotate/**:
  in the screenshot annotator a "shape" is the drawn geometry -- pen, arrow,
  rect, ellipse, highlight. That is a genuine domain noun, and it pervades
  every symbol in the module.
* repo-icon.tsx, repo-header-project-actions.tsx, mobile MobileRepoIcon.tsx:
  lucide exports the icon component as `Shapes`. The name is theirs, and the
  matching REPO_LUCIDE_ICONS key is the persisted icon name shared with the
  desktop picker -- renaming it would orphan saved repo icons.
* src/shared/onboarding-state-types.ts, src/shared/constants.ts:
  `shapedSidebar` is a persisted onboarding-checklist field and a telemetry
  enum member; renaming it would orphan saved state.
* src/shared/rpc-contract/rpc-send-params.ts: matching zod's own literal `shape`
  property is what selects the ZodObject branch of the conditional type.

No exemption was added merely to avoid a rename. Eight symbols initially
suppressed as "a cross-module refactor outside this change" were proven to have
zero non-TypeScript references repo-wide and renamed instead.

Zod's `ZodRawShape` needed no exemption at all: `Readonly<Record<string,
z.ZodType>>` is its definition, so repo-update-params.ts and
ui-update-value-tolerance-params.ts spell it out instead. Likewise
telemetry-event-classification.ts now reads `.shape` through an `in` narrowing,
which also retires two pre-existing type assertions; three more assertions the
rename had dragged onto changed lines (two `JSON.parse` sites, one node:sqlite
row read) became annotations and an explicit row mapping.

Verified
--------
* Audit reports zero violations; confirmed the rule genuinely fires by
  planting a probe violation.
* node config/scripts/run-typecheck-projects-in-parallel.mjs exits 0.
* Vitest over src/shared, src/main/github/project-view, the annotate module,
  the repo-icon components and the Chromium SameSite electron spec: all green.
* All 66 removed "shape" identifiers grepped repo-wide across every file type;
  none survive.
* node config/scripts/generate-rpc-params-catalog.mjs --check exits 0.
* node --check on every changed .mjs; oxfmt clean on all changed files.
* `pnpm run check:code-quality:changed` reports 0 findings.

Not machine-verified: the 3 mobile/ files (its Vitest run cannot resolve
`expo/tsconfig.base.json` in this worktree), and the WSL- and Playwright-gated
specs. All are rename- or comment-only hunks, read in full.
2026-09-15 02:00:27 -07:00

138 lines
3.5 KiB
TypeScript

import { describe, expect, it } from 'vitest'
import { TUI_AGENT_CONFIG } from './tui-agent-config'
import {
SKILLS_CLI_AGENT_KEY_BY_TUI_AGENT,
isUsableSkillsCliAgentKey,
SKILLS_CLI_UNIVERSAL_AGENT_KEY,
toSkillsCliAgentKeys
} from './skills-cli-agent-keys'
// Why: the community `skills` CLI validates --agent against this namespace and
// exits 1 on anything else, so a typo here breaks installs outright. Captured
// from `skills add --agent <invalid>`, which prints its own valid list (v1.5.20).
const SKILLS_CLI_VALID_AGENT_KEYS = new Set([
'aider-desk',
'amp',
'antigravity',
'antigravity-cli',
'astrbot',
'autohand-code',
'augment',
'bob',
'claude-code',
'openclaw',
'cline',
'codearts-agent',
'codebuddy',
'codemaker',
'codestudio',
'codex',
'command-code',
'continue',
'cortex',
'crush',
'cursor',
'deepagents',
'devin',
'dexto',
'droid',
'eve',
'firebender',
'forgecode',
'gemini-cli',
'github-copilot',
'goose',
'grok',
'hermes-agent',
'inference-sh',
'jazz',
'junie',
'iflow-cli',
'kilo',
'kimchi',
'kimi-code-cli',
'kiro-cli',
'kode',
'lingma',
'loaf',
'mcpjam',
'mistral-vibe',
'moxby',
'mux',
'opencode',
'openhands',
'ona',
'pi',
'qoder',
'qoder-cn',
'qwen-code',
'replit',
'reasonix',
'rovodev',
'roo',
'tabnine-cli',
'terramind',
'tinycloud',
'trae',
'trae-cn',
'warp',
'windsurf',
'zed',
'zcode',
'zencoder',
'zenflow',
'neovate',
'pochi',
'promptscript',
'adal',
'universal'
])
describe('skills CLI agent keys', () => {
it('only maps onto keys the skills CLI accepts', () => {
for (const [agent, key] of Object.entries(SKILLS_CLI_AGENT_KEY_BY_TUI_AGENT)) {
if (key !== null) {
expect(SKILLS_CLI_VALID_AGENT_KEYS, `${agent} -> ${key}`).toContain(key)
}
}
expect(SKILLS_CLI_VALID_AGENT_KEYS).toContain(SKILLS_CLI_UNIVERSAL_AGENT_KEY)
})
it('covers every agent Orca can detect', () => {
// Why: a new TuiAgent must be considered here, even if the answer is null —
// otherwise it silently falls back to universal-only with no decision made.
expect(Object.keys(SKILLS_CLI_AGENT_KEY_BY_TUI_AGENT).sort()).toEqual(
Object.keys(TUI_AGENT_CONFIG).sort()
)
})
it("follows Orca's own evidence for the two non-obvious mappings", () => {
// Why: src/shared/native-chat-agent-profiles.ts states OpenClaude reads
// Claude-owned roots, so it is not unmappable.
expect(SKILLS_CLI_AGENT_KEY_BY_TUI_AGENT.openclaude).toBe('claude-code')
// Why: Orca detects trae via `traecli`, which tui-agent-config calls an alias
// only TRAE CN ships, so the CN directory is the right target.
expect(SKILLS_CLI_AGENT_KEY_BY_TUI_AGENT.trae).toBe('trae-cn')
})
it('rejects values the skills CLI would drop, and allows the explicit wildcard', () => {
for (const bad of ['-y', '--copy', '', ' ', 'a b', 'a,b']) {
expect(isUsableSkillsCliAgentKey(bad), bad).toBe(false)
}
for (const good of ['claude-code', 'universal', 'trae-cn', 'inference-sh', '*']) {
expect(isUsableSkillsCliAgentKey(good), good).toBe(true)
}
})
it('always includes the shared directory and drops unmappable agents', () => {
expect(toSkillsCliAgentKeys(['claude', 'rovo'])).toEqual([
'claude-code',
'rovodev',
'universal'
])
// Why: `omp` has no skills-CLI equivalent, so it must not reach the argv.
expect(toSkillsCliAgentKeys(['omp'])).toEqual(['universal'])
expect(toSkillsCliAgentKeys([])).toEqual(['universal'])
})
})