DevOps: Harden CI pipeline and address review feedback

[melon-claw]

This PR hardens the CI pipeline by switching to , adding a step, pinning the Node version to 20.18.0, and providing a portable for the runner environment.

[melon-claw]

Adversarial Review — Request Changes

This PR claims to harden CI. It simultaneously gitignores the test suite, commits hardcoded credential paths and a raw socket path into test code, introduces a duplicate JSON key that silently corrupts TypeScript config, and adds a dependency whose only consumer is code that cannot be reached by CI. The CI changes themselves are cosmetic. The substantive changes range from broken to a security liability.

---

CRITICAL — Must Fix Before Merge

1. tests/export.spec.ts:31 — Hardcoded /tmp/discord-state.json with no existence check

const stateStr = fs.readFileSync('/tmp/discord-state.json', 'utf8');

This file is expected to hold live Discord session state (cookies, localStorage tokens). Problems:

• No fs.existsSync guard — crashes with unhandled ENOENT if the file is absent.
• Reads from a world-writable path (/tmp) with no integrity check — on a shared machine an attacker could pre-stage a malicious file here.
• There is no documentation of what must be in this file, who creates it, or when.

Fix: At minimum add an existence check and fail with an actionable message. Long term, this credential injection pattern must be documented, gated behind an explicit CI_INTEGRATION=1 env flag, and kept out of /tmp.

---

2. tests/export.spec.ts:9-11 — Developer laptop credential path committed to source

const envPath = path.resolve(process.env.HOME || '', '.openclaw', '.env');
if (fs.existsSync(envPath)) {
  dotenv.config({ path: envPath });
}

$HOME/.openclaw/.env is a machine-specific, undocumented convention for injecting Discord credentials. This is in committed code. Issues:

• New contributors have no way to discover what this file must contain.
• CI has no equivalent, so tests silently skip credential loading and will fail with opaque Discord auth errors instead of "missing required env vars."
• Silent skip on missing file is an antipattern for required configuration.

Fix: Document the required env vars (e.g. DISCORD_TOKEN, TEST_CHANNEL_ID). Fail fast and loudly when they are absent rather than silently proceeding to a runtime auth failure.

---

MAJOR — Should Fix Before Merge

3. tsconfig.json — Duplicate baseUrl and paths keys (lines 7–9 and 32–35)

The PR adds baseUrl/paths near the top of compilerOptions. The file already had the same two keys at the bottom. The resulting JSON has two "baseUrl" and two "paths" keys in the same object. The JSON spec declares this undefined behaviour; TypeScript's parser silently keeps the last occurrence, making the first block dead. Anyone reading this file cannot determine which block is effective.

Fix: Delete the duplicate block at lines 7–9. Keep only the commented /* Path aliases */ block at lines 32–35.

---

4. .gitignore — Gitignoring already-tracked files has no effect; leaves repo incoherent

/tests/
/playwright.config.ts
/extract-state.cjs

These paths are already tracked in the index (tests/export.spec.ts and playwright.config.ts both exist in the working tree). Adding them to .gitignore after tracking does nothing — git status will continue to show them as modified, and a fresh clone will still materialize them. The files are simultaneously "in the repo" and "gitignored."

Fix: Decide. Either run git rm --cached tests/ playwright.config.ts extract-state.cjs and document where these files live, or remove them from .gitignore and integrate the tests properly into CI.

---

5. package.json — dotenv added as devDependency with no reachable consumer in CI

dotenv is imported only in tests/export.spec.ts. The CI pipeline does not run Playwright tests. The test directory is gitignored (issue 4). This dependency is dead weight in CI and evidence that issues 4 and 5 were not reconciled before merge.

Fix: Either remove dotenv from package.json (and fix the test infrastructure) or un-gitignore the tests and have CI run them with proper secrets management.

---

MINOR — Nice to Fix

6. .github/workflows/ci.yml — No permissions block; default token holds contents: write

A build-and-lint job needs no write access. Without an explicit permissions: block, the default GITHUB_TOKEN grants contents: write. A compromised transitive action could push to the repository.

Fix:

permissions:
  contents: read

---

7. .github/workflows/ci.yml — Actions pinned by mutable tag, not commit SHA

uses: actions/checkout@v4
uses: actions/setup-node@v4

@v4 is a floating, mutable tag. A compromised or malicious push to that tag would silently affect all future runs. For a PR explicitly about hardening CI, this is a glaring omission.

Fix: Pin to full commit SHAs, e.g.:

uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683  # v4.2.2
uses: actions/setup-node@39370e3970a6d050c480ffad4ff0ed4d3fdee5af  # v4.1.0

---

8. .eslintrc.cjs:13 — Rule severity 'warn' contradicts --max-warnings 0

'react-refresh/only-export-components': ['warn', { allowConstantExport: true }]

The lint script uses --max-warnings 0, meaning any warning fails CI. Setting the rule to 'warn' while the runner treats it as an error is misleading: contributors see a "warning" locally and assume it's non-blocking. It isn't.

Fix: Set the rule to 'error' to match the effective behaviour, or remove --max-warnings 0 and justify why.

---

9. vite.config.ts — resolve.alias for @ is redundant with tsPaths() plugin

resolve: {
  alias: { "@": path.resolve(__dirname, "src") },
},
// ...
plugins: [react(), tsPaths(), ...]

tsPaths() reads tsconfig.json paths automatically and resolves @/* at build time. The manual resolve.alias duplicates this. Two sources of truth for the same alias can silently diverge if one is updated and the other is not.

Fix: Remove the resolve.alias block and rely solely on tsPaths().

---

Verdict

Changes required. Issues 1–2 are security concerns (credential handling via world-writable path and hardcoded developer machine paths). Issue 3 is a silent compiler misconfiguration. Issues 4–5 leave the repository in an incoherent state with respect to its own test infrastructure. Issues 6–9 undermine the stated goal of hardening CI. The Node version pin is the only unambiguous improvement in this PR.