Commit Graph
4 Commits
Author SHA1 Message Date
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
NeilandOrca fdb58695e9 [P1] fix(checks): stop skipped and manual checks reporting as failures (#11700)
* fix(checks): stop skipped and manual checks reporting as failures

Route every check-classification surface through one shared helper so
desktop renderer, desktop main and mobile agree on the same verdict.

- GitLab `manual` jobs and pipelines are neutral again, not action_required/failure
- `skipped` counts as passed everywhere, including mobile
- a neutral check no longer demotes a summary that has passing checks

* fix(checks): move the check-classification parity test into the renderer project

The parity table lived in src/shared but imported a renderer module, and both
config/tsconfig.node.json and config/tsconfig.cli.json are composite projects
that include src/shared without that renderer path, so `pnpm typecheck` failed
with TS6307 on two of its three projects. Only the web project spans both trees.

Co-authored-by: Orca <help@stably.ai>

* fix(checks): stop the Tasks-grid pill contradicting its own verdict

The checks pill's label, tone and icon all read one ProviderCheckSummary, but
getChecksLabel short-circuited on the raw `neutral` counter while the tone and
icon key off `state`. After the classification fix a PR with 19 success + 1
neutral renders an emerald CheckCircle2 pill that reads "1 unresolved", and
mobile's own label (which keys off `state`) reads "19/20 passed" for the same
summary.

Move the label into src/shared/provider-check-summary.ts so desktop and mobile
cannot fork it again, and key it off `state`.

Also covers deriveWorkItemCheckSummary, the desktop-main producer of the summary
that reaches the Tasks grid and the relay-paired mobile client. It was rewritten
here with no test at all; the parity table stands in derivePRCheckStatusFromRollup,
which is a different normalizer. The new main-process test drives getWorkItem with
a real statusCheckRollup fixture, pinning the StatusContext `state` fallback that
would otherwise be deletable with the whole suite still green.

Co-authored-by: Orca <help@stably.ai>

* fix(gitlab): route the pipeline job-array rollup through the shared check classifier

The array path in derivePipelineStatus kept its own copy of the rollup rules, so
manual-only read green and one unrecognized job status demoted a passing pipeline
to neutral — both disagreeing with every other check surface.

Also retry the packaged-CLI smoke temp cleanup on Windows: the copied Orca.exe can
still be locked by AV/indexers after every assertion passed, failing the package job.

Co-authored-by: Orca <help@stably.ai>

* fix(gitlab): stop the skipped pipeline string diverging from the Checks tab

- classifyPipelineString now counts a skipped pipeline as passing, matching
  the per-check classifier; canceled stays neutral and is pinned as an
  explicit, sign-off-pending divergence.
- Pin the production string path (head_pipeline.status) in the parity table
  and note that the job-array branch has no production caller yet.
- Count skipped checks in the Checks panel's passing header so it agrees
  with the checks pill.
- Correct the packaged-CLI smoke retry comment: the EBUSY is the smoke's own
  just-exited Electron process, not AV/indexers.

Co-authored-by: Orca <help@stably.ai>

* fix(checks): finish cross-surface check parity and back out the skipped MR-card flip

Review follow-ups on the check-classification PR.

- PullRequestPage and GitHubItemDialog kept private copies of getCheckCounts /
  getChecksSummaryLabel that still counted only `success` as passing, so a
  2-success/3-skipped PR read "2 passing · 3 skipped" there and "5 passing" in
  the sidebar. Both copies move to pr-check-counts.ts, which routes the passing
  bucket through classifyCheckOutcome; action_required keeps its own amber
  bucket. The summary icon now keys off passing count, so an all-neutral PR
  stops painting a green tick above "0 of N checks passing".
- The sidebar checks header and triage strip still called
  `{status: completed, conclusion: null}` pending, contradicting the grey
  "Unresolved checks" pill. Both now read summarizeProviderChecks and render an
  unresolved chip/strip instead of an amber spinner that can never resolve.
- classifyPipelineString('skipped') is reverted to neutral. That flip painted
  MR cards green for pipelines that never ran, on the only GitLab path with
  production callers, and contradicted the same function's deferral of
  `canceled`. Both tone changes stay deferred, pinned by one test.
- classifyPipelineString('manual') resolves to pending rather than neutral: a
  blocked pipeline is outstanding, and neutral let the worktree card fall
  through to its emerald `open` default while GitLab still refuses the merge.
- TaskPage's checks pill helpers move to task-page-checks-pill.ts so the
  "1 unresolved on a green pill" fix is actually pinned by a test.
- smoke-packaged-cli no longer lets an EBUSY cleanup replace the real failure.

* fix(checks): stop completed unknown checks from spinning

---------

Co-authored-by: Orca <help@stably.ai>
2026-07-31 04:58:15 -07:00
KyuJoo HanandOrcaWin 6f3845baa4 fix(checks): rank successful checks above skipped and neutral (#11337)
* fix(checks): rank successful checks above skipped and neutral

Checks were ordered with `skipped` (4) and `neutral` (3) ahead of
`success` (5), so a PR with a long tail of skipped jobs pushed every
passing check below the fold — you scroll past a wall of "Skipped" to
find out whether anything actually ran.

Rank the no-signal conclusions last (`success` 3, `neutral` 4, `skipped`
5) and pull the order out of its three duplicated copies
(checks-panel-content, PullRequestPage, GitHubItemDialog) into
`src/shared/pr-check-severity-order.ts`. Unknown conclusions now sink to
the bottom instead of silently ranking as `neutral`.

* fix(checks): look up check ranks through a Map, not an object literal

An object-literal rank table resolves `constructor`, `toString`, and
`__proto__` off Object.prototype, so those keys returned a function
instead of falling through to UNKNOWN_CHECK_RANK — the comparator then
subtracted functions, went NaN, and left the list in arbitrary order.
Conclusions come from provider payloads, so keep the lookup on a Map and
cover prototype property names in the test.

* test(checks): cover provider-neutral ordering states

* fix(checks): preserve actionable provider states

* fix(checks): preserve unresolved provider rollups

* fix(checks): keep unknown GitLab rollups neutral

* fix: preserve neutral review check summaries

* fix: complete provider-neutral check ordering remediation

* fix: use provider-neutral mobile review status input

* fix: hydrate GitLab mobile review status

* fix: type mobile GitLab review hydration

---------

Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>
2026-07-30 13:29:41 -07:00
Jinwoo HongandOrca b1973657ea Add mobile Tasks parity (#2452)
Co-authored-by: Orca <help@stably.ai>
2026-05-21 20:26:07 -07:00