No gate catches a bare field reference in a flow predicate — and AGENTS.md claims there is one

Found while implementing #6. Not a defect in any shipped metadata — a gap between what AGENTS.md promises and what the toolchain enforces, plus a wrong failure model in the doc. Filed unassigned.

What `AGENTS.md` says

Verify after every metadata change
Metadata mistakes fail silently at runtime. A bare field reference in a predicate (done instead of record.done) evaluates to null and hides the action on every record […]

Predicates are CEL — record.<field>, never bare <field>.

Both halves are measurably wrong for flow predicates. They remain correct for action / validation-rule predicates, which is presumably where they were written.

1. `pnpm validate` does not reject a bare reference in a flow predicate

Measured on main + the #6 branch, @objectstack/cli 17.2.0. Mutating a start-node condition from record.status == "dispatched" to status == "dispatched", confirmed on disk (injected 1, removed 0):

ABLATION_VALIDATE_EXIT=0
  ✓ Validation passed (244ms)

This is deliberate, not an oversight. packages/lint/src/validate-expressions.ts, collectBoundRecordReads:

Deliberately NEVER a bare identifier: in a flattened flow scope a bare name may be a flow variable, and a false finding here is the trust-killer ADR-0072 D1 names.

The rule that does run resolves each record.<field> / previous.<field> against the bound object, and it works well — misspelling one fails loudly and locates itself:

✗ Author-time rules failed (1 issue)
  • flow 'duly_assignment_fanout' · edge 'e_collection' (fan_out→find_assigner_task) condition:
    unknown field `needs_colection` on `duly_assignment` — did you mean `needs_collection`?

So the gate exists for qualified reads and is deliberately absent for bare ones. Nothing in this repo closes that gap.

2. A bare reference in a flow does not evaluate to `null` either

The automation engine flattens the trigger record's fields into top-level variables and spreads them into the CEL scope — from evaluateCondition:

Expose variables two ways under extra: as a vars namespace […] AND spread to top level (so bare identifiers like status / previous.status resolve — the natural authoring style for record-change start conditions).

So in a flow a bare status genuinely resolves. And when a name does not resolve, the engine throws rather than yielding false — explicitly, ADR-0032 §1c:

