Repository navigation
The lint check reports its findings and exits successfully, which is how #63 got through #75
Description
package.json:17 is "lint": "bunx biome lint .", and ci.yml:33 runs it. Biome does not fail a run
for warnings, so the check reports them and succeeds. Measured on main:
$ bunx biome lint .
Checked 338 files in 3s.
Found 25 warnings.
Found 1 info.
$ echo $?
0
Exit codes, measured directly rather than assumed:
| Diagnostics | Exit |
|---|---|
| errors present | 1 |
| warnings only, default | 0 |
warnings only, --error-on-warnings |
1 |
This is not a hypothetical cost, and #63 is the evidence:
Found by Guido Vizoso. Biome had said so too, as an unused import on line 2, which I had waved
through as a warning.
That warning was repairUnansweredToolCalls recursing into its own default parameter until the stack
went — it hung precisely when the function did its job, and the one real caller was the one that
triggered it. The tool reported it. The check went green. The mechanism that allowed that is unchanged.
What is standing today
| Rule | Count | Group |
|---|---|---|
noNonNullAssertion |
15 | style |
noTemplateCurlyInString |
7 | suspicious |
useConst |
1 | style |
noUnusedVariables |
1 | correctness |
noUnusedImports |
1 | correctness |
I opened both correctness ones rather than reporting a count, and neither is a live bug:
agent-langgraph/src/index.ts:431—finalMessageis assigned at:465and never read. The
assembledtool_callsit looks like it should carry are consumed throughpendingin the same
block, so this is a leftover, not a missing feature.server/tests/skill-ownership.integration.test.ts:3—eqimported and unused, in a test.
So there is nothing to be alarmed about on main right now. The point is narrower than that: the two
warnings standing are the same rule family as the one that hung the app, and nothing would tell anyone
if the next one were the load-bearing kind.
The seven noTemplateCurlyInString are false positives — tests asserting on literal ${...} strings,
which is what those tests are for (tests/compose.test.ts:13,
server/tests/tenant-package.test.ts:614). They want suppressing, not fixing.
What I would suggest, narrowly
--error-on-warnings would fail all twenty-five and turn this into a wide mechanical diff across
fifteen ! assertions that are mostly deliberate. That seems like the wrong trade.
Raising only the group that means "this is probably a bug" is surgical. Verified that this form is
accepted by Biome 2.5.8 and that it changes the exit code:
"linter": { "enabled": true, "rules": { "correctness": "error" } }Style and suspicious stay as advice. correctness fails the build. The cost is the two-line fix above.
Whether that is wanted is a judgement about what other contributors should have to clear, which is
yours rather than mine — this is the only one of my recent findings that changes what fails for
everybody else, which is why it is an issue and not a PR. Happy to send the three-line change if you
want it, happy to close this if the current setting is deliberate.
Related but separate, and already raised where it belongs: the typecheck script in agent-computer
never ran either, which #65's CI hunk fixes; I noted the supervisor half on that PR rather than here.
Activity
fix: fail the lint check on the warnings it reports (#75) (#120)