5 Commits

Include renames

Author	SHA1	Message	Date
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
Neil	080c95940f	fix(ssh): let fork-PR worktrees add their contributor remote via the relay (#15827) ... * fix(ssh): let fork-PR worktrees add their contributor remote via the relay Creating a workspace from a fork PR on an SSH host failed with "Destructive git remote operations are not allowed via exec". The relay's git.exec allowlist blocked every `remote` write subcommand, but SSH fork-PR creation has to run `git remote add <fork> <url>` on the host before it can fetch and track the contributor's branch, so the whole create aborted. Allow exactly the two shapes that flow needs -- `remote add <name> <url>` and `remote remove <name>` -- validated with the same remote-name and URL rules the relay already applies to every pushTarget-carrying RPC. Everything else (set-url, rename, prune, extra operands, flags before the action) stays blocked, and the URL must be a github.com clone/ssh URL, so no new reach is granted beyond what push/fetch already accept. `remote remove` was blocked too, which silently leaked fork remotes on SSH hosts: worktree removal swallows the cleanup error. It works again now. A host still running an older relay gets an actionable "reconnect to deploy the latest relay" message instead of the raw policy error. * test(git-exec): pin remote read/write mutation classification Misclassifying `git remote` / `remote get-url` as mutating would flush the relay and SSH provider git read caches on every remote probe, so pin both directions.	2026-08-21 14:44:08 -07:00
Neil	77f23b013f	refactor(shared): drop the shared/types barrel and import from the real modules (#14447) ... #14397 split `shared/types.ts` into 46 per-domain modules but kept the path as a re-export barrel so the import sites did not have to change. This removes the barrel: every consumer now imports from the module that actually declares the type, and `src/shared/types.ts` is deleted. Barrels hide where a type lives, make every consumer look like it depends on the whole domain, and let an unrelated edit invalidate a module that ~2,000 files transitively import. 2,323 import declarations across 2,321 files. Rewritten mechanically: each specifier was resolved to an absolute path via the TypeScript AST and recomputed, rather than string-substituted, so alias forms (`@/../../shared/ types`) and per-specifier `type` modifiers survive. Four cases the mechanical pass had to handle, each found by a gate rather than by reading the diff: - Modules inside `src/shared` import the barrel as `./types`, not `shared/types`. A pre-filter on the latter string skipped 176 of them and left imports dangling at a deleted file, which surfaced as confusing `Property 'x' is optional in type 'Repo' but required in Pick<Repo, ...>` errors rather than "module not found". - The barrel RENAMED one type on the way through (`WorkspaceSource as WorkspaceCreateTelemetrySource`), so the original name in the owning module has to be re-aliased at each consumer. - Three test files put `;(globalThis as ...)` on the line after the import. TypeScript parses that `;` as the import statement's terminator, so replacing through `statement.getEnd()` deletes it and breaks ASI. The rewrite now stops at the module specifier. - A file that already imported directly from a module got a SECOND import from it, because the barrel re-exported those same names — which trips `import/no-duplicates` under `--deny-warnings`. A post-pass merges declarations sharing a specifier and type-only-ness; the `import type` plus `import` pair from one module is left alone, since that form is allowed. Splitting one barrel import into several genuinely adds lines, which pushed `terminal-layout-pty-ownership.ts` to 301 counted lines: its 107-character import must wrap, and neither local type collapses onto one line (101 and 116 characters). Rather than contort a type declaration to fit a line budget, `collectLeafIds` and `pruneLeaves` move to `terminal-pane-layout-tree.ts` — they are pure structural operations on the layout tree and independent of PTY ownership. `visible-worktrees.ts` similarly loses its own mini-barrel re-export of `isDefaultBranchWorkspace`, with the four real consumers repointed at the declaring module. No `max-lines` bypass added. Verified: cold `tsc --noEmit` green on node, cli, and web (buildinfo deleted first — these projects are `composite: true` and reuse stale caches); the full `pnpm lint` green, not just bare oxlint — the narrower local check is what let the duplicate imports reach CI; max-lines ratchet OK at 344.	2026-08-13 22:48:24 -07:00
Neil	e3c30712ca	fix: allow slash remote push targets (#4109)	2026-05-31 04:38:37 -07:00
Jinjing	5a6b2994ea	fix: address review findings (#1770)	2026-05-13 16:14:50 -07:00