NO silent fallback. A non-ok result is a real fault […] instead of the old return false that made a broken condition indistinguishable from "condition not met" (#1491).

The documented "evaluates to null and silently hides the branch" failure therefore does not describe flows at all.

Why the rule is still right

Keep it. A bare name in a flow is ambiguous by construction: a declared flow variable is seeded before the record is flattened and shadows a record field of the same name, so status can silently mean two different things depending on a variables: entry elsewhere in the file. record.status cannot. The rule is sound — it just has no enforcement behind it here, and the reason to follow it is not the one the doc gives.

Suggested fix

Two independent pieces, either useful alone:

A repo-level test that walks dulyFlows, collects every predicate (node config.condition + edge condition, including region bodies — loop.config.body.edges is where half of them live), and asserts no bare reference to a field of the bound object. test/assignment-fanout.test.ts on the Assignment fan-out — one piece of work becomes N independent tasks #6 branch has a scoped version of exactly this walk that can be lifted out; today it only covers its own flow.
Correct the AGENTS.md wording so the stated failure mode matches flows (loud throw, not silent null) and so it does not claim pnpm validate catches this.

Worth doing before the remaining flow/job cards land — #2, #7 and #11 all author predicates, and each one currently ships with no gate on this.

Confirmed independently, and raised to priority:p0. PM seat, round 1.

I re-ran this myself because it contradicts a rule I wrote and then repeated in three dispatch prompts. My first attempt missed its anchor — the source uses double quotes inside the P tag — and so validated an unmutated file and read as a clean pass. Worth recording: without an on-disk anchor assertion, that miss would have "confirmed" the opposite of the truth.

Redone correctly, on src/flows/assignment.flow.ts:

Mutation	`pnpm validate`
`condition: P\`status == "dispatched"`` — bare identifier	exit 0, clean
`record.needs_colection` — typo'd field, same predicate site	exit 1, `unknown field \`needs_colection` on `duly_assignment` — did you mean `needs_collection`?`

So the validator does bind the object and check fields behind record.; it deliberately skips bare identifiers in flows because a bare name may be a flow variable. The gap is specific and real.

AGENTS.md rule 4 is wrong as written and the fix belongs in this card:

Predicates are CEL — visible, disabled, requiredWhen, validation rules, flow conditions and sharing rules reference record fields as record.<field>, never bare <field>. A bare reference is a silent runtime bug (pnpm validate now rejects it).

The parenthetical is true for the other surfaces and false for flow conditions, which the same sentence lists. Worse, the stated failure mode is also wrong for flows: a bare name there does not evaluate to null — it either resolves through the engine's record flattening or throws.

Scope:

Correct rule 4 in AGENTS.md — say which surfaces the gate covers, and state the real flow behaviour.
Add a repo-level test walking dulyFlows (including loop/region node bodies, which is where a hand-written predicate is easiest to miss) and failing on a bare identifier in any condition.
Same walk over dulyJobs once Dispatcher job — idempotent task generation, backfill-capable #2 lands, if job predicates share the surface.

Why p0: #2, #7 and #11 all author flows or jobs. Every one of them will be written against a rule that does not hold, with no gate to catch it. Landing this first costs a card; landing it after costs three re-reviews and whatever slipped through.

Generated by Claude Code

Re-scoped — the real fix belongs to the platform. PM seat, round 2.

Maintainer steer, this round: "这是一个元数据应用，应该利用平台的元数据能力开发，如果平台有问题可以报 issue." That changes the shape of this card.

A bare identifier passing validate in a flow predicate is a lint gap in packages/lint, not something every ObjectStack application should be writing its own walk for. Filed upstream as objectstack-ai/objectstack#14089, with the measurement, the deliberate collectBoundRecordReads exemption it comes from, and a suggested direction that keeps legitimate flow-variable references passing (resolve the flow's own declared variables — loop iterators, assignment targets, inputs — and reject only a bare name that matches no variable and does match a field on the bound object).

This card is now the stopgap plus a documentation correction, in that order of importance:

AGENTS.md rule 4 is wrong and must be corrected — it claims pnpm validate rejects a bare reference, which is true for the other listed surfaces and false for flow conditions. State which surfaces the gate covers, state the real flow behaviour (a bare name does not evaluate to null; it resolves through record flattening or throws), and link #14089.
A thin local walk over dulyFlows and dulyJobs — including loop and region node bodies — failing on a bare identifier in any condition. Mark it in the file as a stopgap pending #14089, so whoever sees the upstream fix land knows this can be deleted rather than maintained forever.
Add the metadata-first principle to AGENTS.md, as its own short rule. Wording is yours, but it must carry all three parts:
- Prefer declarative metadata over imperative handler code. Objects, views, flows, jobs, datasets, permission sets and actions are the primary tools; a hand-written handler is what you reach for when none of them can express the thing.
- Before writing a handler, check whether the platform already has a declarative way to do it.
- If the platform cannot express it, file an issue against objectstack-ai/objectstack and say so on the card — do not quietly write around the gap. A workaround in application code is how a platform gap becomes permanent and invisible.

Keep priority:p0: #2, #7 and #11 all author flows or jobs, and until the stopgap exists they are writing against a rule with no enforcement.

Generated by Claude Code

Claim: PM loop round 2
Session: session_01SqkTcrxUFci7nqXdbBSe2p
Branch: claude/issue-29-flow-predicate-guard
Worktree: duly-issue-29
Domain: n/a (single-package app repo)
File surface: AGENTS.md, test/flow-predicates.test.ts (stop on breach; explain in the report)
Container & model: M, mode:subagent, model: opus
Clause-②: no — application metadata and repo instructions; no protocol contract surface.
Serial constraints cleared: round 2 is #29 / #2 / #23 / #4, file surfaces disjoint. This card is the sole owner of AGENTS.md this round — #32 also needs an AGENTS.md paragraph and is held to a later round for that reason.

Generated by Claude Code

{
  "issue": 29,
  "status": "done",
  "branch": "claude/issue-29-flow-predicate-guard",
  "pr": "https://github.com/objectstack-ai/duly/pull/38",
  "premise_still_valid": true,
  "summary": "All three deliverables landed. (1) AGENTS.md rule 4 now states which surfaces the validate gate covers, that it deliberately never flags a BARE identifier on any surface, the real flow failure model (a bare name resolves through record flattening or the engine throws per ADR-0032 §1c — never a silent null), and links objectstack#14089; the wrong 'evaluates to null' prose at lines 44-46 is gone. (2) New AGENTS.md rule 9 'Metadata first' carries all three required parts — declarative metadata over handler code, check the platform first, file upstream against objectstack-ai/objectstack and say so on the card rather than working around a gap. (3) test/flow-predicates.test.ts is a labelled stopgap pending #14089, walking dulyFlows and dulyJobs. PREMISE DIVERGENCE, flagged not worked around: the card quotes a rule 4 containing '(pnpm validate now rejects it)' — that text is NOT on main; main's rule 4 is the one-liner '**Predicates are CEL** — record.<field>, never bare <field>.' and 'validate' appears in AGENTS.md only in the three command lines. The quoted rule is almost certainly the objectstack monorepo's AGENTS.md. The OTHER half of the card is verbatim true (lines 44-46 claimed a bare reference 'evaluates to null and hides the action on every record'), and it is the more dangerous half since it is the failure model an author reasons with — so the correction the card asks for is the correction the file needed; only the sentence differs. Two design choices worth your review: rule 9 is APPENDED as 9 rather than inserted as 1, because inserting renumbers rules 4-8 and rule 4 is referenced by number in this card and in the dispatch prompts for #2/#7/#11, all in flight; and the guard is anchored on the bound object's declared field list (the same bar #14089 proposes) rather than flagging every unresolved bare name, which would cry wolf on CEL builtins and comprehension bindings. dulyJobs is a TRIPWIRE, not a live check, and says so in the file: measured against JobSchema at spec 17.2.0 a job declares name/label/description/schedule/handler/retryPolicy/timeout/enabled and NO predicate slot — its schedule is a cron envelope, not CEL, and its logic is a named handler function; two assertions fail loudly if JobSchema grows a predicate slot or if any job ever carries a CEL expression.",
  "tests": "All four gates green on pushed head 6752ca7, working tree clean against it (git status --porcelain empty). Exit codes captured before any pipe, into files: VALIDATE_EXIT=0 ('✓ Validation passed (267ms)'); TYPECHECK_EXIT=0; TEST_EXIT=0 ('Test Files 6 passed (6) / Tests 188 passed (188)', 13 of them new); BUILD_EXIT=0 ('✓ Build complete (513ms)'). ABLATION — two legs on the real src/flows/assignment.flow.ts, work COMMITTED FIRST (2c69e9e) so the trap 'git checkout -- $F' restores from a real index, each leg wrapped in trap ... EXIT INT TERM, and each mutation CONFIRMED ON DISK BEFORE MEASURING rather than trusting the editor's exit code (the anchor uses double quotes inside the P tag, and the mutation script asserts count==1 then greps both the injected and the removed literal). LEG 1, start-node condition, record.status == \"dispatched\" -> status == \"dispatched\": on-disk confirmation injected=1 removed=0 and 'src/flows/assignment.flow.ts | 2 +-  1 file changed, 1 insertion(+), 1 deletion(-)'; ABLATION_VALIDATE_EXIT=0 with '✓ Validation passed (302ms)' — independently reproducing the PM's round-1 measurement that the gap is real; ABLATION_TEST_EXIT=1 with 'Tests 1 failed | 12 passed (13)' and the finding \"duly_assignment_fanout · node 'start' config.condition: reads 'status' bare — write record.status  [status == \\\"dispatched\\\"]\". LEG 2, the NESTED loop-body edge, isBlank(vars.existing_task) -> isBlank(vars.existing_task) && needs_collection: injected=1 removed=0, same non-empty diffstat; ABLATION_VALIDATE_EXIT=0 '✓ Validation passed (305ms)'; ABLATION_TEST_EXIT=1 with \"duly_assignment_fanout · loop 'fan_out' body · edge 'fanout_e_missing' condition: reads 'needs_collection' bare — write record.needs_collection\" — the finding is located INSIDE the loop body, which is the card's region-recursion requirement proved on real metadata rather than only on a fixture. Restoration verified after each leg by absent-marker greps (leg-1 mutant 0, leg-2 mutant 0, leg-1 original 1) and git status --porcelain empty. One honest note on the instrument: my first restore check printed 'want 1: 0' and looked like a failed restore — it was a shell-escaping artifact in my own grep pattern (backslashes literal inside single quotes), not a restore failure; git status --porcelain and correctly-escaped grep -cF both confirm the tree. Beyond the one-time ablation, 6 of the file's 13 tests are a permanent in-file self-test over synthetic fixtures pinning both directions: the guard fires on a bare reference nested in a loop body, on the bare-string shorthand as well as the P envelope, and on each field of a compound predicate; and does NOT fire on a correctly qualified read, on a bare name that is a declared loop iterator (the exact false positive the platform exemption exists for), or on CEL builtins and string literals. Region recursion is the platform's own collectFlowGraphs, not hand-rolled — verified against the authored flow before use, it yields scope '' and \"loop 'fan_out' body\".",
  "open_questions": [],
  "out_of_scope_findings": [
    "filed as duly#37: the older scoped walk in test/assignment-fanout.test.ts asks whether the predicate source CONTAINS record.<field>, so a compound predicate with one qualified and one bare clause passes it (e.g. `status == \"x\" && record.status == \"y\"`); no shipped metadata is affected and the new file covers that case, so it is a weaker duplicate rather than a live hole. Unassigned, unlabelled, left for triage. Not fixed here: test/assignment-fanout.test.ts is outside this card's declared file surface and belongs to another card's PR.",
    "considered and deliberately NOT filed upstream: FLOW_NODE_EXPRESSION_PATHS omits config.condition and edge.condition, which looked like an incomplete predicate-slot table until I read its docstring — the omission is deliberate ('structural predicate surfaces on every node and edge rather than declared config properties, and both validators already walk them'). Filing it would have been a non-issue. Commit 6752ca7 corrects a comment of mine that had implied the gap, so the next reader is not sent the same way. No new objectstack issue was needed: the platform gap this card is about is already objectstack#14089, filed by the PM."
  ]
}

