DevOps: Fix and harden CI pipeline (#5)

[melon-claw]

Resolves #5. Hardens the GitHub Actions workflow by fixing npm lockfile mismatches, replacing system jq with a portable node script for manifest manipulation, and resolving vite/path import issues.

[melon-claw]

Summary

This PR does three things: (1) adds a GitHub Actions CI workflow that runs build on every push/PR, (2) replaces the jq-based clean_prod_manifest script with a portable Node.js CJS script, and (3) adds vite-tsconfig-paths plus a resolve.alias for @ to support absolute imports. There are also a large number of new production dependencies added to package.json (file-type, highlight.js, nanoid, react-markdown, react-window, flat, filenamify, classnames, date-fns-tz) that appear to belong to a separate feature branch and have not been exercised by any code in the diff. The changes are generally correct but have several meaningful gaps.

---

CRITICAL (Blocks Merge)

None. No security vulnerabilities or guaranteed data-loss bugs in the code that was actually written.

---

MAJOR (Blocks Merge)

1. CI uses npm install instead of npm ci — reproducibility guarantee is broken

File: .github/workflows/ci.yml, line 23

npm install will happily update package-lock.json and install versions that differ from what was locked. npm ci is the correct command for automated environments: it enforces an exact lockfile install and fails if the lockfile is out of sync. Using npm install here means your CI build is not verifying what you actually ship — it may silently pull in different transitive dependency versions from run to run.

# Replace:
run: npm install
# With:
run: npm ci

2. clean-manifest.cjs silently eats a missing dist/manifest.json with a crash rather than a meaningful error

File: scripts/clean-manifest.cjs, line 8

fs.readFileSync will throw a raw ENOENT if dist/manifest.json does not exist (i.e., if vite build failed silently or was skipped). The error message will be the raw Node.js system error, not something that tells the developer "run npm run build first." In a CI context this means an obscure crash message rather than a build-pipeline diagnostic.

// Replace the readFileSync line with:
if (!fs.existsSync(manifestPath)) {
  console.error(`[clean-manifest] dist/manifest.json not found at ${manifestPath}. Did 'vite build' succeed?`);
  process.exit(1);
}
const manifest = JSON.parse(fs.readFileSync(manifestPath, "utf-8"));

3. @types/uuid is still in devDependencies but uuid has been removed from dependencies

File: package.json

uuid was dropped from dependencies in this PR in favour of nanoid, but @types/uuid remains as a devDependency. With noUnusedLocals: true in tsconfig.json, this is at minimum a silent pollution of the dependency graph. More concretely, if any remaining import of uuid was missed during the migration it will silently compile against stale types. Audit all uuid imports and remove the @types/uuid devDependency.

4. CI has no lint step — the linter is configured but never runs in CI

File: .github/workflows/ci.yml

package.json defines a lint script (eslint . --ext ts,tsx --report-unused-disable-directives --max-warnings 0). The workflow only runs build. A broken ESLint rule, a no-unused-vars violation, or a React hooks violation will pass CI entirely. The lint step must be added between install and build:

- name: Lint
  run: npm run lint

---

MINOR (Does Not Block)

5. The Playwright test file (tests/export.spec.ts) and its supporting files are untracked but not gitignored

Files: /tests/export.spec.ts, /playwright.config.ts, /extract-state.cjs, /playwright-report/, /test-results/

These are visible in git status as untracked. If they are intended to be part of the repo, they belong in a commit. If they are local development scaffolding, they belong in .gitignore. In the current state, any contributor cloning the repo will see a dirty working tree immediately on checkout.

Notably, extract-state.cjs contains a hardcoded CDP WebSocket endpoint (ws://127.0.0.1:18800/devtools/browser/...). This is a session-specific debugging artefact and has no business being anywhere near the repository.

6. dotenv is imported in tests/export.spec.ts but is not declared in any dependencies or devDependencies

File: tests/export.spec.ts, line 4

import dotenv from 'dotenv';

dotenv is not in package.json. This will fail on a clean install in CI the moment the Playwright tests are wired up. Add it as a devDependency, or use Node's native --env-file flag (Node 20+).

7. The Playwright test uses hardcoded absolute paths that only work on the developer's local machine

File: tests/export.spec.ts, lines 13–15, 25

const extensionPath = '/tmp/discrub-ext/dist';
const userDataDir = '/tmp/test-user-data-dir';
const stateStr = fs.readFileSync('/tmp/discord-state.json', 'utf8');

On any other machine, or in a CI runner where the workspace is not at that exact path, this test will fail with ENOENT. Use path.resolve(__dirname, '../../dist') or a process.env variable, and a tmpdir()-based path for userDataDir.

8. Redundant __dirname polyfill alongside tsPaths() plugin — tsconfig.json is missing paths entries

File: vite.config.ts

vite-tsconfig-paths (tsPaths()) is being used, but tsconfig.json does not declare any paths entries — there is no "@/*": ["src/*"] mapping. The resolve.alias handles the Vite side, but TypeScript itself will not resolve @/ imports unless paths is also declared in tsconfig.json:

// tsconfig.json compilerOptions:
"baseUrl": ".",
"paths": {
  "@/*": ["src/*"]
}

Without this, tsc will emit type errors on any @/ import and block the build. The tsPaths plugin is presently a no-op because there are no paths to read.

9. Nine new production dependencies with no corresponding source usage in this diff

File: package.json

file-type, highlight.js, nanoid, react-markdown, react-window, flat, filenamify, classnames, date-fns-tz, and @types/firefox-webext-browser were all added but zero source files in this diff import them (only nanoid shows any usage in the existing codebase). Dependencies committed without corresponding usage bloat the bundle and supply-chain attack surface for no current benefit. Either keep them in the feature branch where they will be used, or land them in the same commit as the code that exercises them.

---

NIT (Does Not Block)

10. No node-version pinning strategy — "20" resolves to 20.x (latest 20 minor)

File: .github/workflows/ci.yml, line 25

Using node-version: "20" will float across patch and minor releases. Pin to a specific version ("20.19.0" or use .nvmrc / node-version-file:) to ensure reproducibility. This is especially important given that npm install (not npm ci) is also being used — two sources of non-determinism stack.

11. CI job name is build but only runs the build step

File: .github/workflows/ci.yml

Once a lint step is added (see #4), the job name build will be misleading. Consider naming it build-and-lint from the outset.

---

Verdict

Request Changes

The npm install vs npm ci issue (#1) alone is sufficient to block: a CI pipeline that does not enforce lockfile integrity provides a false sense of security. The missing lint step (#4) means the linter configured with --max-warnings 0 never runs in CI, defeating its purpose. Both are one-line fixes. Address those two and clean up the untracked Playwright artefacts (#5), and this is close to mergeable.

Review generated with Claude Code (code-reviewer agent, adversarial mode)