lint: flow-update-readonly-when-field skips runAs:'system' flows entirely, but the conditional strip has no isSystem exemption

Found while correcting that rule's hint text in #13832. Not folded into that PR — #13832's triage fenced it to message text only (「⛔ rule id / severity / match set 一律不动」), and this is a match set question.

The defect

packages/lint/src/validate-readonly-flow-writes.ts returns early for any elevated flow:

// `runAs` defaults to 'user' (schema default). Only an explicit 'system'
if (flow.runAs === 'system') return;

That skip is correct for the static readonly branch: the engine's stripReadonlyFields runs under if (!opCtx.context?.isSystem), so a runAs:'system' flow legitimately maintains readonly columns — the intended channel, rightly unflagged.

It is not correct for the readonlyWhen branch. stripReadonlyWhenFields is called on the update path with no isSystem guard at all. The engine says so at the call site, in the #9107 note in packages/objectql/src/engine.ts:

isSystem is still NOT an exemption here, unlike the static strip below.

Pinned from both sides:

engine-readonly-when-derived-writes.test.ts — "LOCK 2 — isSystem does NOT exempt a caller-supplied value (Option B stays rejected)".
engine-readonly-strict-writes.test.ts — "covers readonlyWhen too — the arm a trusted (isSystem) caller can still hit".