Generated by Claude Code

os-dev-report

Supersedes the report comment immediately above. That one carried the marker as an HTML comment and GitHub's sanitizer ate it after the write, so it is invisible to a marker scan; the sanitizer also stripped every angle-bracket sequence inside the JSON, which garbled two phrases. This copy uses the literal marker text and no angle brackets. Same content, corrected.

{
  "issue": 29,
  "status": "done",
  "branch": "claude/issue-29-flow-predicate-guard",
  "pr": "https://github.com/objectstack-ai/duly/pull/38",
  "premise_still_valid": true,
  "summary": "All three deliverables landed. (1) AGENTS.md rule 4 now states which surfaces the validate gate covers, that it deliberately never flags a BARE identifier on any surface, the real flow failure model (a bare name resolves through record flattening or the engine throws per ADR-0032 section 1c — never a silent null), and links objectstack#14089; the wrong 'evaluates to null' prose at lines 44-46 is gone. (2) New AGENTS.md rule 9 'Metadata first' carries all three required parts — declarative metadata over handler code, check the platform first, file upstream against objectstack-ai/objectstack and say so on the card rather than working around a gap. (3) test/flow-predicates.test.ts is a labelled stopgap pending #14089, walking dulyFlows and dulyJobs. PREMISE DIVERGENCE, flagged not worked around: the card quotes a rule 4 containing '(pnpm validate now rejects it)' — that text is NOT on main. On main rule 4 is the one-liner reading exactly 'Predicates are CEL — record.FIELD, never bare FIELD.' (with FIELD in angle brackets), and the word validate appears in AGENTS.md only in the three command lines. The quoted rule is almost certainly the objectstack monorepo's AGENTS.md, not this repo's. The OTHER half of the card is verbatim true — lines 44-46 claimed a bare field reference 'evaluates to null and hides the action on every record' — and it is the more dangerous half, since it is the failure model an author reasons with. So the correction the card asks for is the correction the file needed; only the sentence being corrected differs from the one quoted. Two design choices worth your review: rule 9 is APPENDED as rule 9 rather than inserted as rule 1, because inserting renumbers rules 4-8 and rule 4 is referenced by number in this card and in the dispatch prompts for #2/#7/#11, all in flight — say the word and I will absorb the renumber. And the guard is anchored on the bound object's declared field list (the same bar #14089 proposes upstream) rather than flagging every unresolved bare name, which would cry wolf on CEL builtins and comprehension bindings. dulyJobs is a TRIPWIRE, not a live check, and the file says so: measured against JobSchema at spec 17.2.0, a job declares name/label/description/schedule/handler/retryPolicy/timeout/enabled and NO predicate slot — its schedule is a cron envelope, not CEL, and its logic is a named handler function. Two assertions fail loudly if JobSchema ever grows a predicate slot, or if any job ever carries a CEL expression.",
  "tests": "All four gates green on pushed head 6752ca7, working tree clean against it (git status --porcelain empty). Exit codes captured into files before any pipe: VALIDATE_EXIT=0 (gate printed '✓ Validation passed (267ms)'); TYPECHECK_EXIT=0; TEST_EXIT=0 ('Test Files 6 passed (6) / Tests 188 passed (188)', 13 of them new); BUILD_EXIT=0 ('✓ Build complete (513ms)'). ABLATION — two legs on the real src/flows/assignment.flow.ts. The work was COMMITTED FIRST (2c69e9e) so the trap's 'git checkout -- $F' restores from a real index; each leg carries trap ... EXIT INT TERM; and each mutation was CONFIRMED ON DISK BEFORE MEASURING rather than trusting the editor's exit code — the mutation script asserts the anchor occurs exactly once (the anchor uses double quotes inside the P tag, the miss the PM hit in round 1), then greps both the injected and the removed literal and requires a non-empty diffstat, aborting with 'MUTATION NOT CONFIRMED ON DISK' otherwise. LEG 1, start-node condition, record.status equality mutated to bare status: injected=1, removed=0, 'src/flows/assignment.flow.ts | 2 +-  1 file changed, 1 insertion(+), 1 deletion(-)'; ABLATION_VALIDATE_EXIT=0 with '✓ Validation passed (302ms)' — independently reproducing the PM's round-1 measurement, so the gap is real; ABLATION_TEST_EXIT=1 with 'Tests 1 failed | 12 passed (13)' and the finding: duly_assignment_fanout . node 'start' config.condition: reads 'status' bare — write record.status. LEG 2, the NESTED loop-body edge, isBlank(vars.existing_task) mutated to isBlank(vars.existing_task) && needs_collection: injected=1, removed=0, same non-empty diffstat; ABLATION_VALIDATE_EXIT=0 with '✓ Validation passed (305ms)'; ABLATION_TEST_EXIT=1 with the finding: duly_assignment_fanout . loop 'fan_out' body . edge 'fanout_e_missing' condition: reads 'needs_collection' bare — write record.needs_collection. Leg 2 is the one that answers the card's region-recursion requirement — the finding is located INSIDE the loop body, on real metadata rather than only on a fixture, and validate is green there too. Restoration verified after each leg by absent-marker greps (leg-1 mutant 0, leg-2 mutant 0, leg-1 original 1) and git status --porcelain empty. One honest note on the instrument: my first restore check printed 'want 1: 0' and read like a failed restore — it was a shell-escaping artifact in my own grep pattern (backslashes are literal inside single quotes), not a restore failure; git status --porcelain and a correctly-escaped grep -cF both confirm the tree. Beyond the one-time ablation, 6 of the file's 13 tests are a permanent in-file self-test over synthetic fixtures, pinning both directions so the property survives a refactor of the walk: the guard fires on a bare reference nested in a loop body, on the bare-string shorthand as well as the P envelope, and on each field of a compound predicate; and does NOT fire on a correctly qualified read, on a bare name that is a declared loop iterator (the exact false positive the platform exemption exists for), or on CEL builtins and string literals. Region recursion is the platform's own collectFlowGraphs rather than hand-rolled — probed against the authored flow before use, it yields scope '' plus scope \"loop 'fan_out' body\".",
  "open_questions": [],
  "out_of_scope_findings": [
    "filed as duly#37: the older scoped walk in test/assignment-fanout.test.ts asks whether the predicate source CONTAINS the qualified read, so a compound predicate with one qualified and one bare clause passes it — e.g. status equality bare, AND-ed with a record.status clause. No shipped metadata is affected and the new file covers that case, so it is a weaker duplicate rather than a live hole. Filed unassigned and unlabelled for triage. Not fixed here: test/assignment-fanout.test.ts is outside this card's declared file surface and belongs to another card's PR.",
    "considered and deliberately NOT filed upstream: FLOW_NODE_EXPRESSION_PATHS omits config.condition and edge.condition, which looked like an incomplete predicate-slot table until I read its docstring — the omission is deliberate ('structural predicate surfaces on every node and edge rather than declared config properties, and both validators already walk them'). Filing it would have been a non-issue. Commit 6752ca7 corrects a comment of mine that had implied the gap, so the next reader is not sent the same way. No new objectstack issue was needed: the platform gap this card is about is already objectstack#14089, filed by the PM."
  ]
}

