[STiFLeR7]

Part of #836 — the suspicious/perf oxlint category item specifically (#863 type-aware linting and the anti-slop rules are separate, still-open items of the same issue, so this doesn't close it).

What this does

Enables oxlint's suspicious and perf rule categories under the existing zero-warning CI gate, with the off list from the issue:

• no-await-in-loop — off: sequential awaits are deliberate in a CLI (git, prompts, ordered writes); the rule can't tell an intentional ordering dependency from a parallelizable loop.
• unicorn/no-array-sort — off: every prod sort site sorts a local array built in the same function; none mutates a caller's array.
• unicorn/consistent-function-scoping — off: moves closures to an outer scope; style, not a bug class.
• no-underscore-dangle — off: conflicts with this repo's own _ convention (_setLogFilePath, _resetState).
• oxc/no-map-spread — off: micro-optimization, not worth the churn.
• no-shadow — kept on: real shadowing bugs were found during triage.

Also bumps tsconfig.json's target to ES2023 so Array#toReversed()/Array#findLast() (used by several fixes below) type-check. Node 20+ already runs these at runtime; ES2023 adds no new syntax, so this only changes which lib typings the compiler sees.

Heads up on scope: the issue's counts are from when it was filed and the codebase has grown a lot since (big workflow/session-flow additions). Live oxlint on current main with this config reports 393 warnings, not the issue's 100 (51 prod/49 test) — the off-list rules are unaffected by the drift (they're off either way), but the rules that stay on scaled up roughly 3-10x along with the codebase. All 393 are fixed here, plus one more perf hit the issue didn't know about (oxc/no-accumulating-spread in workflow/drivers/skill/tick.ts).

no-unmodified-loop-condition — investigated, not a bug

The issue flagged this rule specifically: "a loop whose condition never changes may be a real bug." There are 4 current hits (the issue named 1; the other 3 are new since). I checked each — none is a real bug, all are false positives where the condition is flipped by something outside the loop body that the (non-type-aware) rule can't see:

• src/resources/env.ts — a one-time const exists guard in a compound condition; the other half of the condition (a directory walk) is what actually changes and ends the loop.
• src/tool-launch/process-stop.ts — escalated flips inside a setTimeout closure registered before the loop starts (a deliberate SIGKILL-escalation timer).
• src/__tests__/recall-attribution.test.ts — settled flips inside a .finally() callback registered before the loop.
• src/__tests__/workflow-skill-transitions.test.ts — tails gains a key via property assignment inside the loop body; the rule only tracks variable reassignment, not property mutation.

Each got a narrow oxlint-disable-next-line with a one-line reason, not a restructure.

Everything else

Fixed per each rule's actual meaning, not a blanket find-replace:

• preserve-caught-error: attached { cause } to every rethrow, using whichever error the nearest enclosing catch actually holds — two spots in local-agent.ts and one in resources/env.ts had already attached { cause: error } from an outer catch to a throw sitting inside a nested catch; fixed those to reference the inner error instead.
• unicorn/no-array-reverse: switched to Array#toReversed(), dropping any now-redundant defensive [...arr] copy.
• unicorn/prefer-array-find: .filter(p)[0] → .find(p). Where .filter(p).at(-1) or .filter(p).pop() was reaching for the last match, used .findLast(p) instead — .find() there would have silently returned the wrong element.
• unicorn/prefer-set-has: converted to Set+.has(), except workflow/workspace.ts's rest.includes(...), which the rule mistook for array membership — rest is a string and this is a substring check (false positive, suppressed inline).
• no-useless-concat: merged adjacent literals, except two DSN fixtures in redact.test.ts — left alone, with a comment, because merging would reassemble the exact secret-shaped literal its author deliberately split to dodge a repo secret scanner.
• no-shadow: renamed the shadowing (inner) binding at every site, never the outer one. Removed 8 redundant await import('./utils/logger.js') calls in index.ts (it already imports log at the top). The two sites the issue specifically called out as real bugs — team-push.ts's destructured dataHomeKey hiding the imported dataHomeKey() function, and dashboard-collector.ts's inner events/stopAt hiding the outer bindings — are both confined to their own block scope with no cross-use today, so the rename is the complete fix; no logic changed.
• no-useless-constructor, unicorn/require-module-specifiers, oxc/no-accumulating-spread: removed two dead pass-through constructors, deleted an export {} placeholder test file (which then tripped unicorn/no-empty-file, so the whole file went — nothing referenced it), and replaced a loop-accumulator object spread with direct property assignment.

Testing

• npx tsc --noEmit — clean.
• npx oxlint --type-aware --deny-warnings --report-unused-disable-directives (the repo's npm run lint) — clean, 0 warnings.
• npm run build — succeeds; ran the built CLI end to end (doctor, env --help, models --help, push --dry-run) against a scratch HOME.
• npx vitest run (full suite, 857 files / 16227 tests) on this branch vs. an unmodified main checkout: 15137 passed / 899 failed vs. 15145 passed / 891 failed. Diffed the exact failing test names between both runs — the differences are generic Windows-environment flakiness (EPERM on temp-dir cleanup, 15s test timeouts under load), reproduced identically against unmodified main. Also reran the 10 most heavily-touched test files in isolation on both branches: byte-identical result both sides (8 failed / 2 passed files, 74 failed / 1300 passed / 12 skipped tests) — confirms no regression from this change.

Type-aware rules kept off (not this PR's scope)

Enabling suspicious/perf via categories also reaches oxlint's type-aware typescript/* rules in those categories, not just the plain ones listed above. Explicitly turned off in .oxlintrc.json because fixing them is issue #863's separate, not-yet-merged scope:

typescript/no-unsafe-type-assertion, typescript/no-unnecessary-type-assertion, typescript/no-unnecessary-type-conversion, typescript/consistent-return, typescript/no-unnecessary-boolean-literal-compare, typescript/no-unnecessary-template-expression, typescript/no-unnecessary-type-arguments, typescript/no-unnecessary-type-parameters.

[github-actions]

The review is blocked: this shallow checkout cannot resolve the requested three-dot diff (fatal: … no merge base), and the base commit’s parent object is missing. A two-dot diff would include unrelated changes, so I won’t treat it as the PR diff.

The supplied PR description meets the testing-record requirement: it records a successful build and real-CLI runs against a scratch HOME.

Please provide the exact three-dot diff or a checkout with sufficient history. No PR code was run, built, or installed.

[github-actions]

Review remains blocked: the requested three-dot diff fails with no merge base. Fetching the exact comparison from GitHub also failed. The earlier review blocker is therefore unresolved; I cannot verify fixes or report code findings.

The supplied PR description meets the testing-record requirement: it documents a successful build followed by real-CLI runs against a scratch HOME.

Please provide the exact three-dot diff or a checkout with sufficient history. No PR code was run, built, or installed.

[github-actions]

No confident findings in the requested three-dot diff.

• The earlier review blocker is resolved: the exact diff now succeeds and was reviewed.
• The PR description satisfies the testing requirement: it records a successful build followed by real-CLI runs against a scratch HOME.
• No PR code was run, built, or installed during this review.

[github-actions]

No confident findings in the requested three-dot diff.

• The earlier checkout blocker is resolved: the exact diff succeeds and was reviewed.
• The PR description satisfies the testing requirement: it records a successful build followed by real-CLI runs against a scratch HOME.
• No PR code was run, built, or installed during this review.

[github-actions]

No confident findings in the requested three-dot diff.

• The earlier checkout blocker is resolved: the exact diff succeeds and was reviewed.
• The PR description satisfies the testing requirement: it records a successful build followed by real-CLI runs against a scratch HOME.
• No PR code was run, built, or installed during this review.

[SaulMoro]

This was generated by AI during triage.

@STiFLeR7 Thanks, this is careful work. Notes for the maintainer, since this changes the lint gate and tsconfig:

• What we checked. The non-test source changes we went through are behavior-preserving: the reverse→toReversed, findLast and Set.has rewrites, the tick.ts accumulating-spread fix, the parse.ts loop scope, the session-cmd.ts renames and the index.ts logger imports. The four no-unmodified-loop-condition disables are real false positives. The ES2023 target is safe: tsup.config.ts sets target: 'node20', so emitted syntax doesn't change. A base-vs-head diff of dist/cli.js shows only renames, { cause } additions and the array-method swaps. A real workflow start run gives the same outcomes on both builds. tsc, lint and build pass, and CI is green on this head, including Windows and all e2e shards.
• [P2 non-blocking] The off list is longer than the body says. Besides the issue's list, .oxlintrc.json turns off 8 typescript/* rules that suspicious enables: no-unsafe-type-assertion (2,548 hits on the base), no-unnecessary-type-assertion (453), no-unnecessary-type-conversion (225), consistent-return (29), no-unnecessary-boolean-literal-compare (25), no-unnecessary-template-expression (8), no-unnecessary-type-arguments (3) and no-unnecessary-type-parameters (2). The config comment points to ci(lint): enable type-aware linting with no-floating-promises and no-misused-promises (#836) #863 for type-aware rules, which is a reasonable scope, but the body should list them, since the maintainer decides the off list from it.
• [P3 nit]
  • src/session-flow/session-cmd.ts:182 and :323 still say "archive at root" after the rename.
  • Three dynamic re-imports are renamed rather than deleted, though each name is already imported statically: init.ts:889, push.ts:1408 and uninstall.ts:1086. The same kind of import is deleted in index.ts.
  • Four nested-catch sites now chain { cause: restoreError }, so the primary write error is only in the message. That's not user-visible.
  • The body's 393 count is 402 on 99fd665d, probably drift.
  • The real-CLI record (--help, push --dry-run) doesn't exercise changed paths. A workflow start run would.
• Conflicts. Merging this conflicts with fix(recall): include active namespaces in digest and maintenance (#1395) #1410 (src/index.ts) and feat(benchmark): run benchmark suites as a default workflow #1379 (src/workflow/engine.ts). With about 100 files touched, it should land quickly once accepted, before it drifts again.

The CI review reports no confident findings. It didn't list the extra off rules.

[github-actions]

No confident findings in the requested three-dot diff.

• The earlier checkout blocker is resolved: the exact diff succeeds and was reviewed.
• The PR description satisfies the testing requirement: it records a successful build followed by real-CLI runs against a scratch HOME.
• No PR code was run, built, or installed during this review.

[SaulMoro]

This was generated by AI during triage.

Re-checked a320ae00, which merges main into the branch.

• What changed since 69d29936. Apart from the conflict resolutions, the new edits apply the same rules to code that landed on main: findLast in autocompact-reconcile.ts and scorers/judge.ts, find replacing a destructured filter in session-flow/session-cmd.ts, guards hoisted out of while conditions in autocompact-reconcile.ts and review-branch.ts, and shadowing renames in deep-enrich.ts, score-drain.ts, scorers/results.ts, session-outcomes.ts, session-flow/scoring.ts, pull.ts and workflow/drivers/local/local-driver.ts. None of them changes behavior.
• Current main. The branch still merges cleanly with main after feat(worktree): one side-branch checkout per self-mode repo (#1443) #1465. On that merge, tsc --noEmit and npm run lint pass, and CI is green on this head.
• Still open from the last pass. The body doesn't yet list the 8 typescript/* rules that .oxlintrc.json turns off beyond the issue's list ([P2 non-blocking]). The nits from that pass are unchanged.
• Conflicts. fix(recall): include active namespaces in digest and maintenance (#1395) #1410 is closed. feat(benchmark): run benchmark suites as a default workflow #1379 is still open and conflicts in src/workflow/engine.ts and src/tool-launch/stdout.ts.

[github-actions]

No confident findings in the requested three-dot diff.

• The earlier checkout blocker is resolved: the exact diff succeeds and was reviewed.
• The PR description satisfies the testing requirement: it records a successful build followed by real-CLI runs against a scratch HOME.
• No PR code was run, built, or installed during this review.

[SaulMoro]

This was generated by AI during triage.

@STiFLeR7 Re-checked 18a5eb6f, which merges main up to #1490.

• [P1 blocking] Lint fails on every Lint & Test job. oxlint --deny-warnings reports src/update.ts:808:11: 'entry' is already declared in the upper scope. (for example Node 22 on ubuntu). That const entry = resolveTeamaiEntryScript() came in with fix(update): skip self-update for a Volta-managed install #1493, after the last merge. Renaming it, as the other shadowing fixes do, clears the job.
• Conflict resolution. The only conflict, in src/__tests__/workflow-skill-transitions.test.ts, keeps main's claim: 'lacking' and this PR's gatedRun rename. Every other file matches an automatic merge.
• Unchanged from the last pass. The body still doesn't list the 8 typescript/* rules that .oxlintrc.json turns off beyond the issue's list ([P2 non-blocking]).

[github-actions]

No confident findings in the requested three-dot diff.

• The earlier checkout blocker is resolved: the exact diff succeeds and was reviewed.
• The PR description satisfies the testing requirement: it records a successful build followed by real-CLI runs against a scratch HOME.
• No PR code was run, built, or installed during this review.

[SaulMoro]

This was generated by AI during triage.

@STiFLeR7 Re-checked 83df3d3a. Everything we raised is resolved.

• Lint. 83df3d3a renames the shadowing entry in src/update.ts to updatedEntry, with no other change. Every Lint & Test job and every E2E shard passes on this head.
• Body. It now lists the 8 typescript/* rules that .oxlintrc.json turns off, and says why.
• CI review. No findings. We agree.

The earlier nits are unchanged and optional. @jeff-r2026 this is ready for your review.

[jeff-r2026]

Verified 83df3d3 (contains main 299ce8e): tsc, lint (0 warnings) and build pass; the unit suite fails the same 33 root-only cases as main 299ce8e and nothing else. A built-CLI run (init over an insteadOf URL, status, pull delivering a skill, doctor) passes. A review of the 73 changed src/ files found no runtime behavior change beyond added error causes. Thanks, approving.