Automate a review finding as a lint or test: Clean up: see the line comments. #10692

This pull adds one review rule to the gate, covering one of the repeated findings, and renames the four names that broke it. The other findings are still unautomated, so the issue is only partly done. bun scripts/gate.ts and bun x tsc --noEmit are clean. test/review-rules.test.ts passes, along with test/live.test.ts and test/backups.test.ts. I did not run the full suite, and I did not run design-lint, though backups.ts is under src/web/.

The rule. reviewProblems in src/kernel/review_rules.ts now also refuses a const, let, var or function in src/ whose name ends in an underscore. This is sentinel's split_ finding on #9183. The failure reads rule "a name does not end in an underscore": src/x.ts:12 declares split_; name it for what it holds. Vendored code is skipped. reviewRefusal in scripts/gate.ts now reads src/ as well as test/, so bun scripts/gate.ts and the forge's check both run it.

Renames. Behaviour is unchanged.

• said_ is now wrote in src/actions/proposals.ts.
• self_ is now handle in src/system/live.ts.
• remember_ is now rememberView in src/tui/screens/workspace.ts.
• field_ is now sealedField in src/web/pages/backups.ts.

Tests. A new case in test/review-rules.test.ts checks that the rule fires, names itself, gives the file and line, and skips vendored code. The existing "repository passes the rules" test now covers src/ too.

What I left out.

• Named imports in alphabetical order (#9901): 1,708 existing imports in 935 files break it, so a check would go red on day one.
• No ! non-null assertions (#9183): 474 uses today.
• The comments about copy and design principles (#10595, #9901), the double-read finding (#10307) and test-coverage asks like "drive it through readLandings" need judgment, so I did not turn them into lints.

I couldn't read the line comments through the API, so I worked from sentinel's comment bodies on each pull's issue.

Try it: bun scripts/gate.ts. To see the rule fire, add const x_ = 1; to any file in src/ and run it again.

🤖 Generated with Claude Code

Closes #10603

This pull adds one review rule to the gate, covering one of the repeated findings, and renames the four names that broke it. The other findings are still unautomated, so the issue is only partly done. bun scripts/gate.ts and bun x tsc --noEmit are clean. test/review-rules.test.ts passes, along with test/live.test.ts and test/backups.test.ts. I did not run the full suite, and I did not run design-lint, though backups.ts is under src/web/.

The rule. reviewProblems in src/kernel/review_rules.ts now also refuses a const, let, var or function in src/ whose name ends in an underscore. This is sentinel's split_ finding on #9183. The failure reads rule "a name does not end in an underscore": src/x.ts:12 declares split_; name it for what it holds. Vendored code is skipped. reviewRefusal in scripts/gate.ts now reads src/ as well as test/, so bun scripts/gate.ts and the forge's check both run it.

Renames. Behaviour is unchanged.

• said_ is now wrote in src/actions/proposals.ts.
• self_ is now handle in src/system/live.ts.
• remember_ is now rememberView in src/tui/screens/workspace.ts.
• field_ is now sealedField in src/web/pages/backups.ts.

Tests. A new case in test/review-rules.test.ts checks that the rule fires, names itself, gives the file and line, and skips vendored code. The existing "repository passes the rules" test now covers src/ too.

What I left out.

• Named imports in alphabetical order (#9901): 1,708 existing imports in 935 files break it, so a check would go red on day one.
• No ! non-null assertions (#9183): 474 uses today.
• The comments about copy and design principles (#10595, #9901), the double-read finding (#10307) and test-coverage asks like "drive it through readLandings" need judgment, so I did not turn them into lints.

I couldn't read the line comments through the API, so I worked from sentinel's comment bodies on each pull's issue.

Try it: bun scripts/gate.ts. To see the rule fire, add const x_ = 1; to any file in src/ and run it again.

🤖 Generated with Claude Code

This push fixes the one failing check, "every destructive call in src/tui/screens asks first or is on the list". The rest of the pull is unchanged.

The failing line was src/tui/screens/devices.ts:122. It hands revoke to askApp as a callback. askApp opens a prompt, and revoke runs only when the admin submits an app slug. The scan can't see through that callback, so it reported a destructive call that runs unasked.

I added a devices.ts / revoke entry, with its reason, to the allowed list in test/tui-asks-first.test.ts. The code in devices.ts is unchanged. I did not add a y/n confirm before the revoke. The typed slug already makes the admin name the app, but a y/n would be stricter. Say so if you want it.

bun test test/tui-asks-first.test.ts passes locally (7 tests). I did not re-run the gate, tsc or the rest of the suite for this change. The forge's check will run the full suite on this push.

This failure came from devices.ts on main, from the printer-grant pull (#10671). It is unrelated to the lint rule this pull adds.

This pull adds one review rule to the gate, "a name does not end in an underscore" (#9183, split_), and renames the four existing names that broke it. Your second request, a second rule or follow-up issues, is not done; see the last section.

The rule lives in src/kernel/review_rules.ts and applies to src/, skipping vendored code. The gate prints the rule's name, the file and the line when it fails. The renames are behaviour-neutral.

Changes from your review

• Revoke asks first. src/tui/screens/devices.ts now asks y/n before r revokes an app's printer access. After you type the slug, a confirm shows the app and the printer. y revokes and n keeps. Granting access is unchanged.
• Allowlist entry dropped. test/tui-asks-first.test.ts is back to how it was before my first push, with no revoke entry. The guard passes because the code now asks.
• Tests. test/tui-devices.test.ts now checks that a revoke does nothing until y, and that n keeps the grant.
• Rule scope. The comment in review_rules.ts now says the underscore rule scans only names declared with const, let, var or function. Parameters, destructured names and class members are not scanned, because a regex can't tell them from a type or a call.

I ran scripts/gate.ts, tsc and the device and review-rule tests locally, and they pass.

Not done: a second rule The issue's four source comments (#9183, #9901, #10307, #10595) only say "see the line comments", and I can't see those comments. #9183 is the underscore rule. I didn't write a rule for the other three without seeing what they asked for.

The non-null ! ban on newly added lines needs a git diff, and the gate only reads files, so it's a separate change. This pull doesn't close the issue's first box. A person should keep the issue open, or file follow-ups per finding once they're readable.

🤖 Generated with Claude Code

Left #10603 open: 1 unticked item remain (A lint (scripts/) or a test that fails on exactly what these comments ask for, so a reviewer need not say it again.).