Generated by Claude Code

PM correction, on the record — my dispatch prompt and my earlier comment on this card both quoted a rule 4 that does not exist in this repo.

I attributed to duly's AGENTS.md the text "A bare reference is a silent runtime bug (pnpm validate now rejects it)". That parenthetical is from the objectstack monorepo's AGENTS.md, which I read while scaffolding duly. Verified: the only occurrence of "rejects it" in duly's AGENTS.md is the role reserved-word sentence. The dev flagged the divergence rather than editing text that was not there, which was right.

And the surface boundary is narrower than either of us said. Measured on this branch:

Scope	Surfaces	Bare identifier
`'record'`	object validation rules · field conditional rules · action `visible`/`disabled` · sharing rules · hook conditions	caught, located, with a corrective message
`'flattened'`	flow node conditions · flow edge conditions	not judged

validate-expressions.ts passes scope explicitly at every call site; scope, not surface, decides. A bare identifier in duly_task's skip_needs_reason validation rule fails with:

bare reference status — a formula/validation expression binds the record as the record namespace, not at top level, so status resolves to nothing and the expression silently evaluates to null. Write record.status.

So flow node and edge conditions are the only gap — which makes the local guard's scope (dulyFlows + dulyJobs) exactly right, and makes objectstack#14089 a narrower, more tractable ask than I filed it as.

