HarperFast / HarperFast/harper-pro
oxlint config does not inherit core's no-restricted-imports, so the node:assert/strict ban is unenforced in Pro
- Dominant language
- JavaScript
- Stars
- 3
- Forks
- 0
- Avg merge
- 1d 21h
- Merged PRs (30d)
- 80
Description
`core/.oxlintrc.json` carries a `no-restricted-imports` rule rejecting `node:assert/strict` and `assert/strict`:
```json
{
"name": "node:assert/strict",
"message": "Use plain node:assert; call assert.strictEqual/deepStrictEqual for strict checks (AGENTS.md test style)."
}
```
harper-pro's `.oxlintrc.json` has no such rule — its `rules` block holds seven entries (`no-undef`, `no-unused-vars`, `no-console`, `no-constant-condition`, `prefer-const`, `typescript/no-explicit-any`, `typescript/no-var-requires`) and nothing about restricted imports. So `npm run lint:required` in this repo is clean regardless of which assert module a test imports, and the convention documented in `core/AGENTS.md` is unenforced on the Pro side.
## How it surfaced
A new test file added in #594 imported `node:assert/strict`. Lint was clean locally and in CI; a reviewer caught it by eye. `core/AGENTS.md` is explicit about the rule *and* about the trap — it names `unitTests/security/` and `unitTests/utility/` as legacy directories that still import `/strict` but are "not the target shape", which is exactly where the new file landed, so the neighbouring files read as precedent when they aren't. Fixed in b135ac4, but the next new test file will hit the same gap.
## Why this isn't a one-line config change
Adding the rule as-is fails lint on **42** existing files:
| directory | files |
|---|---|
| `integrationTests/` | 29 |
| `unitTests/` | 9 |
| `smokeTests/` | 2 |
| `stressTests/` | 2 |
So it needs a decision about sequencing rather than just a config edit. Options, roughly in increasing order of effort:
1. **Add the rule scoped to new code only** — oxlint supports per-directory overrides, so the rule could apply to `unitTests/**` while the other trees are exempted, then narrowed further over time. Cheapest thing that stops the bleeding.
2. **Add the rule and migrate all 42 files in one pass.** Mechanical — `assert.equal` → `assert.strictEqual`, `assert.notEqual` → `assert.notStrictEqual`, `deepEqual` → `deepStrictEqual`, leaving `ok`/`match`/`rejects`/`throws` alone (identical on plain assert). Every conversion tightens rather than loosens, so it can't turn a passing assertion into a wrong-but-passing one — but it's a large diff across the integration suite and would want its own PR.
3. **Add the rule at `warn` severity** so it's visible without breaking CI, and let the count drift down.
A related question worth settling in the same pass: whether Pro's oxlint config should inherit from core's generally, rather than the two drifting field by field. This is the instance we noticed; there may be others.
## Note on the docs
The `harper-engineering-guidelines` skill's `rules/testing.md` currently states the opposite of `core/AGENTS.md` ("Assertions: `node:assert/strict`"), which is plausibly why agents keep reaching for it. That lives outside this repo and is being fixed separately, but it's worth knowing that the lint gap and the docs conflict are two halves of the same problem — neither alone explains why this keeps happening.
## Provenance
Found while addressing review feedback on #594. Cross-ref: HarperFast/harper-pro#594, and `core/AGENTS.md` (test style section) for the convention itself.
Contributor guide
Research direction
Start by comparing harper-pro’s .oxlintrc.json with core/.oxlintrc.json and reading the test-style section of core/AGENTS.md. Inventory the 42 existing imports across integrationTests/, unitTests/, smokeTests/, and stressTests/, then run npm run lint:required while evaluating the proposed scoping, migration, or warning approaches. Done means the chosen policy is documented in the config and lint behavior matches it without leaving new test files uncovered.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, nodejs
- Domain
- testing-qa, tooling
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100