Files
orca/src/relay/git-handler-sync-operations.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

206 lines
7.8 KiB
TypeScript

import { randomUUID } from 'node:crypto'
import type { RequestContext } from './dispatcher'
import { GitHandlerOperationContext } from './git-handler-operation-context'
import { resolveRelayPushTarget } from './git-handler-push-target'
import { normalizeGitErrorMessage, runPullWithDivergenceFallback } from '../shared/git-remote-error'
import { assertValidGitPushTarget } from '../shared/git-push-target-validation'
import type { GitCommandRunner } from '../shared/git-publish-target-status'
import type { GitPushTarget } from '../shared/worktree/types'
import { resolveEffectiveGitUpstream } from '../shared/git-effective-upstream'
import { runWithGitWorktreeOperationLock } from '../shared/git-worktree-operation-lock'
import {
REBASE_FROM_BASE_OPERATION_TIMEOUT_MS,
REBASE_SOURCE_FETCH_TIMEOUT_MS,
resolveGitRemoteRebaseSource
} from '../shared/git-rebase-source'
import { isNoWriteFetchHeadUnsupportedError } from '../shared/git-fetch-head-capability'
export class GitHandlerSyncOperations extends GitHandlerOperationContext {
async push(params: Record<string, unknown>) {
this.clearGitMutationReadCaches()
const worktreePath = params.worktreePath as string
// Why: mirror src/main/git/remote.ts — push to a configured upstream when present so SSH worktrees with non-origin targets aren't repointed.
void params.publish
try {
try {
const target = await resolveRelayPushTarget(
this.git.bind(this),
worktreePath,
params.pushTarget
)
const args = [
'push',
...(params.forceWithLease === true ? ['--force-with-lease'] : []),
'--set-upstream',
...(target ? [target.remote, target.refspec] : ['origin', 'HEAD'])
]
await this.git(args, worktreePath)
} catch (error) {
// Why: mirror local gitPush normalization so SSH users get "non-fast-forward / pull first" guidance instead of raw git stderr.
throw new Error(normalizeGitErrorMessage(error, 'push'))
}
} finally {
this.clearGitMutationReadCaches()
}
}
private async pullWithArgs(
params: Record<string, unknown>,
pullArgs: string[],
signal?: AbortSignal
) {
const worktreePath = params.worktreePath as string
return runWithGitWorktreeOperationLock(worktreePath, signal, () =>
this.runPullWithArgsUnlocked(params, pullArgs)
)
}
private async runPullWithArgsUnlocked(
params: Record<string, unknown>,
pullArgs: string[]
): Promise<void> {
this.clearGitMutationReadCaches()
const worktreePath = params.worktreePath as string
const runPull = async (effectiveArgs: string[]): Promise<void> => {
if (params.pushTarget !== undefined) {
assertValidGitPushTarget(params.pushTarget)
const pushTarget = params.pushTarget as GitPushTarget
await this.git(['check-ref-format', '--branch', pushTarget.branchName], worktreePath)
await this.git(
['pull', ...effectiveArgs, pushTarget.remoteName, pushTarget.branchName],
worktreePath
)
return
}
const upstream = await resolveEffectiveGitUpstream((args) => this.git(args, worktreePath))
if (upstream && !upstream.isConfiguredUpstream) {
// Why: legacy Orca branches may track origin/main while pushes target origin/<branch>; pull the same effective branch the UI reports.
await this.git(
['pull', ...effectiveArgs, upstream.remoteName, upstream.branchName],
worktreePath
)
return
}
await this.git(['pull', ...effectiveArgs], worktreePath)
}
try {
try {
await runPullWithDivergenceFallback(pullArgs, runPull)
} catch (error) {
// Why: mirror local gitPull normalization so SSH users get actionable messages instead of raw git stderr.
throw new Error(normalizeGitErrorMessage(error, 'pull'))
}
} finally {
this.clearGitMutationReadCaches()
}
}
async pull(params: Record<string, unknown>, context?: RequestContext) {
// Why: plain `git pull` honors user merge/rebase/ff policy.
await this.pullWithArgs(params, [], context?.signal)
}
async fastForward(params: Record<string, unknown>, context?: RequestContext) {
await this.pullWithArgs(params, ['--ff-only'], context?.signal)
}
async rebaseFromBase(params: Record<string, unknown>, context?: RequestContext) {
return runWithGitWorktreeOperationLock(params.worktreePath as string, context?.signal, () =>
this.runRebaseFromBase(params, context)
)
}
private async runRebaseFromBase(params: Record<string, unknown>, context?: RequestContext) {
this.clearGitMutationReadCaches()
const worktreePath = params.worktreePath as string
const baseRef = params.baseRef as string
let rebaseRef: string | null = null
const controller = new AbortController()
const abortFromContext = () => controller.abort()
if (context?.signal?.aborted) {
controller.abort()
} else {
context?.signal?.addEventListener('abort', abortFromContext, { once: true })
}
const timeout = setTimeout(() => controller.abort(), REBASE_FROM_BASE_OPERATION_TIMEOUT_MS)
try {
try {
const source = await resolveGitRemoteRebaseSource(
((args) =>
this.git(args, worktreePath, {
signal: controller.signal,
terminationBarrier: true
})) as GitCommandRunner,
baseRef
)
let forkPoint: string | null = null
let hasHead = true
try {
const { stdout } = await this.git(
['merge-base', '--fork-point', `refs/remotes/${source.displayName}`, 'HEAD'],
worktreePath,
{ signal: controller.signal, terminationBarrier: true }
)
forkPoint = stdout.trim() || null
} catch {
// A first fetch or an unhelpful reflog falls back to Git's merge-base behavior.
try {
await this.git(['rev-parse', '--verify', 'HEAD'], worktreePath, {
signal: controller.signal,
terminationBarrier: true
})
} catch {
hasHead = false
}
}
// Why: concurrent fetches can replace FETCH_HEAD and remote-tracking refs between fetch and rebase.
rebaseRef = `refs/orca/rebase/${randomUUID()}`
const fetchArgs = [
source.remoteName,
`+refs/heads/${source.branchName}:${rebaseRef}`,
`+refs/heads/${source.branchName}:refs/remotes/${source.displayName}`
]
await this.gitCapabilities.runWithFallback(
'fetch-no-write-fetch-head',
() =>
this.git(['fetch', '--no-write-fetch-head', ...fetchArgs], worktreePath, {
timeout: REBASE_SOURCE_FETCH_TIMEOUT_MS,
signal: controller.signal,
terminationBarrier: true
}),
() =>
this.git(['fetch', ...fetchArgs], worktreePath, {
timeout: REBASE_SOURCE_FETCH_TIMEOUT_MS,
signal: controller.signal,
terminationBarrier: true
}),
isNoWriteFetchHeadUnsupportedError
)
await this.git(
hasHead
? forkPoint
? ['rebase', '--onto', rebaseRef, forkPoint]
: ['rebase', rebaseRef]
: ['merge', '--ff-only', rebaseRef],
worktreePath,
{ signal: controller.signal, terminationBarrier: true }
)
} catch (error) {
throw new Error(normalizeGitErrorMessage(error, 'pull'))
}
} finally {
if (rebaseRef) {
try {
await this.git(['update-ref', '-d', rebaseRef], worktreePath)
} catch {
// Cleanup must not hide the fetch or rebase result.
}
}
clearTimeout(timeout)
context?.signal?.removeEventListener('abort', abortFromContext)
this.clearGitMutationReadCaches()
}
}
}