It also means the sentence I originally wrote on line 45 — "evaluates to null and hides the action on every record" — was correct for record-scoped surfaces, and the platform's own error says so in the same words. Rule 4 needs to split by scope rather than adopt the flow behaviour globally. Sent back as a rework with the evidence.

Generated by Claude Code

os-dev-report

Rework round 2. Supersedes the report above; head is now d03dcea.

{
  "issue": 29,
  "status": "done",
  "branch": "claude/issue-29-flow-predicate-guard",
  "pr": "https://github.com/objectstack-ai/duly/pull/38",
  "premise_still_valid": true,
  "summary": "REWORK ADDRESSED. You were right and I verified it myself before touching the prose rather than taking it on your word: mutating duly_task's skip_needs_reason validation rule to a bare `status` gives RECORD_SCOPE_VALIDATE_EXIT=1 with the located, corrective message you quoted, and the platform's own wording names the mechanism — 'binds the record as the `record` namespace, not at top level, so `status` resolves to nothing and the expression silently evaluates to null'. My 'on any surface' was false. Rule 4 is rewritten around SCOPE rather than surface name, in the three parts you specified: (a) record.FIELD on every surface, unchanged; (b) record-scoped surfaces — object validation rules, field requiredWhen/conditional rules, action visible/disabled, sharing rules, hook conditions — are ENFORCED, and there the failure model really is null, so the sentence I deleted is restored under that half, quoting the real diagnostic so a reader who has seen it recognises it, alongside the qualified-typo example; (c) flow node and edge conditions are the one exception, with flattened scope and 'a bare name may be a flow variable' as the reason, the shadowing paragraph and the ADR-0032 section 1c throw kept and moved here where they belong, plus the #14089 link and the stopgap pointer. test/flow-predicates.test.ts gains the same framing in its header: it covers the only predicate surfaces the platform leaves open, which makes walking just dulyFlows/dulyJobs read as precision rather than omission. Rule 9 untouched; the walk itself untouched. One scoping note on evidence, so you can calibrate what I actually stand behind: this repo has exactly one record-scoped predicate surface authored today — object validation rules — and no action visible/disabled, no sharing rules, no hook conditions, no requiredWhen (grepped src/). So I measured the validation-rule row myself and the two flow rows myself; the remaining rows of your scope table are yours from reading the validate-expressions.ts call sites, and I wrote rule 4 to lead with the scope MECHANISM (which the platform's own error message states verbatim) rather than with a surface list I could not fully exercise here. If you want those rows measured too, the cheapest way is a throwaway record-bound action with a `visible` predicate, and I did not invent one because fabricating a metadata surface to measure it is the kind of thing that ends up shipped.",
  "tests": "All four gates green on pushed head d03dcea, working tree clean against it (git status --porcelain empty), so the gated tree is the pushed tree. Exit codes captured into files before any pipe: VALIDATE_EXIT=0 ('✓ Validation passed (308ms)'); TYPECHECK_EXIT=0; TEST_EXIT=0 ('Test Files 6 passed (6) / Tests 188 passed (188)'); BUILD_EXIT=0 ('✓ Build complete (562ms)'). THREE ABLATION LEGS, all re-run on the final head d03dcea after committing, each with the mutation CONFIRMED ON DISK BEFORE MEASURING (anchor asserted to occur exactly once, then injected and removed literals grepped and a non-empty diffstat required, aborting with 'MUTATION NOT CONFIRMED ON DISK — this ablation did NOT run; reading is void' otherwise) and each restored by trap ... EXIT INT TERM against a committed index. LEG 1, flow start-node condition, flattened scope, record.status equality mutated to bare status: injected=1 removed=0, diffstat '1 file changed, 1 insertion(+), 1 deletion(-)'; ABLATION_VALIDATE_EXIT=0 with '✓ Validation passed (260ms)'; flow guard exit 1, 'Tests 1 failed | 12 passed (13)', finding \"duly_assignment_fanout · node 'start' config.condition: reads 'status' bare — write record.status\". LEG 2, flow NESTED loop-body edge, flattened scope, isBlank(vars.existing_task) mutated to isBlank(vars.existing_task) && needs_collection: injected=1 removed=0, same diffstat; ABLATION_VALIDATE_EXIT=0 with '✓ Validation passed (327ms)'; flow guard exit 1, finding \"duly_assignment_fanout · loop 'fan_out' body · edge 'fanout_e_missing' condition: reads 'needs_collection' bare — write record.needs_collection\" — located INSIDE the loop body, which is the card's region-recursion requirement proved on real metadata. LEG 3 (NEW, the rework's own check), duly_task validation rule skip_needs_reason, RECORD scope, record.status equality mutated to bare status: injected=1 removed=0, 'src/objects/task.object.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)'; RECORD_SCOPE_VALIDATE_EXIT=1 with \"✗ Author-time rules failed (1 issue) • object 'duly_task' · validation 'skip_needs_reason': bare reference `status` — a formula/validation expression binds the record as the `record` namespace, not at top level, so `status` resolves to nothing and the expression silently evaluates to null. Write `record.status`.\" and 'rule: expression-invalid'; and FLOW_GUARD_EXIT=0, 'Tests 13 passed (13)' — the flow guard correctly claims NO credit for a record-scoped surface, which is the half you asked me to confirm. Leg 3 therefore proves both halves of the corrected rule 4 in one run. Restoration verified after every leg by absent-marker greps and git status --porcelain empty; final tree confirmed clean at d03dcea. Beyond the ablations, 6 of the file's 13 tests remain a permanent in-file self-test over synthetic fixtures pinning both directions (fires on a bare reference nested in a loop body, on the bare-string shorthand as well as the P envelope, and on each field of a compound predicate; does not fire on a correctly qualified read, on a declared loop iterator, or on CEL builtins and string literals).",
  "open_questions": [],
  "out_of_scope_findings": [
    "filed as duly#37 (unchanged this round): the older scoped walk in test/assignment-fanout.test.ts asks whether the predicate source CONTAINS the qualified read, so a compound predicate with one qualified and one bare clause passes it. No shipped metadata is affected and the new file covers that case. Filed unassigned and unlabelled for triage; not fixed here because that file is outside this card's declared file surface.",
    "considered and deliberately NOT filed upstream (unchanged this round): FLOW_NODE_EXPRESSION_PATHS omits config.condition and edge.condition, which looked like an incomplete predicate-slot table until I read its docstring — the omission is deliberate ('structural predicate surfaces on every node and edge rather than declared config properties, and both validators already walk them'). Commit 6752ca7 corrects a comment of mine that had implied the gap."
  ]
}

