Repository navigation
chore(app): clear lint warnings in GUI files touched by the Effect upgrade - #53098
Merged
Merged
Conversation
# ------------------------ >8 ------------------------ # Do not modify or remove the line above. # Everything below it will be ignored. # # Conflicts: # packages/app/e2e/utils/mock-api.ts # packages/app/e2e/utils/mock-server.ts
Ichinose-Kazuki
pushed a commit
to Ichinose-Kazuki/opencode
that referenced
this pull request
Oct 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
#52868 added a Lint changed files step to the check workflow. It runs
bun run lint:changed HEAD^1, which fails when any GUI package file a PR adds or edits has an oxlint problem, including the warn-level anti-slop rules. #50231 (Effect rc.118) only changes Effect import paths in 31 such files, but those files already had 716 warnings between them, so its check job fails on code it never touched.What Changes
This PR makes exactly those 31 files lint-clean on
v2, so #50231 passes oncev2is merged into it again. It stays on Effect rc.112 and changes no Effect import paths.require-readable-spacingoxlint --fix(blank lines only)no-runtime-typeofPredicate.isString/isNumber/isObject/isFunctionwith the same semanticsno-unknown-parameters/-returns,no-unsafe-dictionary-typecause: unknown(the repo convention); mock fixtures useMockFixture,MockFields,MockSession, andMockProject; request bodies are decoded withSchema.Json/Schema.Recordrequire-safety-comment-for-type-assertion,no-chained-type-assertionsconfig.project as …casts,ipc.ts's fakeElectron.Event); the rest get aSAFETY:commentno-manual-tag-comparison/-tagged-constructionPredicate.isTagged,Option.isSome,SchemaAST.isObjects,Schema.encodeSync(TestEvent)no-conditional-empty-object-spread,no-known-value-wideningSeven files were already clean and are left untouched. About half the hand edits are in
packages/app/e2e/utils/mock-server.ts. Its fixture config is now typed as JSON-like data instead ofunknown, and every e2e spec still typechecks against it unchanged.Targeted suppressions
Following the existing
SAFETY:+ scopedoxlint-disablepattern (for examplegui-extensions/src/sdk/core.tsandapp/e2e/regression/review.spec.ts), these cases keep a justified suppression because the honest fix would change behavior:desktop/src/renderer/ipc-client.tsspeaks Effect's RPC wire format by hand and deliberately keeps the Effect runtime out of the renderer's startup graph. Its plain_tagmessages and comparisons sit in one scoped disable block. The never-readPromise<unknown>dispose callbacks and the trusted reply value also keep suppressions.app/src/runtime/persistence/schema.ts:recoverandmergewalk untyped persisted trees against a schema AST, andwithInitialdecodes the result afterwards.ipc-transport.test.ts, and the twoas unknown asElectron fakes in that test.app/e2e/utils/mock-api.ts:jsonValueis the encoder that turns arbitrary handler output into JSON.Scope
Behavior is unchanged. Production edits only swap equivalent predicates, add comments, or restructure code without changing values (
ipc.tswindow wiring,ssh/controller.tsconfig patch, model selection choices). The e2e mock answers the same responses. Merging this into #50231 conflicts only on adjacent import lines and blank lines. #50231's ownserver.address._tag === "UnixPathAddress"lines needPredicate.isTagged(...)when that merge is resolved, and its new askpass code needsoxlint --fix. A trial merge resolved that way lints and typechecks clean.Verification
CI's production-build e2e passes on Linux and Windows. The only red step is the app component tests, where
browser-pane-restore.spec.tsfails as it already does onv2(fix in #53091). Locally, the full dev-server e2e run left 19 failures (timeline scroll, command palette, composer, and similar). The same 19 also fail with this PR's 24 files reverted tov2, so they come from the local environment, not this change.