Add anti-slop linting and repository cleanup - #190
Merged
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
milind-soni
marked this pull request as ready for review
August 17, 2026 16:40
mnthr7
pushed a commit
to mnthr7/OpenMausMobile
that referenced
this pull request
Aug 17, 2026
One commit landed upstream since the last sync: milind-soni#190, which adds an oxlint "anti-slop" ruleset, deletes the checked-in dist-server/ build output, and gitignores it. The only conflict was .gitignore — this branch ignores dist-companion, upstream now ignores dist-server. Keep both. The dist-server/ deletions ride along in the merge, which incidentally satisfies this PR's own checklist line about never editing that directory. Note for later: `pnpm lint` is not wired into CI and the existing server/ code does not pass it either, so this merge takes the ruleset as-is without attempting to lint the companion code against it. Verified on the merged tree: typecheck, full suite (73 files, 622 passed, 8 skipped), check:electron, build:companion, production UI build. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ACfMX71nKJyzHU5Z3by3dd
mnthr7
added a commit
to mnthr7/OpenMausMobile
that referenced
this pull request
Aug 17, 2026
Both sides added a build-output ignore — dist-companion here, dist-server upstream (milind-soni#190, which also stopped tracking it) — so .gitignore takes both. Everything else merged clean.
milind-soni
added a commit
that referenced
this pull request
Aug 17, 2026
The smoke test assumed dist-server was already on disk. It is gitignored (#190), so a fresh CI checkout has never built it and the test died on ENOENT before it could prove anything. It passed locally only because a build happened to be sitting there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
milind-soni
added a commit
that referenced
this pull request
Aug 17, 2026
* Bundle the server's dependencies so the packaged app can start
0.1.24 built, signed, notarized and installed cleanly, then died on
every launch:
ERR_MODULE_NOT_FOUND: Cannot find package 'zod'
imported from Resources/server/config.js
The packaged app ships no node_modules by design (electron-builder.yml
line 21 calls the three pieces self-contained, line 33 excludes them).
build:server was plain tsc, which transpiles without bundling, so the
`zod` import #194 introduced survived verbatim into a tree with nothing
to resolve it against. zod was the first bare import the server ever
had, so the invariant had never been tested.
Bundle every entry point with esbuild after tsc, mirroring
scripts/bundle-updater.mjs which already vendors electron-updater for
the same reason. All six are bundled, not just index.ts: the proxies run
as their own processes and import nothing external today, but the next
one that does would fail the same silent way. Entry points keep their
relative paths, which the proxy lookups depend on.
Both gates that should have caught this were blind to it. The unit suite
runs inside the repo, where a bare import resolves from ./node_modules;
the Windows packaging check asserts index.js exists but never runs it.
So the new smoke test copies dist-server OUT of the repo before starting
it, and CI now starts the real packaged copy on the runner. Verified by
mutation: reverting to plain tsc output fails the smoke test with the
original ERR_MODULE_NOT_FOUND.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Build the server before smoke-testing it
The smoke test assumed dist-server was already on disk. It is gitignored
(#190), so a fresh CI checkout has never built it and the test died on
ENOENT before it could prove anything. It passed locally only because a
build happened to be sitting there.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Never let scratch cleanup fail the smoke test
Windows holds file handles briefly after the process that owned them
dies, so removing the scratch dir right after the kill raised EPERM and
failed a run whose server had actually started fine. Linux raises EACCES
the same way (f66d30f).
Cleanup is housekeeping; the assertion is the test. Mutation re-checked:
plain tsc output still fails.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
What changed
pnpm lintandpnpm cleancommands.dist-serveroutput and ignore generated/agent worktree paths.Why
The repository had no anti-slop lint boundary, generated server output was committed, and several type annotations discarded useful evidence. This establishes a visible migration baseline and removes generated-file noise from reviews.
Impact
No user-facing behavior change is intended. The lint command is intentionally not a CI gate yet: the first current-
mainaudit found 724 errors and 23 warnings; this pass reduces that to 635 errors and 23 warnings. The remaining boundary parsing and assertion work should be migrated incrementally.Validation
pnpm typecheckpnpm test -- --reporter=dot— 527 Vitest tests and 12 updater tests passed; 8 skippedpnpm check:electronnode --check scripts/clean.mjspnpm audit --prod— no known vulnerabilitiesgit diff --check