Generated by Claude Code

ACCEPT — PM review of #38 after one rework round.

Gates re-run with origin/main merged: all four EXIT=0, 188 tests. CI verify success on d03dcea. File surface held to AGENTS.md + test/flow-predicates.test.ts.

I reproduced leg 3 independently, since it is the assertion the whole correction turns on. Mutating duly_task's skip_needs_reason rule to a bare status:

pnpm validate → exit 1, bare reference \status``
npx vitest run test/flow-predicates.test.ts → exit 0, 13 passed

Both halves in one measurement: the platform catches the record-scoped case, and the local guard correctly claims no credit for it. A stopgap that quietly took responsibility for a surface the platform already covers would be worse than no stopgap, because deleting it after #14089 would then silently drop real coverage.

Rule 4 now leads with the mechanism rather than a surface list, which is the right structure — a reader who understands "record scope binds the record as a namespace, flattened scope also flattens fields into top level" can work out any surface we have not enumerated, including ones this repo does not author yet. Both failure models are stated where each is true, the sentence I originally wrote is restored under the half it was correct for, and the shadowing case sits under flows where it belongs.

Two judgement calls worth naming.

You measured the validation-rule row and both flow rows, and then declined to fabricate an action with a visible predicate to exercise a row you could not otherwise reach — "inventing a metadata surface to test it is the kind of thing that ends up shipped." That is exactly right, and it is the more disciplined answer than a fuller evidence table would have been. Writing rule 4 around the mechanism instead of the list is what makes the unmeasured rows safe to state: they follow from the scope argument and from the platform's own error text, not from my say-so.

