Linus review T1 surfaced three real bugs in the B2 Phase 1 shadow
dual-write surface. Fix them in one pass.
1. advanceTaskStatus left task_leases behind on terminal (done /
cancelled / blocked) and "return to queue" (todo / backlog)
transitions. The status->phase trigger advanced tasks.phase but the
lease row lingered as an orphan. dev-mode verifyPhaseConsistency
could see the divergence; production silently drifted. Wrap the
status UPDATE and an explicit DELETE FROM task_leases in a single
transaction so no caller observes the intermediate state.
2. The no-restricted-syntax guard in eslint.config.mjs only matched
literals shaped "UPDATE tasks SET status" -- advanceTaskStatus's
dynamic `UPDATE tasks SET ${timestampSets.join(", ")}` slipped past,
and so did saveTask / recoverStaleLocks (status not first column).
Tighten the selectors to (a) match any status-write within UPDATE
tasks regardless of column order, and (b) flag template literals
whose head ends with `SET ` so dynamic helpers must carry an
explicit eslint-disable with rationale. Five grandfathered sites
now carry matching disables.
3. interruptStaleRuns scoped its task+lease cleanup with
`WHERE finished_at = ?` against the freshly stamped timestamp --
safe in isolation, vulnerable to ISO-millisecond ties with other
already-interrupted runs. Capture the stale run ids before
mutating anything and drive every downstream statement from that
explicit id list. Original semantics preserved; scoping is exact.
Unit tests added in tests/unit/tasks-phase.test.ts cover the
advanceTaskStatus lease cleanup across done / cancelled / blocked /
todo, and pin the in_review case as "lease stays".
161 lines
6.1 KiB
JavaScript
161 lines
6.1 KiB
JavaScript
import js from "@eslint/js";
|
|
import globals from "globals";
|
|
import reactHooks from "eslint-plugin-react-hooks";
|
|
import reactRefresh from "eslint-plugin-react-refresh";
|
|
import tseslint from "typescript-eslint";
|
|
|
|
export default tseslint.config(
|
|
{
|
|
ignores: [
|
|
"out/**",
|
|
"**/out/**",
|
|
"build/**",
|
|
"dist/**",
|
|
"oldreference/**",
|
|
"Reference/**",
|
|
"claude-code-source-reference/**",
|
|
"pi-mono-reference/**",
|
|
"boss-build/**",
|
|
"local-release/**",
|
|
"builtin-skills/**",
|
|
"landing/**",
|
|
"node_modules/**",
|
|
"**/node_modules/**",
|
|
".next/**",
|
|
"**/.next/**",
|
|
"test-results/**",
|
|
"playwright-report/**",
|
|
"coverage/**",
|
|
"electron.vite.config.*.mjs",
|
|
],
|
|
},
|
|
js.configs.recommended,
|
|
...tseslint.configs.recommended,
|
|
{
|
|
files: ["src/**/*.{ts,tsx}", "tests/**/*.{ts,tsx}", "scripts/**/*.{ts,mjs}", "packages/**/*.{ts,tsx}"],
|
|
languageOptions: {
|
|
ecmaVersion: "latest",
|
|
sourceType: "module",
|
|
parserOptions: {
|
|
ecmaFeatures: {
|
|
jsx: true,
|
|
},
|
|
},
|
|
globals: {
|
|
...globals.node,
|
|
...globals.browser,
|
|
},
|
|
},
|
|
plugins: {
|
|
"react-hooks": reactHooks,
|
|
"react-refresh": reactRefresh,
|
|
},
|
|
rules: {
|
|
...reactHooks.configs.recommended.rules,
|
|
"react-refresh/only-export-components": "off",
|
|
"@typescript-eslint/no-unused-vars": [
|
|
"warn",
|
|
{
|
|
argsIgnorePattern: "^_",
|
|
varsIgnorePattern: "^_",
|
|
caughtErrorsIgnorePattern: "^_",
|
|
},
|
|
],
|
|
"@typescript-eslint/no-explicit-any": "warn",
|
|
"react-hooks/exhaustive-deps": "warn",
|
|
"react-hooks/incompatible-library": "off",
|
|
"react-hooks/set-state-in-effect": "off",
|
|
"react-hooks/refs": "off",
|
|
"react-hooks/preserve-manual-memoization": "off",
|
|
},
|
|
},
|
|
{
|
|
files: ["tests/**/*.{ts,tsx}"],
|
|
languageOptions: {
|
|
globals: {
|
|
...globals.vitest,
|
|
},
|
|
},
|
|
},
|
|
{
|
|
// C1 part 3/3 — block new top-level methods on AppDatabase.
|
|
//
|
|
// The whole point of the C1 codemod was to force callers through
|
|
// the sub-module surface (`db.tasks.saveTask(...)`) rather than a
|
|
// flat pass-through (`db.saveTask(...)`). Any new method added to
|
|
// `AppDatabase` reintroduces exactly the broken-window pattern we
|
|
// just killed. Allowed methods are limited to
|
|
// `createHireRequest`, `rollbackConfigRevision`, and `queryAll`
|
|
// (the documented multi-module composites + raw-SQL escape hatch).
|
|
// Adding a new method here must either use a different name
|
|
// (then the rule fires and you must justify it with a
|
|
// disable-next-line and a rationale) or go on an existing or new
|
|
// domain module.
|
|
files: ["src/main/database.ts"],
|
|
rules: {
|
|
"no-restricted-syntax": [
|
|
"error",
|
|
{
|
|
selector:
|
|
"ClassBody > MethodDefinition[key.name!=/^(createHireRequest|rollbackConfigRevision|queryAll)$/]",
|
|
message:
|
|
"C1: do not add new methods to AppDatabase. Put the implementation on the owning `database/<module>.ts` and invoke via `db.<module>.<method>(...)`. Only `createHireRequest`, `rollbackConfigRevision`, and `queryAll` are grandfathered; adding another requires a proposal and an explicit eslint-disable.",
|
|
},
|
|
],
|
|
},
|
|
},
|
|
{
|
|
// B2 Phase 1 step 5/5 — block new raw status-writes on tasks.
|
|
//
|
|
// During dual-write every `UPDATE tasks SET status = ...` must go
|
|
// through `advanceTaskPhase` (or the four grandfathered sites in
|
|
// database-base.ts / database/tasks-*.ts / database/runs.ts that
|
|
// already dual-write and carry `eslint-disable-next-line
|
|
// no-restricted-syntax` with a comment explaining the exemption).
|
|
// The no-restricted-syntax rule below intercepts string literals
|
|
// and template literals whose text starts with the offending SQL
|
|
// prefix. New code that needs to mutate `tasks.status` must either
|
|
// route through `advanceTaskPhase` (preferred) or add an
|
|
// `eslint-disable-next-line` with a rationale comment.
|
|
//
|
|
// Migrations are deliberately excluded — schema changes operate on
|
|
// the legacy column by design.
|
|
files: ["src/main/**/*.ts"],
|
|
ignores: ["src/main/database/migrations/**/*.ts"],
|
|
rules: {
|
|
"no-restricted-syntax": [
|
|
"error",
|
|
{
|
|
// Plain string literals: any `UPDATE tasks SET ...status = ...`
|
|
// regardless of column ordering. The previous variant only
|
|
// caught `SET status` as the first column, letting writes
|
|
// like `SET checkout_run_id = ..., status = 'todo'` slip
|
|
// through silently.
|
|
selector:
|
|
"Literal[value=/\\b[Uu][Pp][Dd][Aa][Tt][Ee]\\s+tasks\\s+[Ss][Ee][Tt]\\s[^;]*\\bstatus\\s*=/]",
|
|
message:
|
|
"B2 Phase 1: write task.status through advanceTaskPhase (or tag with `// eslint-disable-next-line no-restricted-syntax` + dual-write rationale for grandfathered sites).",
|
|
},
|
|
{
|
|
selector:
|
|
"TemplateElement[value.raw=/\\b[Uu][Pp][Dd][Aa][Tt][Ee]\\s+tasks\\s+[Ss][Ee][Tt]\\s[^;]*\\bstatus\\s*=/]",
|
|
message:
|
|
"B2 Phase 1: write task.status through advanceTaskPhase (or tag with `// eslint-disable-next-line no-restricted-syntax` + dual-write rationale for grandfathered sites).",
|
|
},
|
|
{
|
|
// Dynamic-SQL bypass: `UPDATE tasks SET ${...}` where the
|
|
// first interpolation is a JS-built column list. The literal
|
|
// head ends with `SET ` (trailing space) and `status` never
|
|
// shows up in the template itself. Match the head-only shape
|
|
// so this helper must carry an explicit eslint-disable with
|
|
// a dual-write rationale, same as the grandfathered sites.
|
|
selector:
|
|
"TemplateElement[value.raw=/\\b[Uu][Pp][Dd][Aa][Tt][Ee]\\s+tasks\\s+[Ss][Ee][Tt]\\s*$/]",
|
|
message:
|
|
"B2 Phase 1: dynamic `UPDATE tasks SET ${...}` helpers bypass the literal-SET guard; route through advanceTaskPhase or tag with `// eslint-disable-next-line no-restricted-syntax` + dual-write rationale.",
|
|
},
|
|
],
|
|
},
|
|
}
|
|
);
|