⇒ A runAs:'system' flow whose update_record node writes a readonlyWhen field is still stripped on records whose predicate is TRUE, and the rule says nothing about it. The one class of flow the author was (until #13832) explicitly told to reach for is the class the rule stops inspecting.

Why the early return is wider than it should be

The skip is a single return at flow level, so it removes the flow from both branches at once. The static branch needs it; the conditional branch does not. Whether the fix is to narrow the early return to the static branch, or to keep the skip and accept the blind spot, is a real scope decision — a runAs:'system' flow writing a readonlyWhen field is a legitimate authoring shape whose write silently vanishes per record state, which is the same silent-no-op class the rule family exists to surface.

Worth weighing against false-positive cost: the finding would be a warning (the conditional branch's existing severity), not a gate, so it does not block a build.

Scope note

Message text for all three readonlyWhen carriers is being corrected in #13832 — that PR leaves this skip exactly as it is and records the gap in a comment beside it, so nothing here is a regression from it. This card is only about the match set.

Generated by Claude Code

Triage (R+89, triage seat, session session_019kDRpB7D2XzVzkaLp57T5D): pm:queue · priority:p2 · domain:devx · type Bug.

Lands in: packages/lint/src/validate-readonly-flow-writes.ts — the flow-level early return on runAs === 'system'. Ruled route: narrow the skip to the static readonly branch only; the readonlyWhen branch inspects runAs: 'system' flows like any other, at the branch's existing warning severity. Rationale: the engine is the authority and both its pins say so (isSystem does not exempt stripReadonlyWhenFields — LOCK 2 in engine-readonly-when-derived-writes.test.ts, and engine-readonly-strict-writes.test.ts), so the rule's current match set contradicts the behaviour it exists to surface, on exactly the flow class authors were told to reach for. Warning-only, so no gate moves and no fixture-corpus pass is owed beyond the rule's own tests; the PR must add the positive case (system flow writes a readonlyWhen field ⇒ warning) and keep the negative (system flow writes a static readonly field ⇒ silent). ⛔ Rule id / severity untouched (the #13832 fence stays).

Size/model suggestion: S, opus.

Generated by Claude Code

Claim:

Session: session_01WLJQhde67SeTccsmnBVarV (devx execution seat, seat post [PM seat] domain:devx @ objectstack — 🟢 os-warren · session_012yK1ddAdAqfzwigRHZq3r2 #6023)
Branch: claude/issue-14201-readonly-when-system-flows
Worktree: dedicated per-task worktree off origin/main (dev creates it)
Domain: domain:devx
File surface: packages/lint/src/validate-readonly-flow-writes.ts + its test file; a .changeset/*.md (patch). ⛔ Rule id / severity untouched (the lint: the readonlyWhen hints on the hook and flow rules recommend elevation, which the engine's conditional strip does not honour #13832 fence stays). ⛔ Never content/docs/releases/**.
Container & model: mode:subagent, claude-opus-5. Tier readout (node scripts/pm/dispatch-gates.mjs --tier packages/lint/src/validate-readonly-flow-writes.ts): "no path-derived mandate … floor sonnet · default opus · ceiling fable." PM judgment: default opus.
Clause-②: not engaged (author-time lint warning; no spec contract accept/reject change; no public surface widened).
Serial constraints: none — no other in-flight devx card touches this file. Ruled route from triage is binding: narrow the runAs === 'system' early return to the static readonly branch only; readonlyWhen branch inspects system flows at its existing warning severity; add the positive case and keep the negative.

Generated by Claude Code

os-dev-report

{
  "issue": 14201,
  "status": "done",
  "branch": "claude/issue-14201-readonly-when-system-flows",
  "pr": "https://github.com/objectstack-ai/objectstack/pull/14370",
  "head_sha": "8b99b5c2dda048511ebde049bb0311052bb6b81b",
  "premise_still_valid": true,
  "premise_evidence": "Verified on origin/main before editing, both halves. packages/objectql/src/engine.ts calls stripReadonlyWhenFields on the by-id update path with NO isSystem guard (the #9107 note at the call site: 'isSystem is still NOT an exemption here, unlike the static strip below'), while stripReadonlyFields twenty lines below runs inside 'if (!opCtx.context?.isSystem)'; the bulk path splits the same way (stripReadonlyWhenFieldsMulti unguarded, stripReadonlyFields guarded). LOCK 2 pin present at engine-readonly-when-derived-writes.test.ts:195. The message-text card (14202) had already landed on this file (b992b1d64), so no hunk overlap: that PR touched hint text, this one touches control flow.",
  "files_changed": [
    "packages/lint/src/validate-readonly-flow-writes.ts",
    "packages/lint/src/validate-readonly-flow-writes.test.ts",
    ".changeset/lint-readonly-when-system-flows.md"
  ],
  "summary": "The flow-level 'if (flow.runAs === system) return' removed an elevated flow from BOTH branches of the rule; it now gates the static readonly branch only ('if (meta.readonly && !isSystemRun)'), so a runAs:'system' flow whose update_record node writes a readonlyWhen field reports the conditional branch's existing warning while a system flow writing a static readonly field stays silent. Rule ids and severities untouched, per the triage ruling. A field declaring both flags falls through to the conditional branch under system elevation, which is the truthful verdict for that write. The rule header's paragraph recording the gap was rewritten to describe the split, and the action sibling validate-readonly-action-writes.ts is cited as the precedent (an action body is system-elevated by design and carries the conditional half only). Hint text was deliberately NOT changed: it already reads correctly for an elevated author, and message text is the other card's fence.",
  "tests": "All at head 8b99b5c2d. pnpm --filter '@objectstack/lint^...' build -> exit 0. pnpm --filter @objectstack/lint test -> exit 0, 93 files, 2720 passed / 5 skipped. pnpm --filter @objectstack/lint typecheck (tsc --noEmit) -> exit 0. Targeted rule file: 24 passed. REVERSE VERIFICATION (predicted direction: RED on the new cases): re-injecting the flow-level return with the rest of the change in place gives 4 failed / 20 passed, the four failures being exactly the new runAs:system cases; restored run is 24 passed. Mutation confirmed on disk before the run - anchor asserted unique (grep -cF = 1), injected marker grep -cF = 1, mutated git hash-object differs from the HEAD blob; restore proved by hash equality with the HEAD blob plus an empty 'git diff HEAD', marker count 0, with an absolute-path trap on EXIT INT TERM. No build leg was owed: the test imports the rule by relative source path, not through dist. ESLint run narrowed with the narrowing measured: this repo's single eslint.config.mjs never enables type-aware linting for any file (no parserOptions.project, no typed rules - stated and measured in that config's header), so no untouched file's verdict can move; eslint --no-inline-config --format json over the diff returned 3 entries - 0 errors / 0 warnings on both .ts files, changeset .md reported ignored (no matching configuration) - i.e. 2 lintable files, both clean. Repo-wide sweep left to CI's Lint and Repo Gates.",
  "commands_run": [
    "pnpm install -> 0",
    "pnpm --filter '@objectstack/lint^...' build (under scripts/pm/os-verify-lock.sh) -> 0",
    "pnpm --filter @objectstack/lint exec vitest run --maxWorkers=2 src/validate-readonly-flow-writes.test.ts -> 0 (24 passed)",
    "ablation: same vitest file with the flow-level return re-injected -> 1 (4 failed / 20 passed), restored -> 24 passed",
    "pnpm --filter @objectstack/lint test -> 0",
    "pnpm --filter @objectstack/lint typecheck -> 0",
    "pnpm exec eslint --no-inline-config --format json (3 changed paths) -> 0",
    "node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack -> 33 families; all 33 run at head 8b99b5c2d: 30 exit 0, 3 exit 3",
    "exit-3 rows (PREREQUISITE NOT MET = NOT MEASURED locally, not findings, each says so in its own verdict text): node scripts/check-test-completeness.mjs (grades a saved turbo run test log CI tees), pnpm check:dual-build-cjs-loads (needs the whole workspace built), pnpm check:type-check-debt (needs the ledgered packages' closure built)",
    "node scripts/pm/check-half-states.mjs -> 0 (report-only; first attempt hit my own 300s harness timeout at exit 124, re-run with a 540s budget returned 0)",
    "node scripts/pm/dispatch-gates.mjs --ran CMDFILE --repo objectstack-ai/objectstack -> 0"
  ],
  "gates_reconciliation": "dispatch-gates --ran at head 8b99b5c2d: 33 derived families accounted for - 33 run, 0 UNRUN. Reconciler counts attempted-and-answered families; by my own reading 30 are green and 3 are NOT MEASURED (the exit-3 prerequisite rows above), all three CI-only locally. The 33 = 21 path-derived + 6 kind-derived (2 reached both ways) + 8 changeset-triggered families that became path-matched once the changeset existed. Derivation asserted with --repo objectstack-ai/objectstack and it held. Outside that number by the script's own accounting: the always-runs tail (33 unconditional CI steps), the unreachable listing (0 here), and the full farm CI runs on the PR.",
  "mcp_calls": "0 - every GitHub read and write went through REST with curl (issue body, PR create, PR body read-back, this comment); no MCP GitHub tool was called.",
  "open_questions": [],
  "out_of_scope_findings": [],
  "follow_ups": [
    "None filed. Two observations for the PM, neither a defect I could evidence: (1) the conditional hint's closing clause reads slightly circularly under an elevated run ('runAs:system strips this field on a locked record exactly as this run does') - still true and still actionable, and message text belongs to the 14202 fence, so I left it; (2) the showcase/CRM/todo corpus grows no new warnings from this change - the only example objects declaring readonlyWhen are showcase_invoice's tax_rate and its parent-scoped line-item fields, and the one runAs:'system' flow touching showcase_invoice writes only status; no scaffold template in create-objectstack or cli declares runAs:'system' or readonlyWhen at all."
  ]
}

Generated by Claude Code

ACCEPT — #14201 landed. PR #14370 (head 8b99b5c2) merged to main as 5c28b88d via the merge queue (added_to_merge_queue 03:58:05Z, 33/33 checks green, zero rework).

Close-out probes executed on origin/main (fresh fetch, throwaway worktree):

packages/lint/src/validate-readonly-flow-writes.ts:164 const isSystemRun = runAs === 'system' and :205 if (meta.readonly && !isSystemRun) — the runAs:'system' exemption now applies only on the static readonly branch; the readonlyWhen branch is inspected for system flows too, as the card asked. Dev-reported ablation: reverting the narrowing reds 4 / greens 20.
Control: 11 runAs hits in the file.

pm:dispatched stripped in this same step. Hot-file lock on validate-readonly-flow-writes.ts released.

Seat: session_01WLJQhde67SeTccsmnBVarV (domain:devx, #6023).

Generated by Claude Code

Navigation Menu

Repository navigation

lint: `flow-update-readonly-when-field` skips `runAs:'system'` flows entirely, but the conditional strip has no `isSystem` exemption #14201

The defect

Why the early return is wider than it should be

Scope note

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

lint: flow-update-readonly-when-field skips runAs:'system' flows entirely, but the conditional strip has no isSystem exemption #14201

Description

The defect

Why the early return is wider than it should be

Scope note

Activity

huangyiirene commented on Sep 2, 2026

claude commented on Sep 2, 2026

claude commented on Sep 2, 2026

claude commented on Sep 2, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

lint: `flow-update-readonly-when-field` skips `runAs:'system'` flows entirely, but the conditional strip has no `isSystem` exemption #14201