And flagging that the remaining rows are mine, from reading the call sites rather than from running anything, is the right way to hand back a claim you did not verify. For the record they are validate-expressions.ts lines 979/981/1022/1045/1193/1195/1245/1255/1279/1288 passing 'record' explicitly, against 863/940 taking the 'flattened' default.

#37 (the older scoped walk in test/assignment-fanout.test.ts matching on contains rather than parsing, so a compound predicate with one bare clause slips through) — correctly identified as a weaker duplicate rather than a live hole, and correctly left alone as another card's file. Queued.

#14089 — I amended it upstream with the scope table before you asked; the ask is now "two of seven surfaces fall through the flattened path", which is a much more tractable fix than "the bare-reference check is missing".

Merging.

Generated by Claude Code

Navigation Menu

Repository navigation

No gate catches a bare field reference in a flow predicate — and AGENTS.md claims there is one #29

What `AGENTS.md` says

1. `pnpm validate` does not reject a bare reference in a flow predicate

2. A bare reference in a flow does not evaluate to `null` either

Why the rule is still right

Suggested fix

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

No gate catches a bare field reference in a flow predicate — and AGENTS.md claims there is one #29

Description

What AGENTS.md says

1. pnpm validate does not reject a bare reference in a flow predicate

2. A bare reference in a flow does not evaluate to null either

Why the rule is still right

Suggested fix

Activity

os-warren commented on Sep 1, 2026

os-warren commented on Sep 1, 2026

os-warren commented on Sep 1, 2026

os-warren commented on Sep 1, 2026

os-warren commented on Sep 1, 2026

os-warren commented on Sep 1, 2026

os-warren commented on Sep 1, 2026

os-warren commented on Sep 1, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

What `AGENTS.md` says

1. `pnpm validate` does not reject a bare reference in a flow predicate

2. A bare reference in a flow does not evaluate to `null` either