From f0f8ef950f93ae9d7933620596f40281825b61e7 Mon Sep 17 00:00:00 2001 From: Bei Li Date: Sun, 13 Sep 2026 20:02:07 +0000 Subject: [PATCH 1/5] feat(mdcode): evaluate an action's guards against the store Until now a guarded action was refused outright, because nothing evaluated a constraint and running the write would have applied it past a rule the model says checks it. This turns that blanket refusal into a real check. `lowerGuards` resolves each name in `guards` and turns the constraint's expression into one query against the store. When it runs follows from what the expression reads, and the model never states it: a rule over the action's parameters is settled before any statement runs, and a rule over stored data runs after the statements and inside the same transaction, where it reads the state the write produced. A violation rolls the write back and reports which rule stopped the call, in the words the constraint's `description` gives. `ActionOutcome` gains a `refused` arm carrying the effect, the violated rules and the message. `committed` gains `warnings` for advisory violations and `unchecked` for advisory rules the runtime could not evaluate. Several violations combine by the strictest effect: any reject refuses, failing that any escalate holds, failing that any warn reports. Two kinds of guard still do not become a query. A judgment is one, since nothing calls a judge; an expression outside the grammar is the other. A rule that would reject or escalate and cannot be checked stops the action, and every such rule is named. A rule that would warn does not: it reports nothing to begin with, so failing to check it cannot be grounds for stopping the write, and it is reported back as not checked instead. The grammar accepts comparisons over an entity's stored fields, the action's parameters and literals, joined by AND and OR. It refuses a function call, a parenthesised subexpression, an expression spanning two entities, an abstract or composite-keyed entity, and an entity the action takes no reference parameter for -- each with the reason in the message. The refusal tests from the action-runtime change are replaced one for one by tests of the check that now happens. The guide's status paragraphs, the run flow, the outcome set and the agent-tools listing are rewritten against real command output. --- .../demo/semantic-model/agent/README.md | 60 +- .../EntryGroups/commerce_demo/commerce.yaml | 2 +- toolbox/mdcode/docs/semantic-model/actions.md | 277 +++++--- .../mdcode/docs/semantic-model/reference.md | 13 +- .../src/libts/semantic/runtime/agent_tools.ts | 59 +- .../libts/semantic/runtime/constraint_eval.ts | 658 ++++++++++++++++++ .../src/libts/semantic/runtime/run_action.ts | 184 +++-- toolbox/mdcode/src/tool/commands.ts | 14 + .../semantic/runtime/agent_tools.test.ts | 42 +- .../semantic/runtime/constraint_eval.test.ts | 478 +++++++++++++ .../libts/semantic/runtime/run_action.test.ts | 196 +++++- toolbox/mdcode/tests/tool/action.test.ts | 49 +- 12 files changed, 1774 insertions(+), 258 deletions(-) create mode 100644 toolbox/mdcode/src/libts/semantic/runtime/constraint_eval.ts create mode 100644 toolbox/mdcode/tests/libts/semantic/runtime/constraint_eval.test.ts diff --git a/toolbox/mdcode/demo/semantic-model/agent/README.md b/toolbox/mdcode/demo/semantic-model/agent/README.md index 142159a4..be9ff289 100644 --- a/toolbox/mdcode/demo/semantic-model/agent/README.md +++ b/toolbox/mdcode/demo/semantic-model/agent/README.md @@ -475,47 +475,25 @@ The agent wrote $30 and asked nobody. The agent did not disobey: the policy is written down and never attached to anything. `commerce.yaml` declares three constraints, one per policy rule, and references -none of them. A constraint is inert until an action names it in -`guards`, and this runtime does not evaluate constraints yet — so naming one -makes the action *unrunnable* rather than checked. Try it: add -`guards: [CreditUnderReviewThreshold]` to the action and run `kcmd agent tools` -again. - -```console - action issue_credit (IssueCredit) [NOT RUNNABLE] - ... - This call is gated by CreditUnderReviewThreshold. - - Calling this will not work: Action 'IssueCredit' is guarded by - 'CreditUnderReviewThreshold', and this runtime does not evaluate - constraints yet. Running it would apply a write the model says must be - checked first, so it is refused rather than run unchecked. Report that - rather than retrying. - ... -``` - -Refusing is the point. The model says this write must be checked and the checker -is missing, so the write is refused rather than run unchecked. The tool is still -derived, still named and still described, because an action the model declares -should not vanish from what the model offers. `agent.ts` reports it and leaves -it unbound, so the agent has no call to make and nothing to retry. - -Re-run the same request with the guard attached and you get the reading half and -none of the writing half: - -```console -$ bun agent.ts "Find the order for Morgan Ellis (morgan.ellis@example.com) that was placed on Labor Day. ..." -(withheld) issue_credit: Action 'IssueCredit' is guarded by 'CreditUnderReviewThreshold', and this runtime does not evaluate constraints yet. Running it would apply a write the model says must be checked first, so it is refused rather than run unchecked. - -> find_customer({"email":"morgan.ellis@example.com","name":"Morgan Ellis"}) - <- {"entity":"Customer","fields":["customerId","name","email"],"rows":[["1","Morgan Ellis","morgan.ellis@example.com"]],"truncated":false} - -> find_order({"customerId":1,"placedOn":"2026-09-07"}) - <- {"entity":"Order","fields":["orderId","customerId","placedOn","total","status"],"rows":[["12345","1","2026-09-07","165.85","OPEN"]],"truncated":false} - -> find_line_item({"orderId":12345,"type":"fee"}) - <- {"entity":"LineItem","fields":["lineItemId","orderId","type","amount","memo"],"rows":[["li-12345-3","12345","fee","30","Shipping"]],"truncated":false} -The customer Morgan Ellis (ID 1) has an order (ID 12345) placed on 2026-09-07. This order includes a shipping fee of $30.00 (line item ID li-12345-3). - -I cannot issue credits or modify orders. A person will have to decide how to proceed with the credit. -``` +none of them. A constraint is inert until an action names it in `guards`, so a +rule nobody named is a rule no call consults, whatever it says. + +`CreditUnderReviewThreshold` is the rule the run above walked past, and it reads +`amount <= 25` — a comparison over the action's own parameter, which the runtime +turns into a query and checks before any statement runs. Adding +`guards: [CreditUnderReviewThreshold]` to `IssueCredit` is therefore all it +takes: the $30 credit is refused with `escalate`, the transaction rolls back, +and the agent is told a supervisor decides it. See +[What a violated guard does](../../../docs/semantic-model/actions.md#what-a-violated-guard-does). + +Two of the three rules are less straightforward. `CreditWithinOrderTotal` reads +`Order.total`, so it is a rule over stored data and runs after the statements +inside the same transaction. `OrderTotalMatchesLineItems` aggregates over a +child table, which the expression grammar does not parse, so naming it stops the +action rather than checking it. + +Wiring the guards and re-running the request live is the next step for this +demo, and the transcript above is what it looks like before that happens. It still did the work worth doing — found the order, found the charge, named the amount — and order 12345 is still $165.85. diff --git a/toolbox/mdcode/demo/semantic-model/agent/catalog/EntryGroups/commerce_demo/commerce.yaml b/toolbox/mdcode/demo/semantic-model/agent/catalog/EntryGroups/commerce_demo/commerce.yaml index a077de9c..37567a80 100644 --- a/toolbox/mdcode/demo/semantic-model/agent/catalog/EntryGroups/commerce_demo/commerce.yaml +++ b/toolbox/mdcode/demo/semantic-model/agent/catalog/EntryGroups/commerce_demo/commerce.yaml @@ -134,7 +134,7 @@ semantic_model: decides it. - name: OrderTotalMatchesLineItems - expression: Order.total == SUM(LineItem.amount) + expression: Order.total = SUM(LineItem.amount) on_violation: reject description: >- An order's total must equal the sum of its line items, with credits diff --git a/toolbox/mdcode/docs/semantic-model/actions.md b/toolbox/mdcode/docs/semantic-model/actions.md index 006f4214..f299474f 100644 --- a/toolbox/mdcode/docs/semantic-model/actions.md +++ b/toolbox/mdcode/docs/semantic-model/actions.md @@ -274,15 +274,14 @@ Four stages, and a rule that stops at the first one does nothing: ``` declared referenced checked a breach ───────────────── ───────────────── ────────────── ─────────── - constraints: actions: before the call, reject - - name: X ──▶ - name: Y ──▶ with the ──▶ escalate - expression: … guards: [X] arguments bound warn - or judgment: … - - a rule in the the only thing that a query settles on_violation - catalog, inert gives it effect an expression; names one of - a language model the three - a judgment + constraints: actions: a query settles reject + - name: X ──▶ - name: Y ──▶ an expression; ──▶ escalate + expression: … guards: [X] a language model warn + or judgment: … a judgment + + a rule in the the only thing that when it runs on_violation + catalog, inert gives it effect follows from names one of + what it reads the three ``` An expression over stored data states a condition the data must satisfy: @@ -328,14 +327,13 @@ that reads the action's parameters has no other moment to run. One over stored data, named as a guard, states that the call must not proceed on data that is already broken. -Every guard is checked before the call, with the arguments bound. What each rule -reads decides how much that moment can tell you. A rule over the parameters is -settled completely there, since the arguments are the whole of what it reads. A -rule over stored data is a condition on the state a write produces, and checking -it before the call reports only that the call is not starting from a broken -state. It does not report that the call leaves a sound one. Nothing in the model -binds a rule to the result of a write, which is the gap between what a data rule -says and what a guard can enforce. +When a guard runs follows from what its expression reads, and the model never +states it. A rule over the action's parameters is settled before any statement +runs, since the arguments are the whole of what it reads. A rule over stored +data is a condition on the state a write produces, so it runs after the +statements and inside the same transaction, and a breach rolls that write back. +The difference is derived from the expression rather than authored, so a rule +stays one sentence whether it decides the call or its result. Whatever dispatches the call is what checks its guards. Handing a rule to the store instead works only for some rules. A condition on a single row lowers to a @@ -448,7 +446,7 @@ constraint, carrying its own outcome in its own `on_violation`: severity: medium - name: OrderTotalMatchesLineItems # rule 3 - expression: Order.total == SUM(LineItem.amount) + expression: Order.total = SUM(LineItem.amount) description: >- An order's total must equal the sum of its line items, with credits subtracted. @@ -509,10 +507,16 @@ of every `guards` list it would be a rule the catalog records and no call consults, and the strongest word in the policy would be the one with the least effect. -Naming it makes `IssueCredit` refuse to run against an order whose books already -disagree. Catching the credit that *breaks* the agreement is a different check, -against the state the write produces, and the model cannot bind one yet. Rule 3 -is the rule in this policy whose enforcement is furthest from what it says. +Naming it binds the rule to the state the write produces. A rule over stored +data runs after the statements and inside the same transaction, so `IssueCredit` +refuses the credit that *breaks* the agreement, rather than only the one raised +against an order whose books already disagree. + +Rule 3 is also where this policy meets the limit of what the runtime lowers. +`SUM` over a child table is a function call, and the expressions that become a +query are comparisons over fields, parameters and literals. Rule 3 is named, +published, and still not checked, and because it says `reject`, a call to +`IssueCredit` is refused rather than run past it. **Rules 4 and 5 are why the second body exists.** Neither reduces to arithmetic over `Order` and `LineItem`, and before `judgment` they had nowhere to go but a @@ -574,10 +578,11 @@ guards are expressions. **Status: nothing calls a judge.** `kcmd` parses `judgment`, validates it, publishes it and reads it back, and publishes a derived `evaluation` field saying whether the rule is `deterministic` or `judged` so a consumer can select -on it. No component asks a model to settle a judgment, and nothing combines -guard outcomes. The two calls above are what the published policy says should -happen, and `kcmd` publishes the fields an engine needs in order to make it -happen. +on it. No component asks a model to settle a judgment. Rule 4 says `warn`, so a +call goes ahead and the rule is reported as not checked. Rule 5 says `reject`, +so `IssueCredit` as published above is refused before either call reaches a +gate. The two calls are what the published policy says should happen, and `kcmd` +publishes the fields an engine needs in order to make it happen. `kcmd` reports a mismatch from either side. A guard that names no constraint fails the push. A constraint over parameters that no action names loads with a @@ -585,12 +590,20 @@ warning, because nothing will ever evaluate it. That scan reads expressions only: a judgment is prose, in which a word matching a parameter name is not a read of that parameter. -**Status: nothing evaluates a guard yet.** `kcmd` parses `guards`, resolves each -name, publishes the list, and reads it back. No component checks a guard against -live data, so a guard states what must hold before the call and stops no call by -itself. What it does stop is the call running unchecked: -[`kcmd action run`](#7-run-it) refuses a guarded action outright rather than -apply a write the model says is checked first. +**Status: expression guards are checked.** `kcmd` parses `guards`, resolves each +name, and turns every expression it can into a query against the store. Those +queries run in the same transaction as the write: a rule over the parameters +before the statements, a rule over stored data after them. A violation carries +its own `on_violation` word back to the caller, and several violations combine +by the rule above. + +A guard the runtime cannot turn into a query is what remains. A judgment is one, +since nothing calls a judge, and so is an expression outside the grammar, such +as rule 3's `SUM`. What happens then follows the rule's own `on_violation`. One +that would `reject` or `escalate` refuses the action, because running it would +apply a write the model says is checked first. One that would `warn` lets the +write through and is reported back as not checked, since an advisory rule stops +nothing even when it is evaluated. ## 3. Say what it changes @@ -777,12 +790,6 @@ kcmd action run TransferFunds --arg source="Alice Checking" \ --arg target=ACC-2 --arg amount=250 ``` -That second command does not succeed against the model built up on this page, -and the reason is worth knowing before the mechanics: `TransferFunds` is guarded -by `AmountIsPositive`, nothing evaluates a constraint yet, and `kcmd` refuses a -call rather than apply a write the model says must be checked first. What -follows describes an action that names no guard, which is what runs today. - `kcmd action list` is what the model declares as runnable — parameters, executor, guards, blast radius — and each entry ends with the command line that runs it, so reading the listing is enough to make the call: @@ -863,7 +870,7 @@ does not limit the blast radius either — it *declares* it, so that a reader knows what the write is about and an evaluator can one day check the statements against what was declared. -`kcmd action run` does three things: +`kcmd action run` does four things: ``` kcmd action run TransferFunds --arg source="Alice Checking" --arg amount=250 @@ -875,24 +882,34 @@ against what was declared. │ bind @source = 7 as Integer, the key's declared type │ @amount = 250 as Decimal, so 9 is less than 10 │ - │ apply BEGIN - │ UPDATE account SET balance = balance - @amount - │ WHERE account_id = @source - │ COMMIT + │ check SELECT 1 AS violated FROM UNNEST([1]) + │ WHERE NOT COALESCE((@amount > 0), FALSE) + │ AmountIsPositive, before any statement runs + │ + │ apply UPDATE account SET balance = balance - @amount + │ WHERE account_id = @source ▼ - committed · nothing written · unknown, do not retry + committed · refused · nothing written · unknown, do not retry ``` +One transaction spans all four steps, so a guard that finds a violation and a +statement that fails both leave nothing behind. A guard over stored data runs +after the statements rather than before them, where it reads the state the write +produced. The `NOT COALESCE(…, FALSE)` wrapper makes a predicate that returns +NULL count as a breach rather than as silence. + Nothing is interpolated into a statement; every argument is a query parameter of the store type its declared ontology type implies. Any failure before the -commit rolls back, so no partial write survives, and a commit the store -*refuses* wrote nothing either. The commonest refusal is Spanner's `ABORTED` -under lock contention, and the answer to it is to run the action again. +commit rolls back, so no partial write survives, and a commit the store rejects +wrote nothing either. The commonest rejection is Spanner's `ABORTED` under lock +contention, and the answer to it is to run the action again. -The third outcome is the one `kcmd` cannot settle: a timeout or a 5xx, where -the store may have applied the write and lost the response. It is reported as -unknown rather than as a rollback, because a caller told "nothing happened" -would retry a write that did. +**Refused** is the model's answer rather than the store's: a guard was +violated, so the transaction was rolled back and the caller is told which rule +stopped the call. **Unknown** is the outcome `kcmd` cannot settle: a timeout or +a 5xx, where the store may have applied the write and lost the response. It is +reported as unknown rather than as a rollback, because a caller told "nothing +happened" would retry a write that did. Where the write goes is the model's Spanner deployment target under the selected profile. The command line never names a database: `--profile` changes the store, @@ -908,35 +925,96 @@ transaction and could not be rolled back if the commit failed. Supply a handler that performs the write as DML, or declare the action with a 'sql' executor. ``` -### A guarded action is refused, not run unchecked +### What a violated guard does -Nothing evaluates a constraint yet. A model that declares a rule and a runtime -that quietly ignores it is worse than no runtime, because the model states the -write is checked and nothing says otherwise — so `kcmd action run` refuses such -a call instead: +A violated guard stops the write and answers in the words the constraint's +`description` gives, so the caller reads the policy rather than a predicate. A +rule that declares `reject` ends the call: ``` -Error: Action 'TransferFunds' is guarded by 'AmountIsPositive', and this runtime -does not evaluate constraints yet. Running it would apply a write the model says -must be checked first, so it is refused rather than run unchecked. +$ kcmd action run TransferFunds --arg source="Alice Checking" --arg target=ACC-2 --arg amount=0 +Running 'TransferFunds' on projects/my-project/instances/my-instance/databases/semantic_agent_demo... +Refused (reject): Action 'TransferFunds' was refused and nothing was written. A +transfer must move at least one unit. Ask the caller for the amount again before +retrying. Stopped by 'AmountIsPositive' (amount > 0). ``` -What makes a call "such a call" is `guards`, and only `guards` — the same rule -[section 2](#2-gate-it-with-a-constraint) states, applied here. A constraint the -action does not name is a rule this call does not consult, and the runtime does -not go looking for one: a constraint that merely reads a concept the action -writes gates nothing, and neither does declaring constraints in a model whose -action leaves `affects` out. Refusing on either would mean publishing a rule -could start refusing calls that succeeded the day before, which is exactly what -making the reference explicit prevents. +A rule that declares `escalate` ends it the same way and adds what would change +the answer. Here is the credit policy from +[section 2](#a-policy-whose-rules-end-differently), with the two guards this +runtime cannot check left off the action, taking the 30-dollar credit: + +``` +$ kcmd action run IssueCredit --arg order=12345 --arg amount=30 --arg memo="shipping charge applied in error" +Running 'IssueCredit' on projects/my-project/instances/my-instance/databases/semantic_agent_demo... +Refused (escalate): Action 'IssueCredit' needs an approval, and nothing was +written. A credit over 25 dollars is above the self-service limit. A supervisor +decides it. Stopped by 'CreditUnderSelfServiceLimit' (amount <= 25). Nothing is +held while somebody decides: the transaction was rolled back, so run the action +again once it is approved. +``` + +Both exit non-zero, because the caller asked for a write and did not get one. +Neither is reported as an error: the model was consulted and said no, which is +the runtime working. A rule that declares `warn` commits and reports the +violation alongside the commit. + +When one call violates several guards the strictest outcome applies, by the +rule [section 2](#two-calls-through-that-policy) states, and every violated +rule is named in the message rather than only the one that decided it. + +What makes a rule a guard of this call is `guards`, and only `guards`. A +constraint the action does not name is a rule this call does not consult, and +the runtime does not go looking for one: a constraint that merely reads a +concept the action writes gates nothing, and neither does declaring constraints +in a model whose action leaves `affects` out. Checking either would mean +publishing a rule could start refusing calls that succeeded the day before, +which is what making the reference explicit prevents. + +### A guard the runtime cannot check -A guard whose constraint declares `onViolation: warn` is the one guard that does -not refuse. Such a rule reports a violation rather than rejecting one, so an -evaluator would let the write through, and gating on it would leave a model that -states advisory rules permanently unrunnable. +Two kinds of guard do not become a query. A judgment is one, since nothing calls +a judge. An expression outside the grammar is the other: the comparisons that +lower are over an entity's stored fields, the action's parameters and literals, +joined by `AND` and `OR`, so a function call or a parenthesised subexpression +does not. -Every refusal is decided before a session is opened, so a refused action leaves -no transaction behind. +A model that declares a rule and a runtime that quietly ignores it is worse than +no runtime, because the model states the write is checked and nothing says +otherwise. So a rule that would `reject` or `escalate` and cannot be checked +stops the call, and every such rule is named: + +``` +$ kcmd action run IssueCredit --arg order=12345 --arg amount=30 --arg memo="shipping charge applied in error" +Running 'IssueCredit' on projects/my-project/instances/my-instance/databases/semantic_agent_demo... +Error: Action 'IssueCredit' cannot be run: constraint 'OrderTotalMatchesLineItems' +cannot be checked: it uses parentheses or a function call (Order.total = +SUM(LineItem.amount)), which the grammar does not parse; constraint +'CreditIsNotSplitToAvoidReview' cannot be checked: it is settled by judgment +rather than by an expression, and this runtime runs no judge. Running it would +apply a write the model says is checked first, so it is refused rather than run +unchecked. +``` + +An advisory rule is the exception. A rule that declares `warn` reports a +violation rather than stopping one, so failing to check it cannot be grounds for +stopping the call either; treating it as one would leave a model that states +advisory rules permanently unrunnable. The write goes ahead and the rule is +reported as not checked, which is the part worth having: a report the model +asked for and did not get is worth knowing about. + +``` +$ kcmd action run IssueCredit --arg order=12345 --arg amount=20 --arg memo="shipping charge applied in error" +Running 'IssueCredit' on projects/my-project/instances/my-instance/databases/semantic_agent_demo... + order: '12345' -> Order 12345 + not checked: advisory rule 'CreditMemoNamesAServiceFailure' (constraint + 'CreditMemoNamesAServiceFailure' cannot be checked: it is settled by judgment + rather than by an expression, and this runtime runs no judge) +Committed at 2026-09-13T18:00:00Z. +``` + +A rule that cannot be checked is found while the guards are lowered, before any +session is opened, so that refusal leaves no transaction behind. ## 8. Hand it to an agent @@ -957,19 +1035,13 @@ changes nothing. Model 'payments' (payments_eg), profile 'operational': store: my-project/my-instance/semantic_agent_demo - action transfer_funds (TransferFunds) [NOT RUNNABLE] + action transfer_funds (TransferFunds) Move money from one account to another. Resolve both accounts before calling. Name the account the money leaves as `source`. This call is gated by AmountIsPositive. - - Calling this will not work: Action 'TransferFunds' is guarded by - 'AmountIsPositive', and this runtime does not evaluate constraints yet. - Running it would apply a write the model says must be checked first, so - it is refused rather than run unchecked. Report that rather than - retrying. source: string -- Which Account this applies to. Give its key, or text that identifies exactly one; the call fails when nothing matches or more than one does. @@ -1061,10 +1133,10 @@ thing: actions[].executor ───▶ what the write tool runs ``` -Two lines come from neither file. `[NOT RUNNABLE]` and the paragraph under it -are the runtime's answer to whether this call could succeed. The second -paragraph of the instruction is the derivation's own text about using the -tools, identical for every model. +Two things come from neither file. Whether a tool can be called at all is the +runtime's answer rather than a key, and a tool it cannot offer is marked and +carries the reason. The second paragraph of the instruction is the derivation's +own text about using the tools, identical for every model. Nothing in the listing was written for a particular agent, which is the property worth being able to see: it reads the same whether the caller is ADK, @@ -1074,8 +1146,8 @@ LangChain, or a person deciding whether the model says enough yet. A **write tool** runs the action. Invoking `transfer_funds` performs the same resolve, bind and transact that [`kcmd action run TransferFunds`](#7-run-it) -performs, with the same argument resolution, the same single transaction and -the same three outcomes. +performs, with the same argument resolution, the same guards, the same single +transaction and the same four outcomes. A **lookup tool** reads one entity: exact match on any bound field, combined with AND, capped at 50 rows. No joins, no ranges, no aggregation, no ordering. @@ -1092,10 +1164,12 @@ allows the collision to be noticed at all. ### A tool says whether it can be called -`transfer_funds` above is listed and marked `[NOT RUNNABLE]`. `TransferFunds` -names a guard, nothing evaluates constraints yet, and so the [refusal from -section 7](#a-guarded-action-is-refused-not-run-unchecked) is reported here -instead — before any agent exists, rather than inside a transaction. +`transfer_funds` above is listed with no caveat. `AmountIsPositive` is an +expression over one of the action's parameters, so the runtime can check it, and +naming it as a guard costs the tool nothing. An action guarded by a rule the +runtime [cannot check](#a-guard-the-runtime-cannot-check) is the other case, and +it is reported here — before any agent exists, rather than inside a +transaction. The tool is still returned, still named and still described. An action the model declares should not vanish from what the model offers; what it is waiting @@ -1106,7 +1180,7 @@ on is the useful thing to print. Both halves carry a `runnable` flag, and |-------------------------------|---------------------------| | a profile withdrew the executor | the entity is abstract, so it has no table | | the executor is remote and no handler was supplied | no profile bound it to a table | -| it names a guard, as above | its binding is a query rather than a table | +| it names a guard this runtime cannot check | its binding is a query rather than a table | | a parameter references an entity keyed by several columns | | | the statements ask for a generated key a UUID cannot fill | | @@ -1204,18 +1278,21 @@ not. The place to put it is the model. operational store: a commerce model, a binding profile, and one file of 56 lines that names no table, no column and no business term. Thirteen of those lines are the adapter onto the agent framework. Its README walks the same four steps and -states what the run cannot yet do — the $30 credit it issues is over the model's -declared $25 self-service ceiling and is written anyway, because nothing -evaluates constraints. +states what the run cannot yet do. ## What is not modeled yet -This is a prototype. Three things a reader reasonably expects are absent. - -- **Nothing checks the write.** No component evaluates a constraint or a guard. - A guarded action is refused rather than run, so the gap is loud where a model - states a rule gates the call, but it is still a gap: the correctness of what a - statement does belongs to whoever wrote it. +This is a prototype. Four things a reader reasonably expects are absent. + +- **No judge settles a judgment.** An expression guard is turned into a query + and checked; a judgment is published and nothing reads it. An action guarded + by one is stopped rather than run past it, unless the rule is advisory, so the + gap is loud wherever a model states a judged rule gates the call. +- **The expression grammar is narrow.** Comparisons over an entity's stored + fields, the action's parameters and literals, joined by `AND` and `OR`. A + function call, a subquery or a rule spanning two entities does not lower, and + the action is stopped rather than run past it. Beyond the guards, the + correctness of what a statement does belongs to whoever wrote it. - **`kcmd` calls no executor but its own.** A `sql` action runs; an `mcp`, `rest` or `grpc` one is published for whoever dispatches it, which is why those three name coordinates rather than a statement. diff --git a/toolbox/mdcode/docs/semantic-model/reference.md b/toolbox/mdcode/docs/semantic-model/reference.md index 23e8b83d..ff7b94fd 100644 --- a/toolbox/mdcode/docs/semantic-model/reference.md +++ b/toolbox/mdcode/docs/semantic-model/reference.md @@ -73,9 +73,12 @@ kcmd action run --arg = ... `list` prints every action the models in the scope declare, with the store a run would reach and the command line that runs each one. `run` executes one against the Spanner database the selected profile's deployment target names; only a -`sql` executor runs, and an action that names a constraint in `guards` is -refused rather than run unchecked, because nothing evaluates a constraint yet. -See [Run it](actions.md#7-run-it). +`sql` executor runs. Every constraint an action names in `guards` is checked in +the same transaction as the write: one over the parameters before the +statements, one over stored data after them. A violation rolls the write back +and reports which rule stopped the call. A guard this runtime cannot turn into a +query — a judgment, or an expression outside the grammar — stops the action +unless the rule is advisory. See [Run it](actions.md#7-run-it). | Flag | Effect | |------|--------| @@ -97,8 +100,8 @@ runs nothing. A tool the runtime cannot call is listed and marked `[NOT RUNNABLE]` rather than dropped, with the reason in its description, so a refusal is visible before any -agent exists. To see what a guard costs today, add a constraint to an action's -`guards` and run this again. +agent exists. An action guarded by a rule the runtime cannot check is one such +case; an action guarded by an expression it can check is offered normally. A model whose profile names no Spanner database offers no tools, because calling one needs a store. That model is reported as offering none and the rest of the diff --git a/toolbox/mdcode/src/libts/semantic/runtime/agent_tools.ts b/toolbox/mdcode/src/libts/semantic/runtime/agent_tools.ts index c0bc9429..2caec80c 100644 --- a/toolbox/mdcode/src/libts/semantic/runtime/agent_tools.ts +++ b/toolbox/mdcode/src/libts/semantic/runtime/agent_tools.ts @@ -23,7 +23,7 @@ * knob it can turn. * * One thing the runtime cannot yet do shows through here. Nothing evaluates a - * constraint, so runAction refuses any action that names one in `guards` + * constraint it cannot lower, so runAction refuses an action that names one * rather than running it unchecked. A tool for such an action would fail every * time it was called, which is a bad thing to hand a caller that cannot see * why. So a tool carries `runnable`, and an adapter binds the ones that are; @@ -111,6 +111,18 @@ export interface ToolResult { * The statements ran and the commit itself failed to answer. */ unknown?: boolean; + /** + * Set when a rule the model states stopped the write. Distinct from a plain + * failure: the call was understood and answered, and the answer was no. + */ + refused?: boolean; + /** + * Set when somebody is entitled to say yes. The write did not happen and is + * not held anywhere; the caller's move is to get the approval and call again. + */ + needsApproval?: boolean; + /** Rules that did not hold and let the write through, each in its own words. */ + warnings?: string[]; /** What the caller should do next, when the outcome permits only one thing. */ whatToDo?: string; } @@ -204,15 +216,14 @@ function toolDescription( } -// Which rules a caller will meet. `guards` names the ones checked before the -// write; a constraint over stored state is checked after it and is not named -// here, because a caller cannot do anything differently about one. +// Which rules a caller will meet, so it can ask for something that satisfies +// them rather than learn them one refusal at a time. // -// An ADVISORY guard is not named either. A constraint whose `onViolation` is -// `warn` reports and lets the write through, so the runtime stands down and -// the call goes ahead -- telling a caller it is "gated" by a rule that gates -// nothing is the one kind of claim this file must not make. Saying less is the -// honest half of saying it accurately. +// An ADVISORY rule is left out. A constraint whose `onViolation` is `warn` +// reports the violation and lets the write through, so it gates nothing, and +// telling a caller it is "gated" by a rule that stops nothing is the one kind +// of claim this file must not make. Saying less is the honest half of saying +// it accurately. function gatingRules(action: Action, model: SemanticModel): string[] { const gating = new Set((model.constraints ?? []) .filter(c => c.onViolation !== 'warn') @@ -304,8 +315,38 @@ export function describeOutcome(outcome: ActionOutcome): ToolResult { } const result: ToolResult = {applied: true, actedOn}; if (outcome.commitTimestamp) result.committedAt = outcome.commitTimestamp; + // A rule that reported rather than stopped the write. It travels with the + // success because that is the whole of what `warn` asks for, and a caller + // that never sees it has been told the write was clean when it was not. + const warnings = (outcome.warnings ?? []).map(v => v.message); + // An advisory rule nothing could evaluate. It stopped nothing, so the + // write stands; saying so is the difference between a clean write and + // one whose advice was never sought. + for (const rule of outcome.unchecked ?? []) { + warnings.push( + `Advisory rule '${rule.constraint}' was not checked: ` + + `${rule.reason}. It reports rather than stops a write, so the ` + + `action went ahead.`); + } + if (warnings.length) result.warnings = warnings; return result; } + case 'refused': + // Not an error, and the difference matters to what the caller does next. + // A refusal is the model answering: retrying the same call reaches the + // same rule, so the only moves are to change the request or, where the + // rule allows one, to get an approval. + return { + applied: false, + refused: true, + ...(outcome.effect === 'escalate' ? {needsApproval: true} : {}), + reason: outcome.message, + whatToDo: outcome.effect === 'escalate' ? + 'Do not retry unchanged. Report what needs approving and why, ' + + 'or propose a request that stays within the rule.' : + 'Do not retry unchanged. Change the request so the rule holds, ' + + 'or report that it cannot be done. Nothing was written.', + }; case 'error': // `indeterminate` is the one outcome where "applied: false" would be a // lie the caller acts on: it retries, and the write lands twice. diff --git a/toolbox/mdcode/src/libts/semantic/runtime/constraint_eval.ts b/toolbox/mdcode/src/libts/semantic/runtime/constraint_eval.ts new file mode 100644 index 00000000..2661d60f --- /dev/null +++ b/toolbox/mdcode/src/libts/semantic/runtime/constraint_eval.ts @@ -0,0 +1,658 @@ +// Checking a model's constraints against a live store. +// +// A constraint is a logical invariant over the ontology (`Order.total >= 0`, +// `amount <= Order.total`). Enforcing one against a store means turning that +// logical statement into a query the store can answer. That is this module: +// given a model, an action and a constraint the action `guards`, it produces a +// PROBE -- a SELECT that returns the rows which VIOLATE the constraint. No rows +// means the rule holds. +// +// WHEN a probe runs is derived from the expression rather than authored, and +// the two answers are genuinely different checks: +// +// * A GUARD reads an action parameter (`amount <= Order.total`). It asks +// whether this call may proceed, so it runs BEFORE the writes, against the +// rows the arguments denote. A parameter is not in the store, so no +// post-state check could ask it. +// * An INVARIANT reads only stored state (`Order.total >= 0`). It asks +// whether the data is still sound, so it runs AFTER the writes and before +// the commit, when the new state exists to be read. +// +// Both run inside the action's own transaction, so a violation rolls back with +// everything else and no violating state is ever visible to another reader. +// +// Three properties matter more than expressive power: +// +// * The lowering FAILS CLOSED. An expression this module cannot lower does +// not quietly pass: it returns a reason, and the runtime refuses the action +// rather than running it unchecked. A gate that lets writes through is +// worse than no gate, because it is believed. +// * The lowering needs no argument VALUES. A probe is SQL with the action's +// own parameters left as `@name`, so the call that decides whether an +// action is runnable at all -- asked before any argument arrives, to decide +// whether to offer an agent the tool -- produces the very SQL that later +// runs. One answer, so the advertised verdict and the real one cannot +// drift. +// * A probe is SCOPED to the rows the call touches. An action writes a +// handful of rows, and a gate whose cost grows with the table is a gate +// that gets switched off. +// +// The grammar is small (see parseExpression): comparisons between a field, an +// action parameter and a literal, joined by AND/OR. It covers the rules an +// operational action actually trips -- an amount over a ceiling, a credit +// larger than what it credits -- and everything outside it is reported rather +// than approximated. Aggregates, parentheses and function calls are named as +// unlowerable, which refuses the action rather than guessing at it. + +import * as spanner from '../../gcp/spanner'; + +import {spannerTable} from '../binding'; +import { + Action, + Constraint, + constraintEvaluation, + Entity, + fieldBinding, + SemanticModel, + ViolationEffect, +} from '../ir'; +import {quoteIfReserved, referencedParameters} from '../sql_identifiers'; + + +// When a probe runs, relative to the action's own writes. +// +// - `before` the write has not happened. The probe reads the pre-state and +// the call's arguments, and a violation means the call is refused. +// - `after` the writes have run in the transaction but nothing is committed. +// The probe reads the post-state, and a violation rolls it back. +export type ProbeTiming = 'before'|'after'; + + +// A constraint lowered to SQL, ready to run inside the action's transaction. +export interface ConstraintProbe { + constraint: Constraint; + timing: ProbeTiming; + // The entity the probe reads, absent when the expression names none: a rule + // over the call's arguments alone, such as `amount <= 25`, reads no table. + entity?: string; + // A SELECT returning violating rows; an empty result means the rule holds. + // Parameters are the action's own, bound by the caller at run time. + sql: string; + // The columns `sql` selects, so a violation can name the rows it found + // rather than only reporting that one exists. + columns: string[]; +} + + +export type Lowering = { + ok: true; probe: ConstraintProbe; +}|{ + ok: false; + // Why this constraint cannot be checked here, phrased for whoever wrote the + // model: the runtime surfaces it verbatim when it refuses the action. + reason: string; +}; + + +// A rule that did not hold, in the terms a caller acts on. +export interface ConstraintViolation { + constraint: string; + effect: ViolationEffect; + // The constraint's own `description` where it has one, which is written as + // the instruction to the refused caller, followed by the citation. + message: string; + // The violating rows, each as its key values joined by '/'. Empty for a rule + // over the arguments alone, which has no row to name. + instances: string[]; +} + + +// A rule that was named as a guard and not evaluated. Only an advisory rule +// reaches this: one that stops the call and cannot be checked refuses the +// action instead. +export interface UncheckedRule { + constraint: string; + // Why it could not be checked, in the same words a refusal would have used. + reason: string; +} + + +// How many violating rows a probe returns. A gate needs enough to explain +// itself, not the whole violation set. +const PROBE_LIMIT = 5; + + +// The comparison operators the grammar accepts, longest first so `>=` is +// matched before `>`. +const OPERATORS = ['>=', '<=', '!=', '<>', '=', '>', '<'] as const; + + +/** + * Lowers every constraint `action` names in `guards`. + * + * Returns the probes it could build, the reasons for the ones it could not, + * and the advisory rules it had to leave unevaluated. A non-empty `errors` + * means the action cannot be run: the model says the call is checked, and a + * check that cannot be performed is not one. + * + * An ADVISORY rule -- `on_violation: warn` -- is the exception, and it is + * `unchecked` rather than an error. It reports and lets the write through, so + * being unable to evaluate it costs the caller a report and stops nothing; + * refusing over it would turn a rule the author wrote as advice into the one + * thing that makes the action unrunnable. It is still named, because a report + * that was owed and not made is news in its own right. + */ +export function lowerGuards(model: SemanticModel, action: Action): { + probes: ConstraintProbe[]; + errors: string[]; + unchecked: UncheckedRule[]; +} { + const probes: ConstraintProbe[] = []; + const errors: string[] = []; + const unchecked: UncheckedRule[] = []; + for (const name of action.guards ?? []) { + const constraint = (model.constraints ?? []).find(c => c.name === name); + if (!constraint) { + // Not classifiable as advisory: the model declares nothing by this name, + // so there is no `on_violation` to read, and guessing is not on offer. + errors.push( + `action '${action.name}' is guarded by '${name}', which this model ` + + `does not declare`); + continue; + } + const lowered = lowerGuard(model, action, constraint); + if (lowered.ok) { + probes.push(lowered.probe); + } else if (effectOf(constraint) === 'warn') { + unchecked.push({constraint: name, reason: lowered.reason}); + } else { + errors.push(lowered.reason); + } + } + return {probes, errors, unchecked}; +} + + +/** Lowers one constraint as a gate on `action`, or says why it cannot be. */ +export function lowerGuard( + model: SemanticModel, action: Action, constraint: Constraint): Lowering { + const fail = (reason: string): Lowering => ({ + ok: false, + reason: `constraint '${constraint.name}' cannot be checked: ${reason}`, + }); + + // A judged rule is settled by a language model reading the proposed change. + // Nothing here calls one, and the honest report of that is a refusal: the + // alternative is an action whose model says it is judged running unjudged. + if (constraintEvaluation(constraint) === 'judged') { + return fail( + `it is settled by judgment rather than by an expression, and this ` + + `runtime runs no judge`); + } + + const parameters = new Map(action.parameters.map(p => [p.name, p])); + const parsed = parseExpression(constraint.expression ?? '', parameters); + if ('error' in parsed) return fail(parsed.error); + + const entityNames = new Set(); + for (const comparison of parsed.comparisons) { + for (const operand of [comparison.left, comparison.right]) { + if (operand.kind === 'field') entityNames.add(operand.entity); + } + } + if (entityNames.size > 1) { + return fail( + `it spans ${[...entityNames].sort().join(' and ')}; a probe reads ` + + `one entity's table, so write one constraint per entity and list ` + + `them together in 'guards'`); + } + + // A rule that reads a parameter asks about this call, so it is answered + // before the write; one that reads only stored state asks whether the data + // is sound, which only the post-state can answer. + const readsParameter = parsed.comparisons.some( + c => c.left.kind === 'parameter' || c.right.kind === 'parameter'); + const timing: ProbeTiming = readsParameter ? 'before' : 'after'; + + if (!entityNames.size) { + // No table to read: the rule is entirely about the call's own arguments. + // `UNNEST([1])` is the one-row source GoogleSQL needs for a SELECT that + // has a WHERE and nothing to select from. + const predicate = renderPredicate(parsed, new Map()); + return { + ok: true, + probe: { + constraint, + timing, + sql: `SELECT 1 AS violated FROM UNNEST([1]) WHERE ${ + violating(predicate)}`, + columns: ['violated'], + }, + }; + } + + const entityName = [...entityNames][0]; + const entity = (model.entities ?? []).find(e => e.name === entityName); + if (!entity) return fail(`'${entityName}' is not an entity of this model`); + if (entity.abstract) { + return fail(`'${entityName}' is abstract, so it has no table to read`); + } + + const columns = new Map(); + for (const comparison of parsed.comparisons) { + for (const operand of [comparison.left, comparison.right]) { + if (operand.kind !== 'field') continue; + if (columns.has(operand.field)) continue; + const column = columnFor(entity, operand.field); + if ('error' in column) return fail(column.error); + columns.set(operand.field, column.column); + } + } + + const scope = scopeToTouchedRows(action, entity); + if ('error' in scope) return fail(scope.error); + + const keys = keyColumns(entity); + if ('error' in keys) return fail(keys.error); + + const warnings: string[] = []; + const table = + spannerTable(entity.dataSource, warnings, `entity '${entity.name}'`); + if (warnings.length) { + return fail(`'${entityName}' has no usable table (${warnings.join('; ')})`); + } + + const predicate = renderPredicate(parsed, columns); + return { + ok: true, + probe: { + constraint, + timing, + entity: entityName, + sql: `SELECT ${keys.columns.join(', ')} FROM ${table} WHERE ${ + scope.predicate} AND ${violating(predicate)} LIMIT ${PROBE_LIMIT}`, + columns: keys.columns, + }, + }; +} + + +/** + * `probe` as a statement, carrying the argument values it reads. + * + * Filtered to the parameters the SQL actually names: a statement carrying one + * it never reads is a statement the store may refuse, and an action's + * parameter list is wider than any single rule. + */ +export function probeStatement( + probe: ConstraintProbe, params: Record, + types: Record): spanner.Statement { + const statement: spanner.Statement = {sql: probe.sql}; + const used: Record = {}; + const usedTypes: Record = {}; + for (const name of new Set(referencedParameters(probe.sql))) { + if (!(name in params)) continue; + used[name] = params[name]; + if (types[name]) usedTypes[name] = types[name]; + } + if (Object.keys(used).length) { + statement.params = used; + statement.paramTypes = usedTypes; + } + return statement; +} + + +/** + * A violation of `probe`, given the rows it returned. + * + * The constraint's `description` leads, because it is the model author's own + * words about what the caller should do differently; the name and the + * expression follow as the citation for it. + */ +export function violationFrom( + probe: ConstraintProbe, rows: string[][]): ConstraintViolation { + const constraint = probe.constraint; + const lead = constraint.description?.trim() || + `Constraint '${constraint.name}' does not hold.`; + const parts = + [lead, `Stopped by '${constraint.name}' (${constraint.expression}).`]; + // Rows are named only when they identify something: the one-row result of a + // rule over the arguments alone says nothing a reader can use. + const instances = probe.entity ? rows.map(row => row.join('/')) : []; + if (instances.length) { + parts.push(`Violating ${probe.entity}: ${instances.join(', ')}.`); + } + return { + constraint: constraint.name, + effect: effectOf(constraint), + message: parts.join(' '), + instances, + }; +} + + +/** + * What a violated constraint does to the write. + * + * An `expression` that does not say defaults to `reject`, which is the safe + * reading of an author who did not say. See VIOLATION_EFFECTS in ir.ts. + */ +export function effectOf(constraint: Constraint): ViolationEffect { + return constraint.onViolation ?? 'reject'; +} + + +// Harshest first. A call that trips two rules gets the stricter answer: being +// told a supervisor could approve a write another rule forbids outright would +// send the caller to ask for something nobody can give. +const EFFECT_ORDER: ViolationEffect[] = ['reject', 'escalate', 'warn']; + + +/** The strictest effect among `violations`, or null if there are none. */ +export function strictestEffect(violations: readonly ConstraintViolation[]): + ViolationEffect|null { + for (const effect of EFFECT_ORDER) { + if (violations.some(v => v.effect === effect)) return effect; + } + return null; +} + + +// NOT COALESCE(p, FALSE) rather than a plain NOT: SQL's three-valued logic +// makes `NULL >= 0` unknown and `NOT unknown` unknown too, so a NULL column +// would slip past a plain negation. Reading unknown as "did not satisfy the +// rule" makes the row a violation, which is the fail-closed answer a gate owes. +function violating(predicate: string): string { + return `NOT COALESCE(${predicate}, FALSE)`; +} + + +// Restricts the probe to the rows this call touches. +// +// An action names the rows it acts on through its entity-typed parameters, and +// that reference is what makes the probe cheap and its answer relevant. +// Without one, `amount <= Order.total` would be asked of every order in the +// table and fail on the first unrelated one, so a constraint over an entity +// the action does not take as a parameter is refused rather than widened into +// a table scan. Checking stored state at large is a different binding point -- +// a conformance sweep over the data rather than a gate on one call -- and it +// needs its own reference instead of this one silently standing in for it. +function scopeToTouchedRows( + action: Action, entity: Entity): {predicate: string}|{error: string} { + const param = + action.parameters.find(p => p.isEntityRef && p.type === entity.name); + if (!param) { + return { + error: `it reads ${entity.name}, and action '${action.name}' takes no ` + + `${entity.name} parameter, so the probe could not be limited to ` + + `the rows this call touches`, + }; + } + const keys = keyColumns(entity); + if ('error' in keys) return {error: keys.error}; + if (keys.columns.length !== 1) { + return { + error: `${entity.name} has a ${keys.columns.length}-part key, and the ` + + `runtime binds an object reference as a single value`, + }; + } + return {predicate: `${keys.columns[0]} = @${param.name}`}; +} + + +// The physical column behind `fieldName`, or why there is none. A bare column +// is required: a field bound to an expression (`price * quantity`) would need +// that expression inlined and re-resolved, which this grammar does not do. +function columnFor(entity: Entity, fieldName: string): + {column: string}|{error: string} { + const field = entity.fields.find(f => f.name === fieldName); + if (!field) { + return {error: `${entity.name} declares no field '${fieldName}'`}; + } + // No binding is what unbound means: the profile in force bound nothing to + // this field, so there is no column to read the rule against. + const binding = (fieldBinding(field) ?? '').trim(); + if (!binding) { + return { + error: `${entity.name}.${fieldName} is unbound under this profile, so ` + + `there is nothing to read it from`, + }; + } + if (!/^[A-Za-z_]\w*$/.test(binding)) { + return { + error: `${entity.name}.${fieldName} is bound to an expression (${ + binding}) rather than to a column`, + }; + } + return {column: quoteIfReserved(binding)}; +} + + +// The entity's key columns, resolved through its fields. +function keyColumns(entity: Entity): {columns: string[]}|{error: string} { + if (!entity.keys?.length) { + return { + error: `${entity.name} declares no key, so a violation could not be ` + + `attributed to a row`, + }; + } + const columns: string[] = []; + for (const key of entity.keys) { + const column = columnFor(entity, key); + if ('error' in column) return {error: `its key ${column.error}`}; + columns.push(column.column); + } + return {columns}; +} + + +// An operand of a comparison: a field of an entity, an action parameter, or a +// literal already in SQL form. +type Operand = { + kind: 'field'; entity: string; field: string; +}|{ + kind: 'parameter'; name: string; +}|{ + kind: 'literal'; text: string; +}; + + +interface Comparison { + left: Operand; + right: Operand; + operator: string; +} + + +// Comparisons and the logical operators between them: `joiners[i]` sits +// between `comparisons[i]` and `comparisons[i + 1]`. +interface ParsedExpression { + comparisons: Comparison[]; + joiners: string[]; +} + + +// Parses a constraint expression. +// +// The grammar: +// +// expression := comparison (('AND' | 'OR') comparison)* +// comparison := operand operand +// operand := . | | literal +// op := >= | <= | != | <> | = | > | < +// literal := a number, a single-quoted string, TRUE, FALSE or NULL +// +// `= NULL` and `!= NULL` read as null tests and lower to IS NULL / IS NOT NULL. +// Parentheses, function calls, aggregates, IN, BETWEEN, LIKE and metric +// references are all outside the grammar, on purpose. Each is a real thing a +// constraint might want and each needs a decision this module does not make -- +// how an aggregate is evaluated inside a row-level probe, for one -- so each is +// refused with a reason rather than half-handled. +function parseExpression( + expression: string, + parameters: Map): ParsedExpression|{error: string} { + const text = expression.trim(); + if (!text) return {error: 'it declares no expression'}; + if (text.includes('==')) { + return { + error: `it writes '==' (${text}); equality in the expression language ` + + `is a single '='`, + }; + } + if (/[()]/.test(text)) { + return { + error: `it uses parentheses or a function call (${ + text}), which the grammar does not parse`, + }; + } + + const split = splitOnLogicalOperators(text); + const comparisons: Comparison[] = []; + for (const segment of split.parts) { + const comparison = parseComparison(segment, parameters); + if ('error' in comparison) return comparison; + comparisons.push(comparison); + } + return {comparisons, joiners: split.joiners}; +} + + +// Splits on top-level AND/OR, matched as whole words so a field named `brand` +// survives. There are no parentheses to nest -- parseExpression refuses them -- +// so every operator found is top level. +function splitOnLogicalOperators(expression: string): + {parts: string[]; joiners: string[]} { + const parts: string[] = []; + const joiners: string[] = []; + const pattern = /\s+(AND|OR)\s+/gi; + let last = 0; + let match: RegExpExecArray|null; + while ((match = pattern.exec(expression)) !== null) { + parts.push(expression.slice(last, match.index)); + joiners.push(match[1].toUpperCase()); + last = match.index + match[0].length; + } + parts.push(expression.slice(last)); + return {parts, joiners}; +} + + +function parseComparison( + segment: string, + parameters: Map): Comparison|{error: string} { + const text = segment.trim(); + const found = findOperator(text); + if (!found) { + return { + error: `'${text}' is not a comparison (expected one of ${ + OPERATORS.join(', ')})`, + }; + } + const leftText = text.slice(0, found.index).trim(); + const rightText = text.slice(found.index + found.operator.length).trim(); + if (!leftText) return {error: `'${text}' has nothing left of the operator`}; + if (!rightText) return {error: `'${text}' has nothing right of the operator`}; + + const left = parseOperand(leftText, parameters); + if ('error' in left) return {error: `in '${text}', ${left.error}`}; + const right = parseOperand(rightText, parameters); + if ('error' in right) return {error: `in '${text}', ${right.error}`}; + + const operator = found.operator === '<>' ? '!=' : found.operator; + + // GoogleSQL refuses `col = NULL` outright rather than evaluating it to + // unknown, so lowering it verbatim would emit a probe that cannot run. An + // author writing `Order.closedOn != NULL` means the column must be + // populated, which SQL spells IS NOT NULL -- so the two operators with a + // null-test reading are translated and the four without one are refused, an + // ordering comparison against NULL having no meaning to preserve. + const isNull = (operand: Operand) => + operand.kind === 'literal' && /^NULL$/i.test(operand.text); + if (isNull(left) || isNull(right)) { + if (operator !== '=' && operator !== '!=') { + return { + error: `'${text}' compares with NULL using '${operator}', which has ` + + `no meaning; write '= NULL' or '!= NULL' to ask whether the ` + + `field is set`, + }; + } + return { + left: isNull(left) ? right : left, + right: {kind: 'literal', text: 'NULL'}, + operator: operator === '=' ? 'IS' : 'IS NOT', + }; + } + + return {left, right, operator}; +} + + +function parseOperand( + text: string, + parameters: Map): Operand|{error: string} { + const field = text.match(/^([A-Za-z_]\w*)\.([A-Za-z_]\w*)$/); + if (field) return {kind: 'field', entity: field[1], field: field[2]}; + if (isLiteral(text)) return {kind: 'literal', text}; + if (/^[A-Za-z_]\w*$/.test(text)) { + if (parameters.has(text)) return {kind: 'parameter', name: text}; + return { + error: `'${text}' is not a parameter of this action; a field is ` + + `written .`, + }; + } + return {error: `'${text}' is not a field, a parameter or a literal`}; +} + + +// The first comparison operator in `text`, longest match first so `>=` is not +// read as `>` with a stray `=` after it. +function findOperator(text: string): {operator: string; index: number}|null { + let best: {operator: string; index: number}|null = null; + for (const operator of OPERATORS) { + const index = text.indexOf(operator); + if (index < 0) continue; + if (!best || index < best.index || + (index === best.index && operator.length > best.operator.length)) { + best = {operator, index}; + } + } + return best; +} + + +// A literal the probe embeds verbatim, restricted to shapes with no quoting +// hazard: a number, a single-quoted string with no embedded quote or +// backslash, or one of the three keywords. Anything else is refused rather +// than escaped, because a constraint expression is model text and a surprising +// escape is harder to notice than a refusal. +function isLiteral(text: string): boolean { + if (/^[-+]?\d+(\.\d+)?$/.test(text)) return true; + if (/^'[^'\\]*'$/.test(text)) return true; + return /^(TRUE|FALSE|NULL)$/i.test(text); +} + + +// Renders the parsed expression against the physical columns. Each comparison +// is parenthesized, so a mixed AND/OR expression keeps the precedence the SQL +// engine gives it rather than one this module invents. +function renderPredicate( + parsed: ParsedExpression, columns: Map): string { + const render = (operand: Operand): string => { + switch (operand.kind) { + case 'field': + return columns.get(operand.field)!; + case 'parameter': + return `@${operand.name}`; + case 'literal': + return operand.text; + } + }; + const parts = parsed.comparisons.map( + c => `(${render(c.left)} ${c.operator} ${render(c.right)})`); + let out = parts[0]; + for (let i = 1; i < parts.length; i++) { + out = `${out} ${parsed.joiners[i - 1]} ${parts[i]}`; + } + return out; +} diff --git a/toolbox/mdcode/src/libts/semantic/runtime/run_action.ts b/toolbox/mdcode/src/libts/semantic/runtime/run_action.ts index 020a2c82..0cc02a52 100644 --- a/toolbox/mdcode/src/libts/semantic/runtime/run_action.ts +++ b/toolbox/mdcode/src/libts/semantic/runtime/run_action.ts @@ -23,16 +23,21 @@ // which this module cannot call and could not roll back if it did; for those // the caller supplies a handler that produces the statements. // -// What this does NOT do yet: evaluate the model's constraints. A constraint is -// still text nothing checks, so an action that NAMES one in `guards` is REFUSED -// here rather than run unchecked -- see `unsafeToRunUnchecked`. Refusing is the -// point. A model that declares a rule and a runtime that quietly ignores it is -// worse than no runtime at all, because the model states the call is checked +// Where the model's rules come in. A constraint takes effect here through +// `guards` on the action: each rule the action names is lowered to a probe and +// run inside this same transaction -- before the write when it reads one of the +// call's arguments, after the write when it reads only stored state -- and a +// violation rolls the whole thing back and reports the rule's own words. See +// constraint_eval.ts. +// +// A rule that cannot be lowered REFUSES the action rather than letting it run +// unchecked. A model that declares a rule and a runtime that quietly ignores it +// is worse than no runtime at all, because the model states the call is checked // and nothing says otherwise. // // A constraint no action names gates nothing here, because it gates nothing // anywhere: a rule takes effect where something references it, and `guards` is -// that reference for an action (see Action.guards in ir.ts). Refusing on a +// that reference for an action (see Action.guards in ir.ts). Checking a // constraint that merely reads data the action writes would mean publishing a // rule silently stopped calls that succeeded the day before, which is the // property that reference rule exists to guarantee. @@ -50,6 +55,15 @@ import { } from '../ir'; import {quoteIfReserved, referencedParameters} from '../sql_identifiers'; +import { + ConstraintViolation, + lowerGuards, + UncheckedRule, + probeStatement, + ProbeTiming, + strictestEffect, + violationFrom, +} from './constraint_eval'; import {runtimeClient, SemanticRuntime} from './runtime'; @@ -92,6 +106,32 @@ export type ActionOutcome = { status: 'committed'; commitTimestamp?: string; refs: Record; + // Rules that did not hold and let the write through anyway, which is what + // `on_violation: warn` asks for. Present only when there are some. + warnings?: ConstraintViolation[]; + // Advisory rules the action named and this runtime could not evaluate. They + // stop nothing, so the write stands, and they are reported rather than + // dropped: a report the model asked for and did not get is worth knowing. + unchecked?: UncheckedRule[]; +}|{ + // A rule the model states stopped the write. Kept apart from `error` because + // it is not a failure: the runtime did what the model asked of it, and the + // caller's next move is to change the request or to get an approval rather + // than to look for a fault. Nothing was written. + status: 'refused'; + // The strictest effect among the rules that stopped the call. `reject` is + // final. `escalate` means somebody is entitled to say yes, though nothing + // here holds the write while they decide: it is rolled back, and the action + // is run again once it is approved. + effect: 'reject'|'escalate'; + // The rules that stopped it. A rule violated on the same call whose effect is + // `warn` is not among them: it asks for the write to proceed and be reported, + // and nothing was committed for it to qualify. + violations: ConstraintViolation[]; + // Every violation as one piece of text, each rule's own description first, + // for a caller that reports rather than routes. + message: string; + refs: Record; }|{ status: 'error'; // A failure that stopped the write: an argument that resolved to nothing, an @@ -138,6 +178,13 @@ export async function runAction(opts: RunActionOptions): const refusal = whyRefusedWithoutRunning(model, action, opts.handler); if (refusal) return {status: 'error', message: refusal}; + // Lowered before anything opens, and by the same call the refusal check just + // made: the probes that run are the ones it proved buildable, so an action + // reported as runnable cannot then meet a rule that turns out to be + // uncheckable. + const lowered = lowerGuards(model, action); + const probes = lowered.probes; + // Also before touching the store, because there may be none to touch. const client = runtimeClient(opts.runtime); if ('error' in client) return {status: 'error', message: client.error}; @@ -204,6 +251,34 @@ export async function runAction(opts: RunActionOptions): } const refs = resolved.refs; + // A probe binds the action's own parameters, so a guarded action is + // bound here even when a handler is what supplies the writes. + let probeValues: Bindings|undefined; + if (probes.length) { + const bound = bindArguments(model, action, args, refs); + if ('error' in bound) { + return await rollback({status: 'error', message: bound.error}); + } + probeValues = bound; + } + const check = async (timing: ProbeTiming) => { + const violations: ConstraintViolation[] = []; + for (const probe of probes) { + if (probe.timing !== timing) continue; + const rows = await query(probeStatement( + probe, probeValues!.params, probeValues!.types)); + if (rows.length) violations.push(violationFrom(probe, rows)); + } + return violations; + }; + + // Before the write, because a rule that reads an argument is asking + // whether this call may proceed at all, and a call that may not should + // cost the store no writes. + const beforeWrite = await check('before'); + const refusedBefore = refusedBy(action, beforeWrite, refs); + if (refusedBefore) return await rollback(refusedBefore); + // The bindings exist to fill the model's OWN statements, so they are // built only when the model is what supplies them. A handler is given // `refs` whole and may write a composite key, which this pass refuses @@ -225,6 +300,14 @@ export async function runAction(opts: RunActionOptions): await run(stmt); } + // After the write and still inside the transaction, which is the one + // moment the post-state both exists and can still be undone. + const afterWrite = await check('after'); + const refusedAfter = refusedBy(action, afterWrite, refs); + if (refusedAfter) return await rollback(refusedAfter); + const warnings = + [...beforeWrite, ...afterWrite].filter(v => v.effect === 'warn'); + // Deliberately NOT rolled back. Once commit has been called the // transaction's fate is the server's, and a deadline or a 5xx is // exactly the shape of failure Spanner returns for a commit that @@ -282,6 +365,8 @@ export async function runAction(opts: RunActionOptions): status: 'committed', commitTimestamp: committed.result?.commitTimestamp, refs, + ...(warnings.length ? {warnings} : {}), + ...(lowered.unchecked.length ? {unchecked: lowered.unchecked} : {}), } as ActionOutcome; } catch (err) { try { @@ -365,13 +450,14 @@ export function whyRefusedWithoutRunning( `that performs the write as DML, or declare the action with a 'sql' ` + `executor.`; } - const unchecked = unsafeToRunUnchecked(model, action); + const unchecked = guardsNotCheckable(model, action); if (unchecked) return unchecked; - // The refusals left are about filling the model's OWN statements, so they - // apply only when the model is what supplies them. A handler writes its own - // DML, is handed `refs` whole, and may well spell a composite key across - // several parameters -- none of what follows is owed by it. - if (handler) return null; + // The refusals left are about filling statements with the action's own + // parameters. A handler writes its own DML, is handed `refs` whole, and may + // well spell a composite key across several parameters, so none of what + // follows is owed by it -- unless the action is guarded, because a probe + // binds those parameters whoever supplies the write. + if (handler && !(action.guards ?? []).length) return null; return unbindableByThisRuntime(model, action); } @@ -411,45 +497,57 @@ function unbindableByThisRuntime( } -// Why a rule the model states has to stop `action`, or null if none does. +// Why a rule the model states has to stop `action` from running at all, or null +// if none does. // // One question, and it is narrower than "could some rule bear on this write": -// does the action name a constraint that has to be checked before it runs, -// which nothing can check yet. `guards` is what gives a constraint effect over -// a call -- a rule no action names is a catalogued rule no call consults -- so -// the model's own answer to "what gates this" is the list, and reading further -// would be this module inventing an obligation the model does not state. +// does the action name a constraint this runtime cannot check. `guards` is what +// gives a constraint effect over a call -- a rule no action names is a +// catalogued rule no call consults -- so the model's own answer to "what gates +// this" is the list, and reading further would be this module inventing an +// obligation the model does not state. +// +// A guard it CAN check is not a refusal. It is lowered to a probe and run, and +// the call goes ahead or does not on what the probe finds. What refuses is a +// guard that cannot be lowered -- a rule settled by judgment, an expression +// outside the grammar, a field the profile bound to nothing -- because running +// the action then means running it unchecked, which is not what the model says +// it is. // // An action naming no guard therefore runs. That is not this module judging the // write safe; it is the model saying no rule gates the call. What the write -// does is the author's, which is what `affects` describes and what the -// evaluator will check against the statements once it exists. -function unsafeToRunUnchecked( - model: SemanticModel, action: Action): string|null { - // A guard names a constraint the author says is checked before the call. - // One whose `onViolation` is `warn` reports rather than refuses, so an - // evaluator would let the write through, and refusing here would make a - // model that states advisory rules permanently unrunnable. Only a name the - // model declares AS advisory stands down -- a guard naming nothing this - // model declares still refuses, because it is not something to guess about. - const advisory = new Set((model.constraints ?? []) - .filter(c => c.onViolation === 'warn') - .map(c => c.name)); - const guards = (action.guards ?? []).filter(g => !advisory.has(g)); - if (guards.length) { - return `Action '${action.name}' is guarded by ${quoteList(guards)}, and ` + - `this runtime does not evaluate constraints yet. Running it would ` + - `apply a write the model says must be checked first, so it is ` + - `refused rather than run unchecked.`; - } - return null; +// does is the author's, which is what `affects` describes. +function guardsNotCheckable(model: SemanticModel, action: Action): string|null { + const errors = lowerGuards(model, action).errors; + if (!errors.length) return null; + return `Action '${action.name}' cannot be run: ${errors.join('; ')}. ` + + `Running it would apply a write the model says is checked first, so ` + + `it is refused rather than run unchecked.`; } -function quoteList(names: readonly string[]): string { - const quoted = names.map(n => `'${n}'`); - if (quoted.length === 1) return quoted[0]; - return `${quoted.slice(0, -1).join(', ')} and ${quoted[quoted.length - 1]}`; +// What a set of violations does to the write, or null if it goes ahead. +// +// `warn` is the one effect that stops nothing: the model asked for the +// violation to be reported and the write to proceed, so it rides out on a +// committed outcome instead of stopping here. The rest roll back, and the +// strictest effect among the rules that fired is what the action does -- being +// told a supervisor could approve a write that another rule forbids outright +// would send the caller to ask for something nobody can give. +function refusedBy( + action: Action, violations: ConstraintViolation[], + refs: Record): ActionOutcome|null { + const effect = strictestEffect(violations); + if (effect !== 'reject' && effect !== 'escalate') return null; + const stopping = violations.filter(v => v.effect !== 'warn'); + const reasons = stopping.map(v => v.message).join(' '); + const message = effect === 'reject' ? + `Action '${action.name}' was refused and nothing was written. ${ + reasons}` : + `Action '${action.name}' needs an approval, and nothing was written. ${ + reasons} Nothing is held while somebody decides: the transaction ` + + `was rolled back, so run the action again once it is approved.`; + return {status: 'refused', effect, violations: stopping, message, refs}; } diff --git a/toolbox/mdcode/src/tool/commands.ts b/toolbox/mdcode/src/tool/commands.ts index a08c2f25..11b9970e 100644 --- a/toolbox/mdcode/src/tool/commands.ts +++ b/toolbox/mdcode/src/tool/commands.ts @@ -1553,12 +1553,26 @@ async function runOneAction( console.error(`Error: ${outcome.message}`); return 1; } + // A refusal exits non-zero, because the caller asked for a write and did not + // get one -- but it is not reported as an error, because nothing went wrong. + // The model was consulted and said no, which is the runtime working. + if (outcome.status === 'refused') { + console.log(`Refused (${outcome.effect}): ${outcome.message}`); + return 1; + } // What each reference turned out to be. An agent said "Alice"; this is the // row it wrote to, which is the part worth reading back. for (const [param, ref] of Object.entries(outcome.refs)) { console.log( ` ${param}: '${ref.input}' -> ${ref.entity} ${ref.keys.join('/')}`); } + for (const warning of outcome.warnings ?? []) { + console.log(` warning: ${warning.message}`); + } + for (const rule of outcome.unchecked ?? []) { + console.log(` not checked: advisory rule '${rule.constraint}' (${ + rule.reason})`); + } console.log(`Committed${ outcome.commitTimestamp ? ` at ${outcome.commitTimestamp}` : ''}.`); return 0; diff --git a/toolbox/mdcode/tests/libts/semantic/runtime/agent_tools.test.ts b/toolbox/mdcode/tests/libts/semantic/runtime/agent_tools.test.ts index 50dbe1ec..a888746e 100644 --- a/toolbox/mdcode/tests/libts/semantic/runtime/agent_tools.test.ts +++ b/toolbox/mdcode/tests/libts/semantic/runtime/agent_tools.test.ts @@ -192,12 +192,26 @@ describe('a tool this runtime would refuse', () => { expect(tool.description).not.toContain('will not work'); }); - test('a guarded action is not runnable while nothing checks the guard', () => { + test('a guard this runtime can check leaves the action runnable', () => { + // Being checked is not being blocked. The rule is lowered to a probe and + // run when the action runs, and a tool marked unrunnable over it would + // withhold a call that works. const guarded = withExecutor( model, {...RUNNABLE, guards: ['RequestedQuantityIsPositive']}); const [tool] = actionTools({runtime: rt(guarded)}); + expect(tool.runnable).toBe(true); + expect(tool.unavailable).toBeUndefined(); + }); + + test('a guard this runtime cannot check is not runnable', () => { + // The fixture settles this one by judgment, and there is no judge here. + // The model says the call is checked, so running it would run it + // unchecked, which is not the action the model describes. + const guarded = withExecutor( + model, {...RUNNABLE, guards: ['LargeOrderIsJustified']}); + const [tool] = actionTools({runtime: rt(guarded)}); expect(tool.runnable).toBe(false); - expect(tool.unavailable).toContain('RequestedQuantityIsPositive'); + expect(tool.unavailable).toContain('LargeOrderIsJustified'); expect(tool.unavailable).toContain('refused rather than run unchecked'); }); @@ -716,9 +730,22 @@ describe('sorting the tools an adapter can actually offer', () => { expect(withheld).toEqual([]); }); - test('a guarded action is withheld, and says why', () => { - // A guard is the case that matters: the model says this write must be - // checked, no checker exists, so the tool must not be offered as callable. + test('an action whose guard cannot be checked is withheld, and says why', + () => { + // The model says this write is checked before it runs and the rule is + // settled by judgment, so the tool must not be offered as callable. + const guarded = withExecutor( + model, {...RUNNABLE, guards: ['LargeOrderIsJustified']}); + const {callable, withheld} = + callableTools(modelTools({runtime: rt(guarded)})); + expect(callable.map(t => t.name)).toEqual([ + 'find_orders', 'find_customer' + ]); + expect(withheld.map(t => t.name)).toEqual(['place_order']); + expect(withheld[0].unavailable).toContain('LargeOrderIsJustified'); + }); + + test('an action whose guard can be checked is offered', () => { const guarded = { ...withExecutor(model, {...RUNNABLE, guards: ['UnderReview']}), constraints: [{ @@ -729,9 +756,8 @@ describe('sorting the tools an adapter can actually offer', () => { }; const {callable, withheld} = callableTools(modelTools({runtime: rt(guarded)})); - expect(callable.map(t => t.name)).toEqual(['find_orders', 'find_customer']); - expect(withheld.map(t => t.name)).toEqual(['place_order']); - expect(withheld[0].unavailable).toContain('UnderReview'); + expect(callable.map(t => t.name)).toContain('place_order'); + expect(withheld).toEqual([]); }); test('the instruction is carried through untouched', () => { diff --git a/toolbox/mdcode/tests/libts/semantic/runtime/constraint_eval.test.ts b/toolbox/mdcode/tests/libts/semantic/runtime/constraint_eval.test.ts new file mode 100644 index 00000000..5deee2cb --- /dev/null +++ b/toolbox/mdcode/tests/libts/semantic/runtime/constraint_eval.test.ts @@ -0,0 +1,478 @@ +// Lowering a constraint to a probe. +// +// A probe is a SELECT that returns the rows breaking the rule, so an empty +// result means the rule holds. Two things are under test: the SQL a given +// expression turns into, and -- at least as important -- which expressions are +// refused. The lowering fails closed, so every refusal here is an action the +// runtime will decline to run rather than run unchecked, and each one has to +// say enough for whoever wrote the model to fix it. +// +// No store is involved. A probe is built from the model and the action alone, +// with the arguments left as parameters, which is what lets the same call +// answer both "may this action be offered at all" and "what shall I run". + +import {describe, expect, test} from 'bun:test'; + +import {Action, Constraint, Entity, SemanticModel} from '../../../../src/libts/semantic/ir'; +import { + ConstraintProbe, + effectOf, + lowerGuard, + lowerGuards, + Lowering, + probeStatement, + strictestEffect, + violationFrom, +} from '../../../../src/libts/semantic/runtime/constraint_eval'; + + +const ORDER: Entity = { + name: 'Order', + dataSource: 'demo.commerce.Orders', + keys: ['key'], + fields: [ + {name: 'key', expression: 'OrderId'}, + {name: 'total', expression: 'Total'}, + {name: 'closedOn', expression: 'ClosedOn'}, + {name: 'status', expression: 'Status', type: 'String'}, + // Bound to an expression rather than to a column. + {name: 'margin', expression: 'Price * Quantity'}, + // Bound to nothing: the profile in force gives it no column. + {name: 'memo'}, + ], +}; + +const ENTRY: Entity = { + name: 'Entry', + dataSource: 'demo.commerce.LedgerEntry', + keys: ['key'], + fields: [ + {name: 'key', expression: 'EntryId'}, + {name: 'amount', expression: 'Amount'}, + ], +}; + +const LINE: Entity = { + name: 'Line', + dataSource: 'demo.commerce.OrderLine', + keys: ['orderKey', 'lineNo'], + fields: [ + {name: 'orderKey', expression: 'OrderId'}, + {name: 'lineNo', expression: 'LineNo'}, + {name: 'qty', expression: 'Qty'}, + ], +}; + +const PARTY: Entity = { + name: 'Party', + dataSource: '', + keys: [], + abstract: true, + fields: [{name: 'name', expression: 'Name'}], +}; + +const ISSUE_CREDIT: Action = { + name: 'IssueCredit', + description: 'Credit an order.', + executor: { + kind: 'sql', + sql: { + statements: [ + 'INSERT INTO LedgerEntry (EntryId, OrderId, Amount) ' + + 'VALUES (@newEntryKey, @order, @amount)', + ], + }, + }, + parameters: [ + {name: 'order', type: 'Order', isEntityRef: true}, + {name: 'amount', type: 'Decimal', isEntityRef: false}, + ], +}; + +function modelWith(over: Partial = {}): SemanticModel { + return { + name: 'commerce', + entities: [ORDER, ENTRY, LINE, PARTY], + relationships: [], + metrics: [], + actions: [ISSUE_CREDIT], + ...over, + }; +} + +function lowerRule( + constraint: Constraint, action: Action = ISSUE_CREDIT, + model: SemanticModel = modelWith()): Lowering { + return lowerGuard(model, action, constraint); +} + +function lower(expression: string, action?: Action): Lowering { + return lowerRule({name: 'Rule', expression}, action); +} + +function probeOf(lowered: Lowering): ConstraintProbe { + if (!lowered.ok) throw new Error(`expected a probe: ${lowered.reason}`); + return lowered.probe; +} + +function reasonOf(lowered: Lowering): string { + if (lowered.ok) throw new Error(`expected a refusal: ${lowered.probe.sql}`); + return lowered.reason; +} + + +describe('what the probe reads and when it runs', () => { + test('a rule over stored state alone reads the table, after the write', + () => { + const probe = probeOf(lower('Order.total >= 0')); + expect(probe.timing).toBe('after'); + expect(probe.entity).toBe('Order'); + expect(probe.sql).toBe( + 'SELECT OrderId FROM Orders WHERE OrderId = @order ' + + 'AND NOT COALESCE((Total >= 0), FALSE) LIMIT 5'); + }); + + test('a rule over an argument alone reads no table, before the write', () => { + // GoogleSQL needs a source for a SELECT with a WHERE and nothing to read, + // and one row is all a question about the arguments takes. + const probe = probeOf(lower('amount <= 25')); + expect(probe.timing).toBe('before'); + expect(probe.entity).toBeUndefined(); + expect(probe.sql).toBe( + 'SELECT 1 AS violated FROM UNNEST([1]) ' + + 'WHERE NOT COALESCE((@amount <= 25), FALSE)'); + }); + + test('a rule comparing an argument to stored state runs before the write', + () => { + // A parameter is not in the store, so no post-state check could ask + // it. Asking before the write is also the only timing that can stop + // the call without undoing anything. + const probe = probeOf(lower('amount <= Order.total')); + expect(probe.timing).toBe('before'); + expect(probe.sql).toContain('NOT COALESCE((@amount <= Total), FALSE)'); + }); + + test('the probe is scoped to the rows the call names', () => { + // Without the scope this is a table scan holding read locks for the length + // of the write, which is how a gate gets switched off. + expect(probeOf(lower('Order.total >= 0')).sql) + .toContain('WHERE OrderId = @order AND'); + }); + + test('the probe returns the key columns, so a violation names its row', () => { + expect(probeOf(lower('Order.total >= 0')).columns).toEqual(['OrderId']); + }); + + test('an unknown answer counts as a violation', () => { + // SQL three-valued logic: `NULL >= 0` is unknown, and a plain NOT would + // leave that row out of the violating set, passing a rule nothing verified. + expect(probeOf(lower('Order.total >= 0')).sql).toContain('NOT COALESCE('); + }); +}); + + +describe('the expressions the grammar accepts', () => { + test('comparisons joined by AND keep their own parentheses', () => { + expect(probeOf(lower('Order.total >= 0 AND Order.total <= 100000')).sql) + .toContain('NOT COALESCE((Total >= 0) AND (Total <= 100000), FALSE)'); + }); + + test('OR is carried through as written', () => { + expect(probeOf(lower("Order.status = 'open' OR Order.total >= 0")).sql) + .toContain("(Status = 'open') OR (Total >= 0)"); + }); + + test('an equality against NULL becomes a null test', () => { + // GoogleSQL refuses `col = NULL` outright, so lowering it verbatim would + // emit a probe that cannot run. + expect(probeOf(lower('Order.closedOn = NULL')).sql) + .toContain('(ClosedOn IS NULL)'); + expect(probeOf(lower('Order.closedOn != NULL')).sql) + .toContain('(ClosedOn IS NOT NULL)'); + }); + + test('<> reads as !=', () => { + expect(probeOf(lower("Order.status <> 'void'")).sql) + .toContain("(Status != 'void')"); + }); + + test('>= is not read as > with a stray = after it', () => { + expect(probeOf(lower('Order.total >= 0')).sql).toContain('(Total >= 0)'); + }); +}); + + +describe('the expressions it refuses, and what it says about them', () => { + test('a rule settled by judgment', () => { + // Nothing here calls a judge, and running the action anyway would run + // unjudged an action whose model says it is judged. + const reason = reasonOf(lowerRule({ + name: 'LargeCreditIsJustified', + judgment: 'The request must name a specific service failure.', + onViolation: 'escalate', + })); + expect(reason).toContain("constraint 'LargeCreditIsJustified' cannot be checked"); + expect(reason).toContain('runs no judge'); + }); + + test('an expression that is not there', () => { + expect(reasonOf(lower(''))).toContain('declares no expression'); + }); + + test("'==' for equality", () => { + expect(reasonOf(lower('Order.total == 0'))).toContain("a single '='"); + }); + + test('parentheses and function calls', () => { + expect(reasonOf(lower('Order.total >= ABS(0)'))) + .toContain('parentheses or a function call'); + }); + + test('an aggregate, which is a function call and needs a decision beyond it', + () => { + // How an aggregate is evaluated inside a row-level probe is a real + // question with more than one answer, so it is named rather than + // guessed at. + expect(reasonOf(lower('Order.total = SUM(Entry.amount)'))) + .toContain('parentheses or a function call'); + }); + + test('an ordering comparison against NULL', () => { + expect(reasonOf(lower('Order.closedOn > NULL'))).toContain('no meaning'); + }); + + test('a segment that is not a comparison at all', () => { + expect(reasonOf(lower('Order.total'))).toContain('is not a comparison'); + }); + + test('a bare name that is not a parameter of this action', () => { + // The likely mistake is a field written without its entity, so the message + // says how a field is written. + const reason = reasonOf(lower('total >= 0')); + expect(reason).toContain("'total' is not a parameter of this action"); + expect(reason).toContain('.'); + }); + + test('a rule spanning two entities, with both named', () => { + const reason = reasonOf(lower('Order.total >= Entry.amount')); + expect(reason).toContain('it spans Entry and Order'); + expect(reason).toContain("one constraint per entity"); + }); + + test('a field the entity does not declare', () => { + expect(reasonOf(lower('Order.shipped >= 0'))) + .toContain("Order declares no field 'shipped'"); + }); + + test('a field the profile in force bound to nothing', () => { + // Unbound is structurally absent, not null: there is no column to read the + // rule against, so the rule cannot be checked here. + expect(reasonOf(lower("Order.memo != ''"))) + .toContain('Order.memo is unbound under this profile'); + }); + + test('a field bound to an expression rather than to a column', () => { + expect(reasonOf(lower('Order.margin >= 0'))) + .toContain('bound to an expression (Price * Quantity)'); + }); + + test('an entity with no table to read', () => { + expect(reasonOf(lower("Party.name != ''"))) + .toContain("'Party' is abstract"); + }); + + test('an entity of no model', () => { + expect(reasonOf(lower('Customer.tier >= 0'))) + .toContain("'Customer' is not an entity of this model"); + }); + + test('an entity the action takes no reference to', () => { + // Widening the probe to every row is the alternative, and a gate whose + // cost grows with the table is one that gets switched off. + const reason = reasonOf(lower('Entry.amount > 0')); + expect(reason).toContain("action 'IssueCredit' takes no Entry parameter"); + expect(reason).toContain('the rows this call touches'); + }); + + test('an entity whose key has more than one part', () => { + const adjust: Action = { + ...ISSUE_CREDIT, + name: 'AdjustLine', + parameters: [{name: 'line', type: 'Line', isEntityRef: true}], + }; + expect(reasonOf(lower('Line.qty > 0', adjust))) + .toContain('Line has a 2-part key'); + }); + + test('a string literal containing AND, rather than mis-splitting it', () => { + // The split runs before the literals are read, so this expression comes + // apart in the wrong place. What matters is that the pieces then fail to + // parse: it is refused, never lowered to something that is not the rule. + expect(reasonOf(lower("Order.status != 'held AND pending'"))) + .toContain('is not a field, a parameter or a literal'); + }); +}); + + +describe('lowering the guards of one action', () => { + const positive: Constraint = {name: 'Positive', expression: 'amount > 0'}; + const judged: Constraint = { + name: 'Justified', + judgment: 'The request must name a specific service failure.', + onViolation: 'reject', + }; + + const guardedBy = (constraints: Constraint[], guards: string[]) => + lowerGuards( + modelWith({constraints}), {...ISSUE_CREDIT, guards}); + + test('a guard naming a constraint the model does not declare is an error', + () => { + // Not something to guess about, and not classifiable as advisory: + // there is no declaration to read an `on_violation` from. + const {errors} = guardedBy([positive], ['NoSuchRule']); + expect(errors.join(' ')).toContain("guarded by 'NoSuchRule'"); + }); + + test('a guard it cannot check stops the action', () => { + const {probes, errors} = guardedBy([judged], ['Justified']); + expect(probes).toEqual([]); + expect(errors).toHaveLength(1); + }); + + test('an advisory rule it cannot check is reported, not turned into a stop', + () => { + // `warn` reports and lets the write through, so being unable to + // evaluate it costs a report and stops nothing. Refusing over it + // would make the advice the one thing that blocks the action. + const advisory: Constraint = {...judged, onViolation: 'warn'}; + const {errors, unchecked} = guardedBy([advisory], ['Justified']); + expect(errors).toEqual([]); + expect(unchecked.map(u => u.constraint)).toEqual(['Justified']); + expect(unchecked[0].reason).toContain('runs no judge'); + }); + + test('the checkable guards are still lowered alongside the rest', () => { + const {probes, errors} = + guardedBy([positive, judged], ['Positive', 'Justified']); + expect(probes.map(p => p.constraint.name)).toEqual(['Positive']); + expect(errors).toHaveLength(1); + }); + + test('an action naming no guard produces nothing to run', () => { + const {probes, errors, unchecked} = guardedBy([positive], []); + expect(probes).toEqual([]); + expect(errors).toEqual([]); + expect(unchecked).toEqual([]); + }); +}); + + +describe('binding a probe to the call', () => { + const params = {order: '12345', amount: 30, other: 'x'}; + const types = { + order: {code: 'STRING'}, + amount: {code: 'NUMERIC'}, + other: {code: 'STRING'}, + }; + + test('carries only the parameters its SQL names', () => { + // An action's parameter list is wider than any one rule, and a statement + // carrying a parameter it never reads is one the store may refuse. + const statement = + probeStatement(probeOf(lower('amount <= 25')), params, types); + expect(statement.params).toEqual({amount: 30}); + expect(statement.paramTypes).toEqual({amount: {code: 'NUMERIC'}}); + }); + + test('carries the scope parameter too, when the probe reads a table', () => { + const statement = + probeStatement(probeOf(lower('Order.total >= 0')), params, types); + expect(statement.params).toEqual({order: '12345'}); + }); + + test('a probe naming no parameter carries none at all', () => { + const statement = probeStatement( + { + constraint: {name: 'Rule', expression: 'TRUE = TRUE'}, + timing: 'before', + sql: 'SELECT 1 AS violated FROM UNNEST([1]) WHERE FALSE', + columns: ['violated'], + }, + params, types); + expect(statement.params).toBeUndefined(); + }); +}); + + +describe('reporting a violation', () => { + test("the author's own words lead, and the citation follows", () => { + // The description is written as the instruction to the caller who was + // refused; the name and expression are what lets someone look the rule up. + const probe = probeOf(lowerRule({ + name: 'NonNegativeTotal', + expression: 'Order.total >= 0', + description: 'An order total never goes negative.', + })); + const violation = violationFrom(probe, [['12345']]); + expect(violation.message) + .toBe( + 'An order total never goes negative. ' + + "Stopped by 'NonNegativeTotal' (Order.total >= 0). " + + 'Violating Order: 12345.'); + expect(violation.constraint).toBe('NonNegativeTotal'); + expect(violation.instances).toEqual(['12345']); + }); + + test('a rule with no description still says which rule it was', () => { + const violation = violationFrom(probeOf(lower('Order.total >= 0')), [['7']]); + expect(violation.message).toContain("Constraint 'Rule' does not hold."); + }); + + test('a rule over the arguments alone names no row', () => { + // Its probe returns one row that identifies nothing, and "Violating: 1" + // would read as a row key. + const violation = violationFrom(probeOf(lower('amount <= 25')), [['1']]); + expect(violation.instances).toEqual([]); + expect(violation.message).not.toContain('Violating'); + }); + + test('several violating rows are all named', () => { + const violation = + violationFrom(probeOf(lower('Order.total >= 0')), [['1'], ['2']]); + expect(violation.message).toContain('Violating Order: 1, 2.'); + }); +}); + + +describe('what a violation does to the write', () => { + test('an expression that does not say rejects', () => { + // The safe reading of an author who did not say. + expect(effectOf({name: 'Rule', expression: 'amount > 0'})).toBe('reject'); + }); + + test('an effect the author did state is used as written', () => { + expect(effectOf({ + name: 'Rule', + expression: 'amount > 0', + onViolation: 'escalate', + })).toBe('escalate'); + }); + + test('the strictest effect among several is the answer', () => { + // Being told a supervisor could approve a write another rule forbids + // outright sends the caller to ask for something nobody can give. + const violation = (effect: 'reject'|'escalate'|'warn') => + ({constraint: effect, effect, message: '', instances: []}); + expect(strictestEffect([violation('warn'), violation('escalate')])) + .toBe('escalate'); + expect(strictestEffect([violation('escalate'), violation('reject')])) + .toBe('reject'); + expect(strictestEffect([violation('warn')])).toBe('warn'); + }); + + test('nothing violated is no answer at all', () => { + expect(strictestEffect([])).toBeNull(); + }); +}); diff --git a/toolbox/mdcode/tests/libts/semantic/runtime/run_action.test.ts b/toolbox/mdcode/tests/libts/semantic/runtime/run_action.test.ts index 997b6143..bd2558e4 100644 --- a/toolbox/mdcode/tests/libts/semantic/runtime/run_action.test.ts +++ b/toolbox/mdcode/tests/libts/semantic/runtime/run_action.test.ts @@ -641,15 +641,39 @@ describe('an action whose write is declared in the model', () => { }); -// Nothing evaluates a constraint yet, so an action that says it is checked -// before it runs must not run. `guards` is what says that, and it is the only -// thing that does: a constraint takes effect where something references it. -describe('a guarded action is refused, not run unchecked', () => { +// A constraint takes effect over a call because the action names it in +// `guards`, and these tests are about what happens then: the rule is lowered to +// a probe, the probe runs inside the action's own transaction, and what it +// finds decides whether the write stands. +describe('a guard the runtime checks', () => { + // Reads stored state alone, so it is asked after the write. const balance: Constraint = { name: 'NonNegativeBalance', expression: 'Account.balance >= 0', description: 'An account cannot go negative.', }; + // Reads one of the call's arguments, so it is asked before the write. + const ceiling: Constraint = { + name: 'UnderCeiling', + expression: 'amount <= 50', + description: 'A credit over 50 is above the self-service ceiling.', + onViolation: 'escalate', + }; + const positive: Constraint = { + name: 'PositiveAmount', + expression: 'amount > 0', + description: 'A credit must be for a positive amount.', + }; + // Fragments identifying each probe in what the fake was asked. The balance + // probe reads a table; the argument-only ones have no table to read, so + // GoogleSQL gives them a one-row source. + const BALANCE_PROBE = 'COALESCE((balance >= 0)'; + const ARGUMENT_PROBE = 'UNNEST([1])'; + + const guardedBy = (constraints: Constraint[]) => ({ + actions: [{...credit, guards: constraints.map(c => c.name)}], + constraints, + }); const runWith = (over: Partial, fake = resolvingFake()) => act({ @@ -659,31 +683,144 @@ describe('a guarded action is refused, not run unchecked', () => { client: fake.client, }); - test('an action that names a guard is refused', async () => { - const outcome = await runWith({ - actions: [{...credit, guards: ['NonNegativeBalance']}], - constraints: [balance], - }); - if (outcome.status !== 'error') throw new Error('expected an error'); - expect(outcome.message).toContain("guarded by 'NonNegativeBalance'"); - expect(outcome.message).toContain('does not evaluate constraints yet'); + test('a rule that holds lets the write through', async () => { + const fake = resolvingFake(); + const outcome = await runWith(guardedBy([balance]), fake); + if (outcome.status !== 'committed') throw new Error(outcome.message); + // The probe really ran. A guard that passes and a guard that was skipped + // produce the same outcome, and only one of them is the runtime working. + expect(fake.sql.filter(s => s.includes(BALANCE_PROBE))).toHaveLength(1); }); - test('a refused action never opens a transaction', async () => { - // The point of deciding before the store is touched: there is nothing to - // roll back, and no session to leak. + test('a rule over stored state is asked after the write', async () => { + // Inside the same transaction, which is the one moment the post-state both + // exists and can still be undone. const fake = resolvingFake(); - await runWith( - { - actions: [{...credit, guards: ['NonNegativeBalance']}], - constraints: [balance], - }, - fake); - expect(fake.statements).toHaveLength(0); - expect(fake.sessionsOpened).toBe(0); - expect(fake.rolledBack).toBe(false); + await runWith(guardedBy([balance]), fake); + expect(fake.sql.findIndex(s => s.includes(BALANCE_PROBE))) + .toBeGreaterThan(fake.sql.findIndex(s => s.startsWith('UPDATE'))); + }); + + test('a violated rule rolls the write back and reports the author words', + async () => { + const fake = resolvingFake([{match: BALANCE_PROBE, rows: [['1']]}]); + const outcome = await runWith(guardedBy([balance]), fake); + if (outcome.status !== 'refused') throw new Error('expected a refusal'); + expect(outcome.effect).toBe('reject'); + expect(outcome.message).toContain('An account cannot go negative.'); + expect(outcome.message).toContain("'NonNegativeBalance'"); + // Which row broke it, so the caller can say something specific. + expect(outcome.message).toContain('Violating Account: 1'); + expect(fake.rolledBack).toBe(true); + expect(fake.committed).toBe(false); + }); + + test('a rule over the arguments alone is asked before anything is written', + async () => { + // A rule reading an argument is asking whether this call may proceed + // at all, and a call that may not should cost the store no writes. + const fake = resolvingFake([{match: ARGUMENT_PROBE, rows: [['1']]}]); + const outcome = await runWith(guardedBy([ceiling]), fake); + if (outcome.status !== 'refused') throw new Error('expected a refusal'); + expect(fake.sql.some(s => s.startsWith('INSERT'))).toBe(false); + expect(fake.sql.some(s => s.startsWith('UPDATE'))).toBe(false); + }); + + test('an argument reaches the probe as a parameter, interpolating nothing', + async () => { + const fake = resolvingFake(); + await runWith(guardedBy([ceiling]), fake); + const probe = fake.statements.find(s => s.sql.includes(ARGUMENT_PROBE)); + expect(probe!.sql).toContain('@amount'); + expect(probe!.sql).not.toContain('100'); + expect(probe!.params).toEqual({amount: 100}); + }); + + test('a probe carries only the parameters it names', async () => { + // The action takes an account too. A statement carrying a parameter it + // never reads is one the store may refuse. + const fake = resolvingFake(); + await runWith(guardedBy([ceiling]), fake); + const probe = fake.statements.find(s => s.sql.includes(ARGUMENT_PROBE)); + expect(Object.keys(probe!.params ?? {})).toEqual(['amount']); }); + test('a rule needing an approval says nobody is holding the write', + async () => { + // There is no approval queue here. Saying the write is "held for + // review" would leave the caller waiting for something that is not + // coming. + const fake = resolvingFake([{match: ARGUMENT_PROBE, rows: [['1']]}]); + const outcome = await runWith(guardedBy([ceiling]), fake); + if (outcome.status !== 'refused') throw new Error('expected a refusal'); + expect(outcome.effect).toBe('escalate'); + expect(outcome.message) + .toContain('run the action again once it is approved'); + expect(fake.rolledBack).toBe(true); + }); + + test('the strictest effect among the broken rules is what happens', + async () => { + // Sending the caller to ask for an approval that cannot authorize the + // other rule spends somebody's time on a write that was never going + // to land. + const fake = resolvingFake([{match: ARGUMENT_PROBE, rows: [['1']]}]); + const outcome = await runWith(guardedBy([ceiling, positive]), fake); + if (outcome.status !== 'refused') throw new Error('expected a refusal'); + expect(outcome.effect).toBe('reject'); + expect(outcome.violations.map(v => v.constraint)) + .toEqual(['UnderCeiling', 'PositiveAmount']); + }); + + test('a rule that only reports lets the write through and is carried out', + async () => { + const advisory: Constraint = { + name: 'RoundAmount', + expression: 'amount <= 10', + description: 'Credits over 10 are usually reviewed.', + onViolation: 'warn', + }; + const fake = resolvingFake([{match: ARGUMENT_PROBE, rows: [['1']]}]); + const outcome = await runWith(guardedBy([advisory]), fake); + if (outcome.status !== 'committed') throw new Error(outcome.message); + expect(fake.committed).toBe(true); + // A caller that never sees it has been told the write was clean when + // it was not. + expect(outcome.warnings?.map(v => v.constraint)).toEqual( + ['RoundAmount']); + }); + + test('a rule this runtime cannot check refuses before the store is touched', + async () => { + // Nothing to roll back, and no session to leak. Running the action + // would apply a write the model says is checked first. + const fake = resolvingFake(); + const outcome = await runWith( + guardedBy([{ + name: 'CreditIsJustified', + judgment: 'The memo must name a specific service failure.', + onViolation: 'reject', + }]), + fake); + if (outcome.status !== 'error') throw new Error('expected an error'); + expect(outcome.message).toContain('runs no judge'); + expect(outcome.message).toContain('refused rather than run unchecked'); + expect(fake.statements).toHaveLength(0); + expect(fake.sessionsOpened).toBe(0); + expect(fake.rolledBack).toBe(false); + }); + + test('a rule over an entity the call does not name is refused, not widened', + async () => { + // Dropping the scope would turn the probe into a table scan holding + // read locks for the length of the write. + const outcome = await runWith(guardedBy( + [{name: 'EntriesArePositive', expression: 'Entry.amount > 0'}])); + if (outcome.status !== 'error') throw new Error('expected an error'); + expect(outcome.message) + .toContain("takes no Entry parameter"); + }); + test('an action writing data a constraint reads runs, if it names no guard', async () => { // Credit affects Account and NonNegativeBalance reads Account.balance. @@ -734,16 +871,17 @@ describe('a guarded action is refused, not run unchecked', () => { expect(outcome.message).toContain("guarded by 'NoSuchRule'"); }); - test('several guards are all named', async () => { + test('every rule it cannot check is named, not just the first', async () => { const outcome = await runWith({ - actions: [{...credit, guards: ['ZBalance', 'AEntry']}], + actions: [{...credit, guards: ['ZSpansTwo', 'ACallsAFunction']}], constraints: [ - {name: 'ZBalance', expression: 'Account.balance >= 0'}, - {name: 'AEntry', expression: 'Entry.amount > 0'}, + {name: 'ZSpansTwo', expression: 'Account.balance >= Entry.amount'}, + {name: 'ACallsAFunction', expression: 'Account.balance >= ABS(amount)'}, ], }); if (outcome.status !== 'error') throw new Error('expected an error'); - expect(outcome.message).toContain("'ZBalance' and 'AEntry'"); + expect(outcome.message).toContain("'ZSpansTwo'"); + expect(outcome.message).toContain("'ACallsAFunction'"); }); }); diff --git a/toolbox/mdcode/tests/tool/action.test.ts b/toolbox/mdcode/tests/tool/action.test.ts index f1189657..bd162920 100644 --- a/toolbox/mdcode/tests/tool/action.test.ts +++ b/toolbox/mdcode/tests/tool/action.test.ts @@ -3,7 +3,9 @@ // // Almost nothing here reaches a store, and that is not a compromise: `list` // never opens one, and every `run` covered but the last fails before the first -// request. The exception fakes the Spanner client's own surface, because what +// request. `IssueCredit` names a guard settled by judgment, which this runtime +// cannot check and therefore refuses, so a `run` of it stops at the runtime +// without a session being opened. The exception fakes the Spanner client's own surface, because what // it checks is the QUESTION the runtime asks the store. The // argument parse, the choice of database, and the runtime's own refusal to run // an action a constraint is supposed to decide all happen before a session @@ -27,8 +29,8 @@ const CTX = new ApiContext('test-project', 'us', 'test-token'); const SPANNER = '//spanner.googleapis.com/projects/acme-ops/instances/prod'; // The model as authored: bound to Spanner, with one action the runtime could -// run and one it could not, plus the two constraint shapes -- an invariant over -// stored data and a guard over an argument. +// run and one it could not, plus three constraint shapes -- an invariant over +// stored data, a guard over an argument, and a rule settled by judgment. const MODEL = `version: "0.2.0.dev0/google" semantic_model: - name: commerce @@ -58,7 +60,7 @@ semantic_model: parameters: - {name: order, type: Order} - {name: amount, type: Decimal} - guards: [CreditIsPositive] + guards: [CreditIsPositive, CreditIsJustified] affects: - {concept: Entry, operation: create} - name: NotifyCustomer @@ -75,6 +77,10 @@ semantic_model: - name: CreditIsPositive expression: amount > 0 description: A credit must be for a positive amount. + - name: CreditIsJustified + judgment: The request must name a specific service failure. + on_violation: escalate + description: A credit needs a stated reason. `; // The same model with nothing to run. @@ -395,7 +401,7 @@ describe('kcmd action run: what it will not send to a store', () => { // below is about the guard, which is proof the parse succeeded. const code = await action('run', 'IssueCredit', {arg: 'amount=30'}); expect(code).toBe(1); - expect(logs.join('\n')).toContain('does not evaluate constraints yet'); + expect(logs.join('\n')).toContain('runs no judge'); }); test('refuses an action whose executor runs outside the transaction', @@ -406,19 +412,18 @@ describe('kcmd action run: what it will not send to a store', () => { expect(logs.join('\n')).toContain('which runs outside this transaction'); }); - test('refuses a guarded action while nothing evaluates the guard', - async () => { - writeWorkspace(); - const code = await action( - 'run', 'IssueCredit', {arg: ['order=12345', 'amount=30']}); - expect(code).toBe(1); - const out = logs.join('\n'); - expect(out).toContain("is guarded by 'CreditIsPositive'"); - // It got as far as choosing a database, so the refusal is the - // runtime's and not a wiring failure earlier on. - expect(out).toContain( - "Running 'IssueCredit' on projects/acme-ops/instances/prod/databases/commerce"); - }); + test('refuses an action whose guard it cannot check', async () => { + writeWorkspace(); + const code = await action( + 'run', 'IssueCredit', {arg: ['order=12345', 'amount=30']}); + expect(code).toBe(1); + const out = logs.join('\n'); + expect(out).toContain("constraint 'CreditIsJustified' cannot be checked"); + // It got as far as choosing a database, so the refusal is the + // runtime's and not a wiring failure earlier on. + expect(out).toContain( + "Running 'IssueCredit' on projects/acme-ops/instances/prod/databases/commerce"); + }); }); @@ -506,14 +511,14 @@ describe('kcmd action run: which model has to be valid', () => { withWarehouse(); const code = await action( 'run', 'IssueCredit', {arg: ['order=12345', 'amount=30']}); - // Still refused -- IssueCredit is guarded and nothing evaluates a - // guard yet -- but refused on its OWN terms. + // Still refused -- IssueCredit names a guard this runtime cannot + // check -- but refused on its OWN terms. expect(code).toBe(1); // The broken document is still WARNED about -- it is a real problem, // reported where it is. What must not happen is it becoming the // reason this call failed. const errors = logs.filter(l => l.startsWith('Error:')).join('\n'); - expect(errors).toContain("is guarded by 'CreditIsPositive'"); + expect(errors).toContain("'CreditIsJustified' cannot be checked"); expect(errors).not.toContain('Pallet'); expect(errors).not.toContain('warehouse'); }); @@ -569,7 +574,7 @@ describe('kcmd action: what the command line can actually contain', () => { const out = logs.join('\n'); expect(out).not.toContain('given twice'); // It gets as far as the refusal, which is where this model stops. - expect(out).toContain('CreditIsPositive'); + expect(out).toContain('CreditIsJustified'); }); test('a genuinely repeated argument is still reported', async () => { From 732d6ecd4cdff2386c96fa0d7801f18171f52bef Mon Sep 17 00:00:00 2001 From: Bei Li Date: Sun, 13 Sep 2026 21:14:25 +0000 Subject: [PATCH 2/5] fix(mdcode): scope a guard to every row it names, and read literals whole Review of the constraint evaluator turned up three defects and three claims the code does not support. A rule over stored data was probed against the first parameter of that entity and no other, so `TransferFunds(source: Account, target: Account)` guarded by a rule over `Account` asked about the source alone and reported the rule as checked. The probe now scopes to every reference the action names. The scan for AND/OR and the scan for a comparison operator both ran over the raw expression, so `Account.status = 'ON HOLD OR CLOSED'` came apart inside the quotes and, because an unlowerable rule that rejects stops the call, was refused for a fault it does not have. Both scans now read a mask that blanks quoted spans and preserves offsets. A `warn` probe the store refused escaped as a StoreError and rolled the write back, undoing one layer down the carve-off `lowerGuards` makes for an advisory rule. Such a failure now joins the rules that could not be lowered. The documentation said a guard over stored data stops a call on data that is already broken, which the derived timing makes false: a rule reading no parameter is asked of the post-state. That claim is corrected, and the limitation it hid -- a precondition on the state before the write cannot be expressed -- is stated. The demo README described `CreditWithinOrderTotal` as running after the statements when it reads a parameter and runs before them. The `Constraint` doc comment still said no component evaluates a constraint. The captured runs under "What a violated guard does" were re-run. The rejecting example named an action whose table does not exist in the demo store, so it could not have been captured as shown; it is replaced with a rejecting run of the credit policy, and the committed run carries its real timestamp. --- .../demo/semantic-model/agent/README.md | 11 +-- toolbox/mdcode/docs/semantic-model/actions.md | 51 ++++++++---- toolbox/mdcode/src/libts/semantic/ir.ts | 13 ++-- .../libts/semantic/runtime/constraint_eval.ts | 45 +++++++++-- .../src/libts/semantic/runtime/run_action.ts | 29 ++++++- .../semantic/runtime/constraint_eval.test.ts | 77 +++++++++++++++++-- .../libts/semantic/runtime/run_action.test.ts | 38 +++++++++ 7 files changed, 219 insertions(+), 45 deletions(-) diff --git a/toolbox/mdcode/demo/semantic-model/agent/README.md b/toolbox/mdcode/demo/semantic-model/agent/README.md index be9ff289..0958a817 100644 --- a/toolbox/mdcode/demo/semantic-model/agent/README.md +++ b/toolbox/mdcode/demo/semantic-model/agent/README.md @@ -486,11 +486,12 @@ takes: the $30 credit is refused with `escalate`, the transaction rolls back, and the agent is told a supervisor decides it. See [What a violated guard does](../../../docs/semantic-model/actions.md#what-a-violated-guard-does). -Two of the three rules are less straightforward. `CreditWithinOrderTotal` reads -`Order.total`, so it is a rule over stored data and runs after the statements -inside the same transaction. `OrderTotalMatchesLineItems` aggregates over a -child table, which the expression grammar does not parse, so naming it stops the -action rather than checking it. +Two of the three rules are less straightforward. `CreditWithinOrderTotal` is +`amount <= Order.total`, which reads the orders table and the `amount` parameter +both, so its probe queries the order the call names and still runs before any +statement. `OrderTotalMatchesLineItems` aggregates over a child table, which the +expression grammar does not parse, so naming it stops the action rather than +checking it. Wiring the guards and re-running the request live is the next step for this demo, and the transcript above is what it looks like before that happens. diff --git a/toolbox/mdcode/docs/semantic-model/actions.md b/toolbox/mdcode/docs/semantic-model/actions.md index f299474f..76b1edd8 100644 --- a/toolbox/mdcode/docs/semantic-model/actions.md +++ b/toolbox/mdcode/docs/semantic-model/actions.md @@ -324,8 +324,7 @@ runs: `guards` holds the names of constraints the same model declares, and it is how a constraint acquires effect over an action. Both kinds of rule belong there. One that reads the action's parameters has no other moment to run. One over stored -data, named as a guard, states that the call must not proceed on data that is -already broken. +data, named as a guard, states that the call must leave the data sound. When a guard runs follows from what its expression reads, and the model never states it. A rule over the action's parameters is settled before any statement @@ -335,6 +334,12 @@ statements and inside the same transaction, and a breach rolls that write back. The difference is derived from the expression rather than authored, so a rule stays one sentence whether it decides the call or its result. +A rule over stored data is asked only of the rows the call names, and of every +one of them. `TransferFunds` takes two `Account` parameters and writes both, so +a rule over `Account` is asked about the source and the target together. Scoping +it to the first of the two would report the rule as checked while letting +through the write that breaks the second. + Whatever dispatches the call is what checks its guards. Handing a rule to the store instead works only for some rules. A condition on a single row lowers to a store-level `CHECK`. One that aggregates across a child table, such as an order @@ -928,21 +933,25 @@ that performs the write as DML, or declare the action with a 'sql' executor. ### What a violated guard does A violated guard stops the write and answers in the words the constraint's -`description` gives, so the caller reads the policy rather than a predicate. A -rule that declares `reject` ends the call: +`description` gives, so the caller reads the policy rather than a predicate. +Both runs below take the credit policy from +[section 2](#a-policy-whose-rules-end-differently) with the two guards this +runtime cannot check left off the action. + +A rule that declares `reject` ends the call. The policy as written has no +rejecting rule a query settles, so this run adds one — `CreditAmountIsPositive`, +`amount > 0` — and asks for a credit of nothing: ``` -$ kcmd action run TransferFunds --arg source="Alice Checking" --arg target=ACC-2 --arg amount=0 -Running 'TransferFunds' on projects/my-project/instances/my-instance/databases/semantic_agent_demo... -Refused (reject): Action 'TransferFunds' was refused and nothing was written. A -transfer must move at least one unit. Ask the caller for the amount again before -retrying. Stopped by 'AmountIsPositive' (amount > 0). +$ kcmd action run IssueCredit --arg order=12345 --arg amount=0 --arg memo="shipping charge applied in error" +Running 'IssueCredit' on projects/my-project/instances/my-instance/databases/semantic_agent_demo... +Refused (reject): Action 'IssueCredit' was refused and nothing was written. A +credit must return at least one cent. Ask the caller for the amount again before +retrying. Stopped by 'CreditAmountIsPositive' (amount > 0). ``` A rule that declares `escalate` ends it the same way and adds what would change -the answer. Here is the credit policy from -[section 2](#a-policy-whose-rules-end-differently), with the two guards this -runtime cannot check left off the action, taking the 30-dollar credit: +the answer. Taking the 30-dollar credit: ``` $ kcmd action run IssueCredit --arg order=12345 --arg amount=30 --arg memo="shipping charge applied in error" @@ -960,8 +969,10 @@ the runtime working. A rule that declares `warn` commits and reports the violation alongside the commit. When one call violates several guards the strictest outcome applies, by the -rule [section 2](#two-calls-through-that-policy) states, and every violated -rule is named in the message rather than only the one that decided it. +rule [section 2](#two-calls-through-that-policy) states, and every rule that +stopped the call is named in the message rather than only the strictest. A rule +that declares `warn` is not among them: it asks for the write to proceed and be +reported, and a refused call wrote nothing for it to report on. What makes a rule a guard of this call is `guards`, and only `guards`. A constraint the action does not name is a rule this call does not consult, and @@ -1010,7 +1021,7 @@ Running 'IssueCredit' on projects/my-project/instances/my-instance/databases/sem not checked: advisory rule 'CreditMemoNamesAServiceFailure' (constraint 'CreditMemoNamesAServiceFailure' cannot be checked: it is settled by judgment rather than by an expression, and this runtime runs no judge) -Committed at 2026-09-13T18:00:00Z. +Committed at 2026-09-13T21:09:10.688549Z. ``` A rule that cannot be checked is found while the guards are lowered, before any @@ -1282,7 +1293,7 @@ states what the run cannot yet do. ## What is not modeled yet -This is a prototype. Four things a reader reasonably expects are absent. +This is a prototype. Five things a reader reasonably expects are absent. - **No judge settles a judgment.** An expression guard is turned into a query and checked; a judgment is published and nothing reads it. An action guarded @@ -1293,6 +1304,14 @@ This is a prototype. Four things a reader reasonably expects are absent. function call, a subquery or a rule spanning two entities does not lower, and the action is stopped rather than run past it. Beyond the guards, the correctness of what a statement does belongs to whoever wrote it. +- **A guard cannot ask about the state before the write.** Whether a rule runs + before the statements or after them is derived from whether it reads a + parameter, so a condition over stored data alone is always asked of the + post-state. `Order.status = 'OPEN'` on an action that closes the order is a + precondition no model can currently express: written as a guard it is checked + after the close and fails every call. Say it with a parameter the rule can + read, or leave it to the statement's own `WHERE` clause, until the model has a + way to name the moment. - **`kcmd` calls no executor but its own.** A `sql` action runs; an `mcp`, `rest` or `grpc` one is published for whoever dispatches it, which is why those three name coordinates rather than a statement. diff --git a/toolbox/mdcode/src/libts/semantic/ir.ts b/toolbox/mdcode/src/libts/semantic/ir.ts index a542ddea..e4f22fa7 100644 --- a/toolbox/mdcode/src/libts/semantic/ir.ts +++ b/toolbox/mdcode/src/libts/semantic/ir.ts @@ -616,12 +616,13 @@ export function constraintEvaluation(c: Constraint): ConstraintEvaluation { * schema changes that. `onViolation` is required on a judgment so that the * consequence of that non-determinism is always stated rather than inherited. * - * STATUS: authored, validated and published; not yet enforced. kcmd carries a - * constraint to Knowledge Catalog, where an agent can read the rules a model - * requires. No component evaluates one, so nothing today rejects a write that - * would break it. Enforcement is the point of declaring them: an operational - * agent running an action writes to a live store, and a bad write corrupts - * data. The rule has to be stated and governed before it can be checked. + * STATUS: an expression is enforced where an action names the constraint in + * `guards`; a judgment is not. kcmd lowers such an expression into one query + * against the store and runs it in the write's own transaction, so a rule that + * does not hold rolls the write back. An expression outside that grammar, and + * every judgment, is carried to Knowledge Catalog for a reader to enforce and + * stops the action here rather than being run past. A constraint no action + * names is inert either way, which is what makes publishing one safe. * * `description` is the error text a violation would surface, so write it to * steer an agent's next move -- "reduce the order quantity or choose another diff --git a/toolbox/mdcode/src/libts/semantic/runtime/constraint_eval.ts b/toolbox/mdcode/src/libts/semantic/runtime/constraint_eval.ts index 2661d60f..98c10ddf 100644 --- a/toolbox/mdcode/src/libts/semantic/runtime/constraint_eval.ts +++ b/toolbox/mdcode/src/libts/semantic/runtime/constraint_eval.ts @@ -378,11 +378,18 @@ function violating(predicate: string): string { // a table scan. Checking stored state at large is a different binding point -- // a conformance sweep over the data rather than a gate on one call -- and it // needs its own reference instead of this one silently standing in for it. +// +// EVERY parameter of that entity is in scope, not the first one found. +// `TransferFunds(source: Account, target: Account, amount)` writes both +// accounts, so a rule over `Account` that asked only about `source` would let +// the write that breaks `target` through while reporting the rule as checked. +// A gate that answers about some of the rows it was asked about is worse than +// one that refuses, because its answer is believed. function scopeToTouchedRows( action: Action, entity: Entity): {predicate: string}|{error: string} { - const param = - action.parameters.find(p => p.isEntityRef && p.type === entity.name); - if (!param) { + const params = + action.parameters.filter(p => p.isEntityRef && p.type === entity.name); + if (!params.length) { return { error: `it reads ${entity.name}, and action '${action.name}' takes no ` + `${entity.name} parameter, so the probe could not be limited to ` + @@ -397,7 +404,13 @@ function scopeToTouchedRows( `runtime binds an object reference as a single value`, }; } - return {predicate: `${keys.columns[0]} = @${param.name}`}; + const key = keys.columns[0]; + if (params.length === 1) { + return {predicate: `${key} = @${params[0].name}`}; + } + return { + predicate: `${key} IN (${params.map(p => `@${p.name}`).join(', ')})`, + }; } @@ -518,6 +531,20 @@ function parseExpression( } +// The expression with every single-quoted span blanked out, character for +// character, so a scan can find structure without seeing inside a literal. +// Offsets are preserved, which is the point: the caller matches against the +// mask and slices the original at the same index. +// +// Without it `Account.status = 'ON HOLD OR CLOSED'` splits on the OR inside +// the string and the rule is refused for a fault it does not have. `isLiteral` +// already bars an embedded quote or backslash, so a literal is exactly the +// text between one pair of quotes. +function maskLiterals(expression: string): string { + return expression.replace(/'[^']*'/g, m => `'${'.'.repeat(m.length - 2)}'`); +} + + // Splits on top-level AND/OR, matched as whole words so a field named `brand` // survives. There are no parentheses to nest -- parseExpression refuses them -- // so every operator found is top level. @@ -525,10 +552,11 @@ function splitOnLogicalOperators(expression: string): {parts: string[]; joiners: string[]} { const parts: string[] = []; const joiners: string[] = []; + const masked = maskLiterals(expression); const pattern = /\s+(AND|OR)\s+/gi; let last = 0; let match: RegExpExecArray|null; - while ((match = pattern.exec(expression)) !== null) { + while ((match = pattern.exec(masked)) !== null) { parts.push(expression.slice(last, match.index)); joiners.push(match[1].toUpperCase()); last = match.index + match[0].length; @@ -606,11 +634,14 @@ function parseOperand( // The first comparison operator in `text`, longest match first so `>=` is not -// read as `>` with a stray `=` after it. +// read as `>` with a stray `=` after it. Read against the masked text, so an +// operator character inside a string literal -- `'a>b' = Order.tag` -- is not +// mistaken for the comparison. function findOperator(text: string): {operator: string; index: number}|null { let best: {operator: string; index: number}|null = null; + const masked = maskLiterals(text); for (const operator of OPERATORS) { - const index = text.indexOf(operator); + const index = masked.indexOf(operator); if (index < 0) continue; if (!best || index < best.index || (index === best.index && operator.length > best.operator.length)) { diff --git a/toolbox/mdcode/src/libts/semantic/runtime/run_action.ts b/toolbox/mdcode/src/libts/semantic/runtime/run_action.ts index 0cc02a52..c52e47f8 100644 --- a/toolbox/mdcode/src/libts/semantic/runtime/run_action.ts +++ b/toolbox/mdcode/src/libts/semantic/runtime/run_action.ts @@ -57,6 +57,7 @@ import {quoteIfReserved, referencedParameters} from '../sql_identifiers'; import { ConstraintViolation, + effectOf, lowerGuards, UncheckedRule, probeStatement, @@ -184,6 +185,10 @@ export async function runAction(opts: RunActionOptions): // uncheckable. const lowered = lowerGuards(model, action); const probes = lowered.probes; + // Grows during the run: a probe the store refuses joins the rules that could + // not be lowered in the first place, since both leave a rule the model named + // unevaluated and both are worth reporting under the same heading. + const unchecked: UncheckedRule[] = [...lowered.unchecked]; // Also before touching the store, because there may be none to touch. const client = runtimeClient(opts.runtime); @@ -261,12 +266,30 @@ export async function runAction(opts: RunActionOptions): } probeValues = bound; } + // A probe that the store refuses is the same situation as a rule that + // could not be lowered, and it is answered the same way: an advisory + // rule is reported as unchecked and the write goes on, anything + // stricter stops the call. Letting a `warn` probe's StoreError escape + // would roll the transaction back over a rule whose whole contract is + // that it stops nothing -- the carve-off `lowerGuards` makes, undone + // one layer down. const check = async (timing: ProbeTiming) => { const violations: ConstraintViolation[] = []; for (const probe of probes) { if (probe.timing !== timing) continue; - const rows = await query(probeStatement( - probe, probeValues!.params, probeValues!.types)); + let rows; + try { + rows = await query(probeStatement( + probe, probeValues!.params, probeValues!.types)); + } catch (err) { + if (effectOf(probe.constraint) !== 'warn') throw err; + unchecked.push({ + constraint: probe.constraint.name, + reason: `its probe could not be run (${ + err instanceof Error ? err.message : String(err)})`, + }); + continue; + } if (rows.length) violations.push(violationFrom(probe, rows)); } return violations; @@ -366,7 +389,7 @@ export async function runAction(opts: RunActionOptions): commitTimestamp: committed.result?.commitTimestamp, refs, ...(warnings.length ? {warnings} : {}), - ...(lowered.unchecked.length ? {unchecked: lowered.unchecked} : {}), + ...(unchecked.length ? {unchecked} : {}), } as ActionOutcome; } catch (err) { try { diff --git a/toolbox/mdcode/tests/libts/semantic/runtime/constraint_eval.test.ts b/toolbox/mdcode/tests/libts/semantic/runtime/constraint_eval.test.ts index 5deee2cb..a79a9388 100644 --- a/toolbox/mdcode/tests/libts/semantic/runtime/constraint_eval.test.ts +++ b/toolbox/mdcode/tests/libts/semantic/runtime/constraint_eval.test.ts @@ -89,13 +89,30 @@ const ISSUE_CREDIT: Action = { ], }; +// Two references to the SAME entity, which is the shape a transfer takes and +// the shape that catches a probe scoped to only one of them. +const MOVE_CREDIT: Action = { + name: 'MoveCredit', + description: 'Move a credit from one order to another.', + executor: { + kind: 'sql', + sql: {statements: ['UPDATE Orders SET Total = Total WHERE OrderId = @from']}, + }, + parameters: [ + {name: 'from', type: 'Order', isEntityRef: true}, + {name: 'to', type: 'Order', isEntityRef: true}, + {name: 'amount', type: 'Decimal', isEntityRef: false}, + ], +}; + + function modelWith(over: Partial = {}): SemanticModel { return { name: 'commerce', entities: [ORDER, ENTRY, LINE, PARTY], relationships: [], metrics: [], - actions: [ISSUE_CREDIT], + actions: [ISSUE_CREDIT, MOVE_CREDIT], ...over, }; } @@ -122,6 +139,32 @@ function reasonOf(lowered: Lowering): string { describe('what the probe reads and when it runs', () => { + test('every parameter of the entity is in scope, not just the first', () => { + // The failure this guards against is silent: scoping to `from` alone + // would probe one of the two rows the action writes and report the rule + // as checked, which is the one outcome the lowering must never produce. + const probe = probeOf(lower('Order.total >= 0', MOVE_CREDIT)); + expect(probe.sql).toContain('WHERE OrderId IN (@from, @to)'); + expect(probe.sql).not.toContain('OrderId = @from'); + }); + + test('both references are bound to the probe', () => { + const params = {from: 1, to: 2, amount: 5}; + const types = { + from: {code: 'INT64'}, + to: {code: 'INT64'}, + amount: {code: 'NUMERIC'}, + }; + const statement = probeStatement( + probeOf(lower('Order.total >= 0', MOVE_CREDIT)), params, types); + expect(statement.params).toEqual({from: 1, to: 2}); + }); + + test('one reference still reads as a plain equality', () => { + expect(probeOf(lower('Order.total >= 0')).sql) + .toContain('WHERE OrderId = @order'); + }); + test('a rule over stored state alone reads the table, after the write', () => { const probe = probeOf(lower('Order.total >= 0')); @@ -178,6 +221,31 @@ describe('the expressions the grammar accepts', () => { .toContain('NOT COALESCE((Total >= 0) AND (Total <= 100000), FALSE)'); }); + test('a literal containing AND or OR is one operand, not a join', () => { + // The scan for AND/OR runs over the whole expression, so a rule whose + // string happens to spell one of them used to come apart in the middle + // of the quotes and be refused for a fault it does not have. + expect(probeOf(lower("Order.status != 'held AND pending'")).sql) + .toContain("NOT COALESCE((Status != 'held AND pending'), FALSE)"); + expect(probeOf(lower("Order.status != 'ON HOLD OR CLOSED'")).sql) + .toContain("NOT COALESCE((Status != 'ON HOLD OR CLOSED'), FALSE)"); + }); + + test('an operator inside a literal is not the comparison', () => { + expect(probeOf(lower("'a>b' != Order.status")).sql) + .toContain("NOT COALESCE(('a>b' != Status), FALSE)"); + }); + + test('a joined rule whose literal also spells a joiner', () => { + // Both scans have to agree about where the literal ends: the real AND + // joins the two comparisons, the one inside the quotes does not. + expect(probeOf(lower( + "Order.status != 'held AND pending' AND Order.total >= 0")) + .sql) + .toContain( + "(Status != 'held AND pending') AND (Total >= 0)"); + }); + test('OR is carried through as written', () => { expect(probeOf(lower("Order.status = 'open' OR Order.total >= 0")).sql) .toContain("(Status = 'open') OR (Total >= 0)"); @@ -305,13 +373,6 @@ describe('the expressions it refuses, and what it says about them', () => { .toContain('Line has a 2-part key'); }); - test('a string literal containing AND, rather than mis-splitting it', () => { - // The split runs before the literals are read, so this expression comes - // apart in the wrong place. What matters is that the pieces then fail to - // parse: it is refused, never lowered to something that is not the rule. - expect(reasonOf(lower("Order.status != 'held AND pending'"))) - .toContain('is not a field, a parameter or a literal'); - }); }); diff --git a/toolbox/mdcode/tests/libts/semantic/runtime/run_action.test.ts b/toolbox/mdcode/tests/libts/semantic/runtime/run_action.test.ts index bd2558e4..718c65ad 100644 --- a/toolbox/mdcode/tests/libts/semantic/runtime/run_action.test.ts +++ b/toolbox/mdcode/tests/libts/semantic/runtime/run_action.test.ts @@ -790,6 +790,44 @@ describe('a guard the runtime checks', () => { ['RoundAmount']); }); + test('an advisory rule whose probe the store refuses does not stop the write', + async () => { + // The carve-out `lowerGuards` makes for an advisory rule has to hold + // one layer down too. A `warn` probe that fails -- a column dropped + // under the model, a type the store will not compare -- must not roll + // back a write that the rule itself would have let through. + const advisory: Constraint = { + name: 'RoundAmount', + expression: 'amount <= 10', + description: 'Credits over 10 are usually reviewed.', + onViolation: 'warn', + }; + const fake = resolvingFake(); + fake.failOn = [ARGUMENT_PROBE]; + const outcome = await runWith(guardedBy([advisory]), fake); + if (outcome.status !== 'committed') throw new Error(outcome.message); + expect(fake.committed).toBe(true); + expect(fake.rolledBack).toBe(false); + // Reported rather than dropped: the model asked for a check it did + // not get, which is the part a caller can act on. + expect(outcome.unchecked?.map(u => u.constraint)).toEqual( + ['RoundAmount']); + expect(outcome.unchecked?.[0].reason) + .toContain('its probe could not be run'); + }); + + test('a stricter rule whose probe the store refuses stops the write', + async () => { + // The same failure under a rule that stops things is a store error, + // because nothing here knows whether the rule holds. + const fake = resolvingFake(); + fake.failOn = [ARGUMENT_PROBE]; + const outcome = await runWith(guardedBy([positive]), fake); + expect(outcome.status).toBe('error'); + expect(fake.committed).toBe(false); + expect(fake.rolledBack).toBe(true); + }); + test('a rule this runtime cannot check refuses before the store is touched', async () => { // Nothing to roll back, and no session to leak. Running the action From a1a2b70dd724089d3ed41d3bb7d7c85153d9405b Mon Sep 17 00:00:00 2001 From: Bei Li Date: Sun, 13 Sep 2026 22:41:40 +0000 Subject: [PATCH 3/5] refactor(mdcode): share the literal grammar with the rest of the layer The evaluator carried its own regex for a single-quoted span. sql_expr_utils already defines one, used by six other modules, and it also covers the double-quoted form and backslash escapes. The operator scan takes blankStringLiterals unchanged. The AND/OR split cannot: blanking to spaces merges a literal into the whitespace on either side of it, so the pattern matches across where the literal used to be. It reuses the shared literal grammar with a non-space filler instead. --- .../libts/semantic/runtime/constraint_eval.ts | 33 ++++++++----------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/toolbox/mdcode/src/libts/semantic/runtime/constraint_eval.ts b/toolbox/mdcode/src/libts/semantic/runtime/constraint_eval.ts index 98c10ddf..b8c14efd 100644 --- a/toolbox/mdcode/src/libts/semantic/runtime/constraint_eval.ts +++ b/toolbox/mdcode/src/libts/semantic/runtime/constraint_eval.ts @@ -56,6 +56,7 @@ import { SemanticModel, ViolationEffect, } from '../ir'; +import {blankStringLiterals, STRING_LITERAL} from '../sql_expr_utils'; import {quoteIfReserved, referencedParameters} from '../sql_identifiers'; @@ -531,20 +532,6 @@ function parseExpression( } -// The expression with every single-quoted span blanked out, character for -// character, so a scan can find structure without seeing inside a literal. -// Offsets are preserved, which is the point: the caller matches against the -// mask and slices the original at the same index. -// -// Without it `Account.status = 'ON HOLD OR CLOSED'` splits on the OR inside -// the string and the rule is refused for a fault it does not have. `isLiteral` -// already bars an embedded quote or backslash, so a literal is exactly the -// text between one pair of quotes. -function maskLiterals(expression: string): string { - return expression.replace(/'[^']*'/g, m => `'${'.'.repeat(m.length - 2)}'`); -} - - // Splits on top-level AND/OR, matched as whole words so a field named `brand` // survives. There are no parentheses to nest -- parseExpression refuses them -- // so every operator found is top level. @@ -552,7 +539,15 @@ function splitOnLogicalOperators(expression: string): {parts: string[]; joiners: string[]} { const parts: string[] = []; const joiners: string[] = []; - const masked = maskLiterals(expression); + // Scan a copy with the literals masked, so `Account.status = 'ON HOLD OR + // CLOSED'` does not come apart inside the quotes. The mask is the same length + // as what it replaces, so every index still points into the original. + // + // `blankStringLiterals` fills with spaces, which is wrong here: a blanked + // literal would join the whitespace on either side of it, and `\s+AND\s+` + // would then match across the space the literal used to occupy. The filler + // has to be something `\s` does not match. + const masked = expression.replace(STRING_LITERAL, m => '.'.repeat(m.length)); const pattern = /\s+(AND|OR)\s+/gi; let last = 0; let match: RegExpExecArray|null; @@ -634,12 +629,12 @@ function parseOperand( // The first comparison operator in `text`, longest match first so `>=` is not -// read as `>` with a stray `=` after it. Read against the masked text, so an -// operator character inside a string literal -- `'a>b' = Order.tag` -- is not -// mistaken for the comparison. +// read as `>` with a stray `=` after it. Read with the literals blanked, so an +// operator character inside a string -- `'a>b' = Order.tag` -- is not mistaken +// for the comparison. function findOperator(text: string): {operator: string; index: number}|null { let best: {operator: string; index: number}|null = null; - const masked = maskLiterals(text); + const masked = blankStringLiterals(text); for (const operator of OPERATORS) { const index = masked.indexOf(operator); if (index < 0) continue; From 60c1cd534a13162a01d13820cfc1e8e435649624 Mon Sep 17 00:00:00 2001 From: Bei Li Date: Sun, 13 Sep 2026 23:59:06 +0000 Subject: [PATCH 4/5] fix(mdcode): mask literals in the last two gates, and refuse a rule needing both moments Two defects in the guard evaluator, both found by exercising the lowering rather than by reading it. The gates that refuse `==` and parentheses read the expression raw, so `Order.status = 'a==b'` and `Order.note = 'see (attached)'` were turned away for a fault they do not have. On a reject-class guard that leaves the action permanently unrunnable. Both now read the text with its literals blanked, which is what the AND/OR split and the operator scan already did. Timing was derived once for the whole expression, from whether any comparison read a parameter. `Order.total >= 0 AND amount > 0` therefore lowered to one probe timed `before`, which checked the stored conjunct against the pre-state and never looked again: the write that drove the total negative committed, and the outcome reported the rejecting guard as checked. A probe runs at a single moment, so an expression needing both is now refused, the way one spanning two entities already is. --- toolbox/mdcode/docs/semantic-model/actions.md | 6 ++- .../libts/semantic/runtime/constraint_eval.ts | 38 +++++++++++++++---- .../semantic/runtime/constraint_eval.test.ts | 26 +++++++++++++ 3 files changed, 61 insertions(+), 9 deletions(-) diff --git a/toolbox/mdcode/docs/semantic-model/actions.md b/toolbox/mdcode/docs/semantic-model/actions.md index 76b1edd8..fe26e3db 100644 --- a/toolbox/mdcode/docs/semantic-model/actions.md +++ b/toolbox/mdcode/docs/semantic-model/actions.md @@ -1311,7 +1311,11 @@ This is a prototype. Five things a reader reasonably expects are absent. precondition no model can currently express: written as a guard it is checked after the close and fails every call. Say it with a parameter the rule can read, or leave it to the statement's own `WHERE` clause, until the model has a - way to name the moment. + way to name the moment. For the same reason a rule that asks about both at + once, such as `Order.total >= 0 AND amount > 0`, is refused rather than timed + as one: a probe runs at a single moment, and checking the stored half against + the pre-state would let the write that breaks it commit while the rule reports + as checked. Write one constraint for each and name both in `guards`. - **`kcmd` calls no executor but its own.** A `sql` action runs; an `mcp`, `rest` or `grpc` one is published for whoever dispatches it, which is why those three name coordinates rather than a statement. diff --git a/toolbox/mdcode/src/libts/semantic/runtime/constraint_eval.ts b/toolbox/mdcode/src/libts/semantic/runtime/constraint_eval.ts index b8c14efd..8c4d17a7 100644 --- a/toolbox/mdcode/src/libts/semantic/runtime/constraint_eval.ts +++ b/toolbox/mdcode/src/libts/semantic/runtime/constraint_eval.ts @@ -208,12 +208,29 @@ export function lowerGuard( `them together in 'guards'`); } - // A rule that reads a parameter asks about this call, so it is answered - // before the write; one that reads only stored state asks whether the data - // is sound, which only the post-state can answer. - const readsParameter = parsed.comparisons.some( - c => c.left.kind === 'parameter' || c.right.kind === 'parameter'); - const timing: ProbeTiming = readsParameter ? 'before' : 'after'; + // A comparison that reads a parameter asks about this call, so it is + // answered before the write; one that reads only stored state asks whether + // the data is sound, which only the post-state can answer. + const readsParameter = (c: Comparison) => + c.left.kind === 'parameter' || c.right.kind === 'parameter'; + const readsField = (c: Comparison) => + c.left.kind === 'field' || c.right.kind === 'field'; + const aboutTheCall = parsed.comparisons.filter(readsParameter); + const aboutTheData = + parsed.comparisons.filter(c => !readsParameter(c) && readsField(c)); + + // One probe runs at one moment, so an expression that needs both cannot be + // lowered whole. Timing it by whether any comparison reads a parameter would + // put the stored half against the pre-state and never look again: the write + // that breaks it commits, and the rule reports as checked. + if (aboutTheCall.length && aboutTheData.length) { + return fail( + `it asks both about this call's arguments and about stored data ` + + `(${constraint.expression}); the first is answered before the write ` + + `and the second after it, so write one constraint for each and list ` + + `them together in 'guards'`); + } + const timing: ProbeTiming = aboutTheCall.length ? 'before' : 'after'; if (!entityNames.size) { // No table to read: the rule is entirely about the call's own arguments. @@ -508,13 +525,18 @@ function parseExpression( parameters: Map): ParsedExpression|{error: string} { const text = expression.trim(); if (!text) return {error: 'it declares no expression'}; - if (text.includes('==')) { + // Both gates below read the text with its literals blanked. A rule whose + // literal happens to spell `==` or hold a bracket -- `Order.status = 'a==b'`, + // `Order.note = 'see (attached)'` -- says nothing about the grammar, and + // refusing it leaves a reject-class guard permanently unrunnable. + const bare = blankStringLiterals(text); + if (bare.includes('==')) { return { error: `it writes '==' (${text}); equality in the expression language ` + `is a single '='`, }; } - if (/[()]/.test(text)) { + if (/[()]/.test(bare)) { return { error: `it uses parentheses or a function call (${ text}), which the grammar does not parse`, diff --git a/toolbox/mdcode/tests/libts/semantic/runtime/constraint_eval.test.ts b/toolbox/mdcode/tests/libts/semantic/runtime/constraint_eval.test.ts index a79a9388..31d36616 100644 --- a/toolbox/mdcode/tests/libts/semantic/runtime/constraint_eval.test.ts +++ b/toolbox/mdcode/tests/libts/semantic/runtime/constraint_eval.test.ts @@ -196,6 +196,20 @@ describe('what the probe reads and when it runs', () => { expect(probe.sql).toContain('NOT COALESCE((@amount <= Total), FALSE)'); }); + test('a rule needing both moments is refused rather than timed as one', + () => { + // One probe runs at one moment. Timing this by whether any comparison + // reads a parameter would check `Order.total >= 0` against the + // pre-state and never look again, so the write that drives the total + // negative commits while the rule reports as checked. + const reason = reasonOf(lower('Order.total >= 0 AND amount > 0')); + expect(reason).toContain("about this call's arguments"); + expect(reason).toContain('about stored data'); + // The same holds for OR, which cannot be split into two probes at all. + expect(reasonOf(lower('Order.total >= 0 OR amount > 0'))) + .toContain('about stored data'); + }); + test('the probe is scoped to the rows the call names', () => { // Without the scope this is a table scan holding read locks for the length // of the write, which is how a gate gets switched off. @@ -231,6 +245,18 @@ describe('the expressions the grammar accepts', () => { .toContain("NOT COALESCE((Status != 'ON HOLD OR CLOSED'), FALSE)"); }); + test('a literal spelling an operator the grammar bars is still a literal', + () => { + // The gates that refuse `==` and parentheses read the expression with + // its literals blanked. Reading raw text refused these two for a fault + // they do not have, which on a reject-class guard leaves the action + // permanently unrunnable. + expect(probeOf(lower("Order.status = 'a==b'")).sql) + .toContain("NOT COALESCE((Status = 'a==b'), FALSE)"); + expect(probeOf(lower("Order.status = 'see (attached)'")).sql) + .toContain("NOT COALESCE((Status = 'see (attached)'), FALSE)"); + }); + test('an operator inside a literal is not the comparison', () => { expect(probeOf(lower("'a>b' != Order.status")).sql) .toContain("NOT COALESCE(('a>b' != Status), FALSE)"); From 79709b03648bb0bd31ff881eaa2c18a8bb332b3a Mon Sep 17 00:00:00 2001 From: Bei Li Date: Mon, 14 Sep 2026 02:12:13 +0000 Subject: [PATCH 5/5] refactor(mdcode): one module per way a constraint is checked The evaluator did five jobs in one file -- scan the expression, decide when the rule runs, resolve logical names to physical ones, write GoogleSQL, report -- so a second way to check a rule meant editing all five. Two things vary, and independently. WHAT SETTLES THE RULE is the constraint's own body, an axis the IR has named since CONSTRAINT_EVALUATIONS landed while the evaluator special-cased it with one `if`. It is now a registry keyed by that enumeration and total over it, so a third body cannot be added to the model without failing to compile here. HOW A STORE IS ASKED is the query language -- GoogleSQL over the entity's table today, GQL over the pushed property graph next -- and dialect.ts holds that alone. The point of the split is that deciding and spelling come apart. Whether a rule may be checked, which entity it reads and which moment it runs at are settled in analysis.ts with no store and no language in hand; which table and column it reads comes from the profile in bind.ts; only the writing down belongs to the dialect. A dialect can therefore neither widen what the runtime agrees to check nor move when it checks it, which a test now holds it to by planning the same rules through a second one. The package also stops naming a backend. A check reports the parameters it wrote rather than having them scanned back out of its SQL, and the one place a Spanner statement is built is run_action.ts, alongside every other one. Nothing the runtime does changes. The GoogleSQL a rule lowers to is asserted byte for byte, and the 51 tests pass unchanged, with 3 added for the seam. --- .../libts/semantic/runtime/constraint_eval.ts | 706 ------------------ .../semantic/runtime/constraints/analysis.ts | 106 +++ .../semantic/runtime/constraints/bind.ts | 132 ++++ .../semantic/runtime/constraints/check.ts | 213 ++++++ .../semantic/runtime/constraints/dialect.ts | 88 +++ .../runtime/constraints/expression.ts | 217 ++++++ .../semantic/runtime/constraints/index.ts | 142 ++++ .../semantic/runtime/constraints/judgment.ts | 29 + .../semantic/runtime/constraints/sql_check.ts | 151 ++++ .../src/libts/semantic/runtime/run_action.ts | 60 +- ...raint_eval.test.ts => constraints.test.ts} | 120 ++- 11 files changed, 1203 insertions(+), 761 deletions(-) delete mode 100644 toolbox/mdcode/src/libts/semantic/runtime/constraint_eval.ts create mode 100644 toolbox/mdcode/src/libts/semantic/runtime/constraints/analysis.ts create mode 100644 toolbox/mdcode/src/libts/semantic/runtime/constraints/bind.ts create mode 100644 toolbox/mdcode/src/libts/semantic/runtime/constraints/check.ts create mode 100644 toolbox/mdcode/src/libts/semantic/runtime/constraints/dialect.ts create mode 100644 toolbox/mdcode/src/libts/semantic/runtime/constraints/expression.ts create mode 100644 toolbox/mdcode/src/libts/semantic/runtime/constraints/index.ts create mode 100644 toolbox/mdcode/src/libts/semantic/runtime/constraints/judgment.ts create mode 100644 toolbox/mdcode/src/libts/semantic/runtime/constraints/sql_check.ts rename toolbox/mdcode/tests/libts/semantic/runtime/{constraint_eval.test.ts => constraints.test.ts} (82%) diff --git a/toolbox/mdcode/src/libts/semantic/runtime/constraint_eval.ts b/toolbox/mdcode/src/libts/semantic/runtime/constraint_eval.ts deleted file mode 100644 index 8c4d17a7..00000000 --- a/toolbox/mdcode/src/libts/semantic/runtime/constraint_eval.ts +++ /dev/null @@ -1,706 +0,0 @@ -// Checking a model's constraints against a live store. -// -// A constraint is a logical invariant over the ontology (`Order.total >= 0`, -// `amount <= Order.total`). Enforcing one against a store means turning that -// logical statement into a query the store can answer. That is this module: -// given a model, an action and a constraint the action `guards`, it produces a -// PROBE -- a SELECT that returns the rows which VIOLATE the constraint. No rows -// means the rule holds. -// -// WHEN a probe runs is derived from the expression rather than authored, and -// the two answers are genuinely different checks: -// -// * A GUARD reads an action parameter (`amount <= Order.total`). It asks -// whether this call may proceed, so it runs BEFORE the writes, against the -// rows the arguments denote. A parameter is not in the store, so no -// post-state check could ask it. -// * An INVARIANT reads only stored state (`Order.total >= 0`). It asks -// whether the data is still sound, so it runs AFTER the writes and before -// the commit, when the new state exists to be read. -// -// Both run inside the action's own transaction, so a violation rolls back with -// everything else and no violating state is ever visible to another reader. -// -// Three properties matter more than expressive power: -// -// * The lowering FAILS CLOSED. An expression this module cannot lower does -// not quietly pass: it returns a reason, and the runtime refuses the action -// rather than running it unchecked. A gate that lets writes through is -// worse than no gate, because it is believed. -// * The lowering needs no argument VALUES. A probe is SQL with the action's -// own parameters left as `@name`, so the call that decides whether an -// action is runnable at all -- asked before any argument arrives, to decide -// whether to offer an agent the tool -- produces the very SQL that later -// runs. One answer, so the advertised verdict and the real one cannot -// drift. -// * A probe is SCOPED to the rows the call touches. An action writes a -// handful of rows, and a gate whose cost grows with the table is a gate -// that gets switched off. -// -// The grammar is small (see parseExpression): comparisons between a field, an -// action parameter and a literal, joined by AND/OR. It covers the rules an -// operational action actually trips -- an amount over a ceiling, a credit -// larger than what it credits -- and everything outside it is reported rather -// than approximated. Aggregates, parentheses and function calls are named as -// unlowerable, which refuses the action rather than guessing at it. - -import * as spanner from '../../gcp/spanner'; - -import {spannerTable} from '../binding'; -import { - Action, - Constraint, - constraintEvaluation, - Entity, - fieldBinding, - SemanticModel, - ViolationEffect, -} from '../ir'; -import {blankStringLiterals, STRING_LITERAL} from '../sql_expr_utils'; -import {quoteIfReserved, referencedParameters} from '../sql_identifiers'; - - -// When a probe runs, relative to the action's own writes. -// -// - `before` the write has not happened. The probe reads the pre-state and -// the call's arguments, and a violation means the call is refused. -// - `after` the writes have run in the transaction but nothing is committed. -// The probe reads the post-state, and a violation rolls it back. -export type ProbeTiming = 'before'|'after'; - - -// A constraint lowered to SQL, ready to run inside the action's transaction. -export interface ConstraintProbe { - constraint: Constraint; - timing: ProbeTiming; - // The entity the probe reads, absent when the expression names none: a rule - // over the call's arguments alone, such as `amount <= 25`, reads no table. - entity?: string; - // A SELECT returning violating rows; an empty result means the rule holds. - // Parameters are the action's own, bound by the caller at run time. - sql: string; - // The columns `sql` selects, so a violation can name the rows it found - // rather than only reporting that one exists. - columns: string[]; -} - - -export type Lowering = { - ok: true; probe: ConstraintProbe; -}|{ - ok: false; - // Why this constraint cannot be checked here, phrased for whoever wrote the - // model: the runtime surfaces it verbatim when it refuses the action. - reason: string; -}; - - -// A rule that did not hold, in the terms a caller acts on. -export interface ConstraintViolation { - constraint: string; - effect: ViolationEffect; - // The constraint's own `description` where it has one, which is written as - // the instruction to the refused caller, followed by the citation. - message: string; - // The violating rows, each as its key values joined by '/'. Empty for a rule - // over the arguments alone, which has no row to name. - instances: string[]; -} - - -// A rule that was named as a guard and not evaluated. Only an advisory rule -// reaches this: one that stops the call and cannot be checked refuses the -// action instead. -export interface UncheckedRule { - constraint: string; - // Why it could not be checked, in the same words a refusal would have used. - reason: string; -} - - -// How many violating rows a probe returns. A gate needs enough to explain -// itself, not the whole violation set. -const PROBE_LIMIT = 5; - - -// The comparison operators the grammar accepts, longest first so `>=` is -// matched before `>`. -const OPERATORS = ['>=', '<=', '!=', '<>', '=', '>', '<'] as const; - - -/** - * Lowers every constraint `action` names in `guards`. - * - * Returns the probes it could build, the reasons for the ones it could not, - * and the advisory rules it had to leave unevaluated. A non-empty `errors` - * means the action cannot be run: the model says the call is checked, and a - * check that cannot be performed is not one. - * - * An ADVISORY rule -- `on_violation: warn` -- is the exception, and it is - * `unchecked` rather than an error. It reports and lets the write through, so - * being unable to evaluate it costs the caller a report and stops nothing; - * refusing over it would turn a rule the author wrote as advice into the one - * thing that makes the action unrunnable. It is still named, because a report - * that was owed and not made is news in its own right. - */ -export function lowerGuards(model: SemanticModel, action: Action): { - probes: ConstraintProbe[]; - errors: string[]; - unchecked: UncheckedRule[]; -} { - const probes: ConstraintProbe[] = []; - const errors: string[] = []; - const unchecked: UncheckedRule[] = []; - for (const name of action.guards ?? []) { - const constraint = (model.constraints ?? []).find(c => c.name === name); - if (!constraint) { - // Not classifiable as advisory: the model declares nothing by this name, - // so there is no `on_violation` to read, and guessing is not on offer. - errors.push( - `action '${action.name}' is guarded by '${name}', which this model ` + - `does not declare`); - continue; - } - const lowered = lowerGuard(model, action, constraint); - if (lowered.ok) { - probes.push(lowered.probe); - } else if (effectOf(constraint) === 'warn') { - unchecked.push({constraint: name, reason: lowered.reason}); - } else { - errors.push(lowered.reason); - } - } - return {probes, errors, unchecked}; -} - - -/** Lowers one constraint as a gate on `action`, or says why it cannot be. */ -export function lowerGuard( - model: SemanticModel, action: Action, constraint: Constraint): Lowering { - const fail = (reason: string): Lowering => ({ - ok: false, - reason: `constraint '${constraint.name}' cannot be checked: ${reason}`, - }); - - // A judged rule is settled by a language model reading the proposed change. - // Nothing here calls one, and the honest report of that is a refusal: the - // alternative is an action whose model says it is judged running unjudged. - if (constraintEvaluation(constraint) === 'judged') { - return fail( - `it is settled by judgment rather than by an expression, and this ` + - `runtime runs no judge`); - } - - const parameters = new Map(action.parameters.map(p => [p.name, p])); - const parsed = parseExpression(constraint.expression ?? '', parameters); - if ('error' in parsed) return fail(parsed.error); - - const entityNames = new Set(); - for (const comparison of parsed.comparisons) { - for (const operand of [comparison.left, comparison.right]) { - if (operand.kind === 'field') entityNames.add(operand.entity); - } - } - if (entityNames.size > 1) { - return fail( - `it spans ${[...entityNames].sort().join(' and ')}; a probe reads ` + - `one entity's table, so write one constraint per entity and list ` + - `them together in 'guards'`); - } - - // A comparison that reads a parameter asks about this call, so it is - // answered before the write; one that reads only stored state asks whether - // the data is sound, which only the post-state can answer. - const readsParameter = (c: Comparison) => - c.left.kind === 'parameter' || c.right.kind === 'parameter'; - const readsField = (c: Comparison) => - c.left.kind === 'field' || c.right.kind === 'field'; - const aboutTheCall = parsed.comparisons.filter(readsParameter); - const aboutTheData = - parsed.comparisons.filter(c => !readsParameter(c) && readsField(c)); - - // One probe runs at one moment, so an expression that needs both cannot be - // lowered whole. Timing it by whether any comparison reads a parameter would - // put the stored half against the pre-state and never look again: the write - // that breaks it commits, and the rule reports as checked. - if (aboutTheCall.length && aboutTheData.length) { - return fail( - `it asks both about this call's arguments and about stored data ` + - `(${constraint.expression}); the first is answered before the write ` + - `and the second after it, so write one constraint for each and list ` + - `them together in 'guards'`); - } - const timing: ProbeTiming = aboutTheCall.length ? 'before' : 'after'; - - if (!entityNames.size) { - // No table to read: the rule is entirely about the call's own arguments. - // `UNNEST([1])` is the one-row source GoogleSQL needs for a SELECT that - // has a WHERE and nothing to select from. - const predicate = renderPredicate(parsed, new Map()); - return { - ok: true, - probe: { - constraint, - timing, - sql: `SELECT 1 AS violated FROM UNNEST([1]) WHERE ${ - violating(predicate)}`, - columns: ['violated'], - }, - }; - } - - const entityName = [...entityNames][0]; - const entity = (model.entities ?? []).find(e => e.name === entityName); - if (!entity) return fail(`'${entityName}' is not an entity of this model`); - if (entity.abstract) { - return fail(`'${entityName}' is abstract, so it has no table to read`); - } - - const columns = new Map(); - for (const comparison of parsed.comparisons) { - for (const operand of [comparison.left, comparison.right]) { - if (operand.kind !== 'field') continue; - if (columns.has(operand.field)) continue; - const column = columnFor(entity, operand.field); - if ('error' in column) return fail(column.error); - columns.set(operand.field, column.column); - } - } - - const scope = scopeToTouchedRows(action, entity); - if ('error' in scope) return fail(scope.error); - - const keys = keyColumns(entity); - if ('error' in keys) return fail(keys.error); - - const warnings: string[] = []; - const table = - spannerTable(entity.dataSource, warnings, `entity '${entity.name}'`); - if (warnings.length) { - return fail(`'${entityName}' has no usable table (${warnings.join('; ')})`); - } - - const predicate = renderPredicate(parsed, columns); - return { - ok: true, - probe: { - constraint, - timing, - entity: entityName, - sql: `SELECT ${keys.columns.join(', ')} FROM ${table} WHERE ${ - scope.predicate} AND ${violating(predicate)} LIMIT ${PROBE_LIMIT}`, - columns: keys.columns, - }, - }; -} - - -/** - * `probe` as a statement, carrying the argument values it reads. - * - * Filtered to the parameters the SQL actually names: a statement carrying one - * it never reads is a statement the store may refuse, and an action's - * parameter list is wider than any single rule. - */ -export function probeStatement( - probe: ConstraintProbe, params: Record, - types: Record): spanner.Statement { - const statement: spanner.Statement = {sql: probe.sql}; - const used: Record = {}; - const usedTypes: Record = {}; - for (const name of new Set(referencedParameters(probe.sql))) { - if (!(name in params)) continue; - used[name] = params[name]; - if (types[name]) usedTypes[name] = types[name]; - } - if (Object.keys(used).length) { - statement.params = used; - statement.paramTypes = usedTypes; - } - return statement; -} - - -/** - * A violation of `probe`, given the rows it returned. - * - * The constraint's `description` leads, because it is the model author's own - * words about what the caller should do differently; the name and the - * expression follow as the citation for it. - */ -export function violationFrom( - probe: ConstraintProbe, rows: string[][]): ConstraintViolation { - const constraint = probe.constraint; - const lead = constraint.description?.trim() || - `Constraint '${constraint.name}' does not hold.`; - const parts = - [lead, `Stopped by '${constraint.name}' (${constraint.expression}).`]; - // Rows are named only when they identify something: the one-row result of a - // rule over the arguments alone says nothing a reader can use. - const instances = probe.entity ? rows.map(row => row.join('/')) : []; - if (instances.length) { - parts.push(`Violating ${probe.entity}: ${instances.join(', ')}.`); - } - return { - constraint: constraint.name, - effect: effectOf(constraint), - message: parts.join(' '), - instances, - }; -} - - -/** - * What a violated constraint does to the write. - * - * An `expression` that does not say defaults to `reject`, which is the safe - * reading of an author who did not say. See VIOLATION_EFFECTS in ir.ts. - */ -export function effectOf(constraint: Constraint): ViolationEffect { - return constraint.onViolation ?? 'reject'; -} - - -// Harshest first. A call that trips two rules gets the stricter answer: being -// told a supervisor could approve a write another rule forbids outright would -// send the caller to ask for something nobody can give. -const EFFECT_ORDER: ViolationEffect[] = ['reject', 'escalate', 'warn']; - - -/** The strictest effect among `violations`, or null if there are none. */ -export function strictestEffect(violations: readonly ConstraintViolation[]): - ViolationEffect|null { - for (const effect of EFFECT_ORDER) { - if (violations.some(v => v.effect === effect)) return effect; - } - return null; -} - - -// NOT COALESCE(p, FALSE) rather than a plain NOT: SQL's three-valued logic -// makes `NULL >= 0` unknown and `NOT unknown` unknown too, so a NULL column -// would slip past a plain negation. Reading unknown as "did not satisfy the -// rule" makes the row a violation, which is the fail-closed answer a gate owes. -function violating(predicate: string): string { - return `NOT COALESCE(${predicate}, FALSE)`; -} - - -// Restricts the probe to the rows this call touches. -// -// An action names the rows it acts on through its entity-typed parameters, and -// that reference is what makes the probe cheap and its answer relevant. -// Without one, `amount <= Order.total` would be asked of every order in the -// table and fail on the first unrelated one, so a constraint over an entity -// the action does not take as a parameter is refused rather than widened into -// a table scan. Checking stored state at large is a different binding point -- -// a conformance sweep over the data rather than a gate on one call -- and it -// needs its own reference instead of this one silently standing in for it. -// -// EVERY parameter of that entity is in scope, not the first one found. -// `TransferFunds(source: Account, target: Account, amount)` writes both -// accounts, so a rule over `Account` that asked only about `source` would let -// the write that breaks `target` through while reporting the rule as checked. -// A gate that answers about some of the rows it was asked about is worse than -// one that refuses, because its answer is believed. -function scopeToTouchedRows( - action: Action, entity: Entity): {predicate: string}|{error: string} { - const params = - action.parameters.filter(p => p.isEntityRef && p.type === entity.name); - if (!params.length) { - return { - error: `it reads ${entity.name}, and action '${action.name}' takes no ` + - `${entity.name} parameter, so the probe could not be limited to ` + - `the rows this call touches`, - }; - } - const keys = keyColumns(entity); - if ('error' in keys) return {error: keys.error}; - if (keys.columns.length !== 1) { - return { - error: `${entity.name} has a ${keys.columns.length}-part key, and the ` + - `runtime binds an object reference as a single value`, - }; - } - const key = keys.columns[0]; - if (params.length === 1) { - return {predicate: `${key} = @${params[0].name}`}; - } - return { - predicate: `${key} IN (${params.map(p => `@${p.name}`).join(', ')})`, - }; -} - - -// The physical column behind `fieldName`, or why there is none. A bare column -// is required: a field bound to an expression (`price * quantity`) would need -// that expression inlined and re-resolved, which this grammar does not do. -function columnFor(entity: Entity, fieldName: string): - {column: string}|{error: string} { - const field = entity.fields.find(f => f.name === fieldName); - if (!field) { - return {error: `${entity.name} declares no field '${fieldName}'`}; - } - // No binding is what unbound means: the profile in force bound nothing to - // this field, so there is no column to read the rule against. - const binding = (fieldBinding(field) ?? '').trim(); - if (!binding) { - return { - error: `${entity.name}.${fieldName} is unbound under this profile, so ` + - `there is nothing to read it from`, - }; - } - if (!/^[A-Za-z_]\w*$/.test(binding)) { - return { - error: `${entity.name}.${fieldName} is bound to an expression (${ - binding}) rather than to a column`, - }; - } - return {column: quoteIfReserved(binding)}; -} - - -// The entity's key columns, resolved through its fields. -function keyColumns(entity: Entity): {columns: string[]}|{error: string} { - if (!entity.keys?.length) { - return { - error: `${entity.name} declares no key, so a violation could not be ` + - `attributed to a row`, - }; - } - const columns: string[] = []; - for (const key of entity.keys) { - const column = columnFor(entity, key); - if ('error' in column) return {error: `its key ${column.error}`}; - columns.push(column.column); - } - return {columns}; -} - - -// An operand of a comparison: a field of an entity, an action parameter, or a -// literal already in SQL form. -type Operand = { - kind: 'field'; entity: string; field: string; -}|{ - kind: 'parameter'; name: string; -}|{ - kind: 'literal'; text: string; -}; - - -interface Comparison { - left: Operand; - right: Operand; - operator: string; -} - - -// Comparisons and the logical operators between them: `joiners[i]` sits -// between `comparisons[i]` and `comparisons[i + 1]`. -interface ParsedExpression { - comparisons: Comparison[]; - joiners: string[]; -} - - -// Parses a constraint expression. -// -// The grammar: -// -// expression := comparison (('AND' | 'OR') comparison)* -// comparison := operand operand -// operand := . | | literal -// op := >= | <= | != | <> | = | > | < -// literal := a number, a single-quoted string, TRUE, FALSE or NULL -// -// `= NULL` and `!= NULL` read as null tests and lower to IS NULL / IS NOT NULL. -// Parentheses, function calls, aggregates, IN, BETWEEN, LIKE and metric -// references are all outside the grammar, on purpose. Each is a real thing a -// constraint might want and each needs a decision this module does not make -- -// how an aggregate is evaluated inside a row-level probe, for one -- so each is -// refused with a reason rather than half-handled. -function parseExpression( - expression: string, - parameters: Map): ParsedExpression|{error: string} { - const text = expression.trim(); - if (!text) return {error: 'it declares no expression'}; - // Both gates below read the text with its literals blanked. A rule whose - // literal happens to spell `==` or hold a bracket -- `Order.status = 'a==b'`, - // `Order.note = 'see (attached)'` -- says nothing about the grammar, and - // refusing it leaves a reject-class guard permanently unrunnable. - const bare = blankStringLiterals(text); - if (bare.includes('==')) { - return { - error: `it writes '==' (${text}); equality in the expression language ` + - `is a single '='`, - }; - } - if (/[()]/.test(bare)) { - return { - error: `it uses parentheses or a function call (${ - text}), which the grammar does not parse`, - }; - } - - const split = splitOnLogicalOperators(text); - const comparisons: Comparison[] = []; - for (const segment of split.parts) { - const comparison = parseComparison(segment, parameters); - if ('error' in comparison) return comparison; - comparisons.push(comparison); - } - return {comparisons, joiners: split.joiners}; -} - - -// Splits on top-level AND/OR, matched as whole words so a field named `brand` -// survives. There are no parentheses to nest -- parseExpression refuses them -- -// so every operator found is top level. -function splitOnLogicalOperators(expression: string): - {parts: string[]; joiners: string[]} { - const parts: string[] = []; - const joiners: string[] = []; - // Scan a copy with the literals masked, so `Account.status = 'ON HOLD OR - // CLOSED'` does not come apart inside the quotes. The mask is the same length - // as what it replaces, so every index still points into the original. - // - // `blankStringLiterals` fills with spaces, which is wrong here: a blanked - // literal would join the whitespace on either side of it, and `\s+AND\s+` - // would then match across the space the literal used to occupy. The filler - // has to be something `\s` does not match. - const masked = expression.replace(STRING_LITERAL, m => '.'.repeat(m.length)); - const pattern = /\s+(AND|OR)\s+/gi; - let last = 0; - let match: RegExpExecArray|null; - while ((match = pattern.exec(masked)) !== null) { - parts.push(expression.slice(last, match.index)); - joiners.push(match[1].toUpperCase()); - last = match.index + match[0].length; - } - parts.push(expression.slice(last)); - return {parts, joiners}; -} - - -function parseComparison( - segment: string, - parameters: Map): Comparison|{error: string} { - const text = segment.trim(); - const found = findOperator(text); - if (!found) { - return { - error: `'${text}' is not a comparison (expected one of ${ - OPERATORS.join(', ')})`, - }; - } - const leftText = text.slice(0, found.index).trim(); - const rightText = text.slice(found.index + found.operator.length).trim(); - if (!leftText) return {error: `'${text}' has nothing left of the operator`}; - if (!rightText) return {error: `'${text}' has nothing right of the operator`}; - - const left = parseOperand(leftText, parameters); - if ('error' in left) return {error: `in '${text}', ${left.error}`}; - const right = parseOperand(rightText, parameters); - if ('error' in right) return {error: `in '${text}', ${right.error}`}; - - const operator = found.operator === '<>' ? '!=' : found.operator; - - // GoogleSQL refuses `col = NULL` outright rather than evaluating it to - // unknown, so lowering it verbatim would emit a probe that cannot run. An - // author writing `Order.closedOn != NULL` means the column must be - // populated, which SQL spells IS NOT NULL -- so the two operators with a - // null-test reading are translated and the four without one are refused, an - // ordering comparison against NULL having no meaning to preserve. - const isNull = (operand: Operand) => - operand.kind === 'literal' && /^NULL$/i.test(operand.text); - if (isNull(left) || isNull(right)) { - if (operator !== '=' && operator !== '!=') { - return { - error: `'${text}' compares with NULL using '${operator}', which has ` + - `no meaning; write '= NULL' or '!= NULL' to ask whether the ` + - `field is set`, - }; - } - return { - left: isNull(left) ? right : left, - right: {kind: 'literal', text: 'NULL'}, - operator: operator === '=' ? 'IS' : 'IS NOT', - }; - } - - return {left, right, operator}; -} - - -function parseOperand( - text: string, - parameters: Map): Operand|{error: string} { - const field = text.match(/^([A-Za-z_]\w*)\.([A-Za-z_]\w*)$/); - if (field) return {kind: 'field', entity: field[1], field: field[2]}; - if (isLiteral(text)) return {kind: 'literal', text}; - if (/^[A-Za-z_]\w*$/.test(text)) { - if (parameters.has(text)) return {kind: 'parameter', name: text}; - return { - error: `'${text}' is not a parameter of this action; a field is ` + - `written .`, - }; - } - return {error: `'${text}' is not a field, a parameter or a literal`}; -} - - -// The first comparison operator in `text`, longest match first so `>=` is not -// read as `>` with a stray `=` after it. Read with the literals blanked, so an -// operator character inside a string -- `'a>b' = Order.tag` -- is not mistaken -// for the comparison. -function findOperator(text: string): {operator: string; index: number}|null { - let best: {operator: string; index: number}|null = null; - const masked = blankStringLiterals(text); - for (const operator of OPERATORS) { - const index = masked.indexOf(operator); - if (index < 0) continue; - if (!best || index < best.index || - (index === best.index && operator.length > best.operator.length)) { - best = {operator, index}; - } - } - return best; -} - - -// A literal the probe embeds verbatim, restricted to shapes with no quoting -// hazard: a number, a single-quoted string with no embedded quote or -// backslash, or one of the three keywords. Anything else is refused rather -// than escaped, because a constraint expression is model text and a surprising -// escape is harder to notice than a refusal. -function isLiteral(text: string): boolean { - if (/^[-+]?\d+(\.\d+)?$/.test(text)) return true; - if (/^'[^'\\]*'$/.test(text)) return true; - return /^(TRUE|FALSE|NULL)$/i.test(text); -} - - -// Renders the parsed expression against the physical columns. Each comparison -// is parenthesized, so a mixed AND/OR expression keeps the precedence the SQL -// engine gives it rather than one this module invents. -function renderPredicate( - parsed: ParsedExpression, columns: Map): string { - const render = (operand: Operand): string => { - switch (operand.kind) { - case 'field': - return columns.get(operand.field)!; - case 'parameter': - return `@${operand.name}`; - case 'literal': - return operand.text; - } - }; - const parts = parsed.comparisons.map( - c => `(${render(c.left)} ${c.operator} ${render(c.right)})`); - let out = parts[0]; - for (let i = 1; i < parts.length; i++) { - out = `${out} ${parsed.joiners[i - 1]} ${parts[i]}`; - } - return out; -} diff --git a/toolbox/mdcode/src/libts/semantic/runtime/constraints/analysis.ts b/toolbox/mdcode/src/libts/semantic/runtime/constraints/analysis.ts new file mode 100644 index 00000000..567ca8f3 --- /dev/null +++ b/toolbox/mdcode/src/libts/semantic/runtime/constraints/analysis.ts @@ -0,0 +1,106 @@ +// What a constraint says, before anything decides how to ask it. +// +// Three questions are settled here, and none of them depends on the store, on +// the profile in force or on the query language: whether the rule is inside +// the grammar at all, which names it reads, and WHEN it has to be asked. +// Keeping them here is what lets a second dialect emit the same rule without +// re-deciding any of it, and what lets the answer come before a session is +// ever opened -- the call that decides whether to offer an agent a tool asks +// this with no arguments in hand. +// +// The two moments are genuinely different checks: +// +// * A GUARD reads an action parameter (`amount <= Order.total`). It asks +// whether this call may proceed, so it runs BEFORE the writes, against the +// rows the arguments denote. A parameter is not in the store, so no +// post-state check could ask it. +// * An INVARIANT reads only stored state (`Order.total >= 0`). It asks +// whether the data is still sound, so it runs AFTER the writes and before +// the commit, when the new state exists to be read. + +import {Action, Constraint} from '../../ir'; + +import {CheckTiming} from './check'; +import {Comparison, ParsedExpression, parseExpression} from './expression'; + + +/** A constraint expression, read for everything that does not vary by store. */ +export interface AnalyzedRule { + parsed: ParsedExpression; + timing: CheckTiming; + // The entities the rule reads, sorted, empty when it reads only the call's + // own arguments. Reported rather than judged: how many entities one check + // may span is a property of the shape it is emitted in, so whoever emits it + // decides. + entities: string[]; + // The fields it reads, in first-mention order. Meaningful when `entities` + // holds exactly one. + fields: string[]; + // The action parameters it reads, in first-mention order. + parameters: string[]; +} + + +/** Reads `constraint` as a rule over `action`, or says why it cannot be read. */ +export function analyze(action: Action, constraint: Constraint): AnalyzedRule| + {error: string} { + const declared = new Map(action.parameters.map(p => [p.name, p])); + const parsed = parseExpression(constraint.expression ?? '', declared); + if ('error' in parsed) return parsed; + + const entities = new Set(); + const fields: string[] = []; + const parameters: string[] = []; + for (const comparison of parsed.comparisons) { + for (const operand of [comparison.left, comparison.right]) { + if (operand.kind === 'field') { + entities.add(operand.entity); + if (!fields.includes(operand.field)) fields.push(operand.field); + } else if (operand.kind === 'parameter') { + if (!parameters.includes(operand.name)) parameters.push(operand.name); + } + } + } + + const timing = timingOf(parsed, constraint); + if ('error' in timing) return timing; + + return { + parsed, + timing: timing.timing, + entities: [...entities].sort(), + fields, + parameters, + }; +} + + +// When the rule has to be asked, read off what its comparisons reference. +// +// A comparison that reads a parameter asks about this call, so it is answered +// before the write; one that reads only stored state asks whether the data is +// sound, which only the post-state can answer. +function timingOf(parsed: ParsedExpression, constraint: Constraint): + {timing: CheckTiming}|{error: string} { + const readsParameter = (c: Comparison) => + c.left.kind === 'parameter' || c.right.kind === 'parameter'; + const readsField = (c: Comparison) => + c.left.kind === 'field' || c.right.kind === 'field'; + const aboutTheCall = parsed.comparisons.filter(readsParameter); + const aboutTheData = + parsed.comparisons.filter(c => !readsParameter(c) && readsField(c)); + + // One check runs at one moment, so an expression that needs both cannot be + // taken whole. Timing it by whether any comparison reads a parameter would + // put the stored half against the pre-state and never look again: the write + // that breaks it commits, and the rule reports as checked. + if (aboutTheCall.length && aboutTheData.length) { + return { + error: `it asks both about this call's arguments and about stored data ` + + `(${constraint.expression}); the first is answered before the write ` + + `and the second after it, so write one constraint for each and ` + + `list them together in 'guards'`, + }; + } + return {timing: aboutTheCall.length ? 'before' : 'after'}; +} diff --git a/toolbox/mdcode/src/libts/semantic/runtime/constraints/bind.ts b/toolbox/mdcode/src/libts/semantic/runtime/constraints/bind.ts new file mode 100644 index 00000000..b4a30cfe --- /dev/null +++ b/toolbox/mdcode/src/libts/semantic/runtime/constraints/bind.ts @@ -0,0 +1,132 @@ +// Logical names to physical ones. +// +// `Order.total` is what the model says; column `Total` of table `Orders` is +// what the store holds. Which one it resolves to depends on the profile in +// force and on nothing about the query language, which is why this sits apart +// from the dialect: GoogleSQL and GQL read the same column of the same table +// and write the reference differently. +// +// Everything here returns bare physical names. How a name is quoted belongs to +// the language reading it, so the dialect does that at the moment it writes +// the reference. The one exception is the table, which arrives from the shared +// binding layer already in the form the store addresses it by. + +import {spannerTable} from '../../binding'; +import {Action, Entity, fieldBinding} from '../../ir'; + + +/** The physical column behind each of `fields`, or why one has none. */ +export function columnsFor(entity: Entity, fields: readonly string[]): + {columns: Map}|{error: string} { + const columns = new Map(); + for (const field of fields) { + if (columns.has(field)) continue; + const column = columnFor(entity, field); + if ('error' in column) return column; + columns.set(field, column.column); + } + return {columns}; +} + + +/** The entity's key columns, in declared key order, resolved through fields. */ +export function keyColumns(entity: Entity): {columns: string[]}|{error: string} { + if (!entity.keys?.length) { + return { + error: `${entity.name} declares no key, so a violation could not be ` + + `attributed to a row`, + }; + } + const columns: string[] = []; + for (const key of entity.keys) { + const column = columnFor(entity, key); + if ('error' in column) return {error: `its key ${column.error}`}; + columns.push(column.column); + } + return {columns}; +} + + +/** The physical table `entity` is bound to, or why it has no usable one. */ +export function tableFor(entity: Entity): {table: string}|{error: string} { + const warnings: string[] = []; + const table = + spannerTable(entity.dataSource, warnings, `entity '${entity.name}'`); + if (warnings.length) { + return { + error: `'${entity.name}' has no usable table (${warnings.join('; ')})`, + }; + } + return {table}; +} + + +/** + * The rows of `entity` this call touches: one key column and the parameters + * that name values in it. + * + * An action names the rows it acts on through its entity-typed parameters, and + * that reference is what makes a probe cheap and its answer relevant. Without + * one, `amount <= Order.total` would be asked of every order in the table and + * fail on the first unrelated one, so a constraint over an entity the action + * does not take as a parameter is refused rather than widened into a table + * scan. Checking stored state at large is a different binding point -- a + * conformance sweep over the data rather than a gate on one call -- and it + * needs its own reference instead of this one silently standing in for it. + * + * EVERY parameter of that entity is in scope, not the first one found. + * `TransferFunds(source: Account, target: Account, amount)` writes both + * accounts, so a rule over `Account` that asked only about `source` would let + * the write that breaks `target` through while reporting the rule as checked. + * A gate that answers about some of the rows it was asked about is worse than + * one that refuses, because its answer is believed. + */ +export function rowsTouchedBy(action: Action, entity: Entity): + {key: string; parameters: string[]}|{error: string} { + const params = + action.parameters.filter(p => p.isEntityRef && p.type === entity.name); + if (!params.length) { + return { + error: `it reads ${entity.name}, and action '${action.name}' takes no ` + + `${entity.name} parameter, so the probe could not be limited to ` + + `the rows this call touches`, + }; + } + const keys = keyColumns(entity); + if ('error' in keys) return {error: keys.error}; + if (keys.columns.length !== 1) { + return { + error: `${entity.name} has a ${keys.columns.length}-part key, and the ` + + `runtime binds an object reference as a single value`, + }; + } + return {key: keys.columns[0], parameters: params.map(p => p.name)}; +} + + +// The physical column behind `fieldName`, or why there is none. A bare column +// is required: a field bound to an expression (`price * quantity`) would need +// that expression inlined and re-resolved, which this grammar does not do. +function columnFor(entity: Entity, fieldName: string): {column: string}| + {error: string} { + const field = entity.fields.find(f => f.name === fieldName); + if (!field) { + return {error: `${entity.name} declares no field '${fieldName}'`}; + } + // No binding is what unbound means: the profile in force bound nothing to + // this field, so there is no column to read the rule against. + const binding = (fieldBinding(field) ?? '').trim(); + if (!binding) { + return { + error: `${entity.name}.${fieldName} is unbound under this profile, so ` + + `there is nothing to read it from`, + }; + } + if (!/^[A-Za-z_]\w*$/.test(binding)) { + return { + error: `${entity.name}.${fieldName} is bound to an expression (${ + binding}) rather than to a column`, + }; + } + return {column: binding}; +} diff --git a/toolbox/mdcode/src/libts/semantic/runtime/constraints/check.ts b/toolbox/mdcode/src/libts/semantic/runtime/constraints/check.ts new file mode 100644 index 00000000..7da5952f --- /dev/null +++ b/toolbox/mdcode/src/libts/semantic/runtime/constraints/check.ts @@ -0,0 +1,213 @@ +// What a constraint check is, whatever settles it. +// +// A model states an invariant over the ontology. Enforcing one against a live +// deployment means turning that statement into something that can answer it, +// and more than one thing can: the store, asked a query, and a language model, +// asked to read the proposed change. What every answer has in common is here +// -- when the check is made, what a failure looks like, and what a failure +// does to the write -- so the runtime handles any of them the same way and no +// module has to know which others exist. + +import {Action, Constraint, SemanticModel, ViolationEffect} from '../../ir'; + +import {SqlDialect} from './dialect'; + + +// When a check runs, relative to the action's own writes. +// +// - `before` the write has not happened. The check reads the pre-state and +// the call's arguments, and a failure means the call is refused. +// - `after` the writes have run in the transaction but nothing is +// committed. The check reads the post-state, and a failure rolls +// it back. +export type CheckTiming = 'before'|'after'; + + +/** + * A query against the store, in whatever language the dialect emitted. + * + * `parameters` names the action parameters `text` reads, reported by the + * emitter rather than recovered from the text afterwards. An action's + * parameter list is wider than any single rule, a statement carrying a + * parameter it never reads is one the store may refuse, and only the emitter + * knows which sigil it wrote. + */ +export interface StoreQuery { + text: string; + parameters: string[]; +} + + +/** A constraint turned into something runnable, and when to run it. */ +export interface ConstraintCheck { + constraint: Constraint; + timing: CheckTiming; + // The entity the check reads, absent when the rule names none: a rule over + // the call's arguments alone, such as `amount <= 25`, reads no table. + entity?: string; + // Returns the rows that VIOLATE the constraint, so an empty result means the + // rule holds. Parameters are the action's own, bound by the caller at run + // time. + query: StoreQuery; + // The columns `query` returns, so a violation can name the rows it found + // rather than only reporting that one exists. + columns: string[]; +} + + +export type CheckPlan = { + ok: true; check: ConstraintCheck; +}|{ + ok: false; + // Why this constraint cannot be checked here, phrased for whoever wrote the + // model: the runtime surfaces it verbatim when it refuses the action. + reason: string; +}; + + +/** Everything a checker is handed. Each uses the part its own kind needs. */ +export interface CheckerContext { + model: SemanticModel; + action: Action; + constraint: Constraint; + // How a check that asks the store spells its query. A checker that asks + // something other than the store ignores it. + dialect: SqlDialect; +} + + +/** + * Plans one constraint as a gate on an action, or says why it cannot be. + * + * One implementation per way a constraint can be settled, and which one runs + * is read off the constraint's own body rather than chosen by a caller. See + * CONSTRAINT_EVALUATIONS in ir.ts and the registry in index.ts. + */ +export type ConstraintChecker = (ctx: CheckerContext) => CheckPlan; + + +/** A refusal, in the wording every checker gives one. */ +export function cannotCheck(constraint: Constraint, reason: string): CheckPlan { + return { + ok: false, + reason: `constraint '${constraint.name}' cannot be checked: ${reason}`, + }; +} + + +/** A statement as a store client takes it: the query and its bindings. */ +export interface StoreStatement { + sql: string; + params?: Record; + paramTypes?: Record; +} + + +/** + * `check` as a statement, carrying the argument values it reads. + * + * Filtered to the parameters the query names, which the emitter reported when + * it wrote them: a statement carrying one it never reads is a statement the + * store may refuse, and an action's parameter list is wider than any single + * rule. + */ +export function checkStatement( + check: ConstraintCheck, params: Record, + types: Record): StoreStatement { + const statement: StoreStatement = {sql: check.query.text}; + const used: Record = {}; + const usedTypes: Record = {}; + for (const name of new Set(check.query.parameters)) { + if (!(name in params)) continue; + used[name] = params[name]; + if (types[name]) usedTypes[name] = types[name]; + } + if (Object.keys(used).length) { + statement.params = used; + statement.paramTypes = usedTypes; + } + return statement; +} + + +// A rule that did not hold, in the terms a caller acts on. +export interface ConstraintViolation { + constraint: string; + effect: ViolationEffect; + // The constraint's own `description` where it has one, which is written as + // the instruction to the refused caller, followed by the citation. + message: string; + // The violating rows, each as its key values joined by '/'. Empty for a rule + // over the arguments alone, which has no row to name. + instances: string[]; +} + + +// A rule that was named as a guard and not evaluated. Only an advisory rule +// reaches this: one that stops the call and cannot be checked refuses the +// action instead. +export interface UncheckedRule { + constraint: string; + // Why it could not be checked, in the same words a refusal would have used. + reason: string; +} + + +/** + * A violation of `check`, given the rows it returned. + * + * The constraint's `description` leads, because it is the model author's own + * words about what the caller should do differently; the name and the body + * follow as the citation for it. The body cited is whichever one the + * constraint declares, so a rule settled in words cites the words. + */ +export function violationFrom( + check: ConstraintCheck, rows: string[][]): ConstraintViolation { + const constraint = check.constraint; + const lead = constraint.description?.trim() || + `Constraint '${constraint.name}' does not hold.`; + const body = constraint.expression ?? constraint.judgment; + const parts = [ + lead, + `Stopped by '${constraint.name}'${body ? ` (${body})` : ''}.`, + ]; + // Rows are named only when they identify something: the one-row result of a + // rule over the arguments alone says nothing a reader can use. + const instances = check.entity ? rows.map(row => row.join('/')) : []; + if (instances.length) { + parts.push(`Violating ${check.entity}: ${instances.join(', ')}.`); + } + return { + constraint: constraint.name, + effect: effectOf(constraint), + message: parts.join(' '), + instances, + }; +} + + +/** + * What a violated constraint does to the write. + * + * An `expression` that does not say defaults to `reject`, which is the safe + * reading of an author who did not say. See VIOLATION_EFFECTS in ir.ts. + */ +export function effectOf(constraint: Constraint): ViolationEffect { + return constraint.onViolation ?? 'reject'; +} + + +// Harshest first. A call that trips two rules gets the stricter answer: being +// told a supervisor could approve a write another rule forbids outright would +// send the caller to ask for something nobody can give. +const EFFECT_ORDER: ViolationEffect[] = ['reject', 'escalate', 'warn']; + + +/** The strictest effect among `violations`, or null if there are none. */ +export function strictestEffect(violations: readonly ConstraintViolation[]): + ViolationEffect|null { + for (const effect of EFFECT_ORDER) { + if (violations.some(v => v.effect === effect)) return effect; + } + return null; +} diff --git a/toolbox/mdcode/src/libts/semantic/runtime/constraints/dialect.ts b/toolbox/mdcode/src/libts/semantic/runtime/constraints/dialect.ts new file mode 100644 index 00000000..cbf3fe65 --- /dev/null +++ b/toolbox/mdcode/src/libts/semantic/runtime/constraints/dialect.ts @@ -0,0 +1,88 @@ +// How a check is spelled, for one query language. +// +// Binding decides WHICH table and column a rule reads; a dialect decides how +// to write that reference down and how to wrap it in a statement the store +// will run. They are apart because they vary apart: GoogleSQL and GQL read the +// same Spanner column of the same Spanner table and write the reference +// differently, while the same model deployed on another engine would bind +// different tables and be written much the same way. +// +// A dialect spells; it decides nothing. Whether a rule may be checked at all, +// which entity it reads and when it must run are settled in analysis.ts before +// any dialect is consulted, so adding one cannot widen or narrow what the +// runtime agrees to check. + +import {quoteIfReserved} from '../../sql_identifiers'; + + +/** A probe over the rows of one entity. */ +export interface EntityProbe { + // The physical table, resolved and already in the form the store addresses + // it by (see spannerTable in binding.ts, which quotes as it resolves). + table: string; + // Physical key columns, bare, in the entity's declared key order. + keys: string[]; + // Limits the probe to the rows this call touches. + scope: string; + // The constraint, rendered. True of a row that satisfies the rule. + predicate: string; + // How many violating rows to return. A gate needs enough to explain itself, + // not the whole violation set. + limit: number; +} + + +export interface SqlDialect { + // Named in a refusal, so a reader can tell which leg turned the rule away. + readonly name: string; + + /** A physical column of the probed entity, as written inside a predicate. */ + columnRef(column: string): string; + + /** A reference to one of the action's parameters. */ + parameterRef(name: string): string; + + /** A probe returning the rows of one entity that break the rule. */ + entityProbe(probe: EntityProbe): string; + + /** + * A probe over the call's arguments alone, which reads no table and returns + * one row when the rule is broken. + */ + argumentProbe(predicate: string): string; +} + + +// NOT COALESCE(p, FALSE) rather than a plain NOT: SQL's three-valued logic +// makes `NULL >= 0` unknown and `NOT unknown` unknown too, so a NULL column +// would slip past a plain negation. Reading unknown as "did not satisfy the +// rule" makes the row a violation, which is the fail-closed answer a gate +// owes. Shared rather than written per dialect because it is ordinary SQL and +// every dialect here inherits the same three-valued logic. +export function violating(predicate: string): string { + return `NOT COALESCE(${predicate}, FALSE)`; +} + + +/** + * GoogleSQL reading the entity's own table. + * + * The dialect every Spanner deployment can use, because it needs nothing + * deployed beyond the tables the action already writes to. + */ +export const GOOGLE_SQL: SqlDialect = { + name: 'GoogleSQL', + + columnRef: (column) => quoteIfReserved(column), + + parameterRef: (name) => `@${name}`, + + entityProbe: ({table, keys, scope, predicate, limit}) => + `SELECT ${keys.map(quoteIfReserved).join(', ')} FROM ${table} WHERE ${ + scope} AND ${violating(predicate)} LIMIT ${limit}`, + + // `UNNEST([1])` is the one-row source GoogleSQL needs for a SELECT that has + // a WHERE and nothing to select from. + argumentProbe: (predicate) => + `SELECT 1 AS violated FROM UNNEST([1]) WHERE ${violating(predicate)}`, +}; diff --git a/toolbox/mdcode/src/libts/semantic/runtime/constraints/expression.ts b/toolbox/mdcode/src/libts/semantic/runtime/constraints/expression.ts new file mode 100644 index 00000000..4b87304e --- /dev/null +++ b/toolbox/mdcode/src/libts/semantic/runtime/constraints/expression.ts @@ -0,0 +1,217 @@ +// The constraint expression grammar. +// +// A scanner over the small boolean language a guard is written in, producing +// the comparisons and the operators between them. It resolves no names, reads +// no binding and emits no SQL: what comes out says what the author wrote, and +// every later stage works from that rather than from the text. +// +// The grammar: +// +// expression := comparison (('AND' | 'OR') comparison)* +// comparison := operand operand +// operand := . | | literal +// op := >= | <= | != | <> | = | > | < +// literal := a number, a single-quoted string, TRUE, FALSE or NULL +// +// `= NULL` and `!= NULL` read as null tests and lower to IS NULL / IS NOT +// NULL. Parentheses, function calls, aggregates, IN, BETWEEN, LIKE and metric +// references are all outside the grammar, on purpose. Each is a real thing a +// constraint might want and each needs a decision this scanner does not make +// -- how an aggregate is evaluated inside a row-level probe, for one -- so +// each is refused with a reason rather than half-handled. +// +// Every scan here reads the text with its single-quoted spans masked. A +// literal that happens to spell `AND`, `==`, a bracket or an operator says +// nothing about the grammar, and refusing it would leave a reject-class guard +// permanently unrunnable. + +import {blankStringLiterals, STRING_LITERAL} from '../../sql_expr_utils'; + + +// The comparison operators the grammar accepts, longest first so `>=` is +// matched before `>`. +export const OPERATORS = ['>=', '<=', '!=', '<>', '=', '>', '<'] as const; + + +// An operand of a comparison: a field of an entity, an action parameter, or a +// literal already in SQL form. +export type Operand = { + kind: 'field'; entity: string; field: string; +}|{ + kind: 'parameter'; name: string; +}|{ + kind: 'literal'; text: string; +}; + + +export interface Comparison { + left: Operand; + right: Operand; + operator: string; +} + + +// Comparisons and the logical operators between them: `joiners[i]` sits +// between `comparisons[i]` and `comparisons[i + 1]`. +export interface ParsedExpression { + comparisons: Comparison[]; + joiners: string[]; +} + + +/** Parses a constraint expression, or says why it is outside the grammar. */ +export function parseExpression( + expression: string, + parameters: Map): ParsedExpression|{error: string} { + const text = expression.trim(); + if (!text) return {error: 'it declares no expression'}; + const bare = blankStringLiterals(text); + if (bare.includes('==')) { + return { + error: `it writes '==' (${text}); equality in the expression language ` + + `is a single '='`, + }; + } + if (/[()]/.test(bare)) { + return { + error: `it uses parentheses or a function call (${ + text}), which the grammar does not parse`, + }; + } + + const split = splitOnLogicalOperators(text); + const comparisons: Comparison[] = []; + for (const segment of split.parts) { + const comparison = parseComparison(segment, parameters); + if ('error' in comparison) return comparison; + comparisons.push(comparison); + } + return {comparisons, joiners: split.joiners}; +} + + +// Splits on top-level AND/OR, matched as whole words so a field named `brand` +// survives. There are no parentheses to nest -- parseExpression refuses them +// -- so every operator found is top level. +function splitOnLogicalOperators(expression: string): + {parts: string[]; joiners: string[]} { + const parts: string[] = []; + const joiners: string[] = []; + // Scan a copy with the literals masked, so `Account.status = 'ON HOLD OR + // CLOSED'` does not come apart inside the quotes. The mask is the same length + // as what it replaces, so every index still points into the original. + // + // `blankStringLiterals` fills with spaces, which is wrong here: a blanked + // literal would join the whitespace on either side of it, and `\s+AND\s+` + // would then match across the space the literal used to occupy. The filler + // has to be something `\s` does not match. + const masked = expression.replace(STRING_LITERAL, m => '.'.repeat(m.length)); + const pattern = /\s+(AND|OR)\s+/gi; + let last = 0; + let match: RegExpExecArray|null; + while ((match = pattern.exec(masked)) !== null) { + parts.push(expression.slice(last, match.index)); + joiners.push(match[1].toUpperCase()); + last = match.index + match[0].length; + } + parts.push(expression.slice(last)); + return {parts, joiners}; +} + + +function parseComparison( + segment: string, + parameters: Map): Comparison|{error: string} { + const text = segment.trim(); + const found = findOperator(text); + if (!found) { + return { + error: `'${text}' is not a comparison (expected one of ${ + OPERATORS.join(', ')})`, + }; + } + const leftText = text.slice(0, found.index).trim(); + const rightText = text.slice(found.index + found.operator.length).trim(); + if (!leftText) return {error: `'${text}' has nothing left of the operator`}; + if (!rightText) return {error: `'${text}' has nothing right of the operator`}; + + const left = parseOperand(leftText, parameters); + if ('error' in left) return {error: `in '${text}', ${left.error}`}; + const right = parseOperand(rightText, parameters); + if ('error' in right) return {error: `in '${text}', ${right.error}`}; + + const operator = found.operator === '<>' ? '!=' : found.operator; + + // GoogleSQL refuses `col = NULL` outright rather than evaluating it to + // unknown, so lowering it verbatim would emit a probe that cannot run. An + // author writing `Order.closedOn != NULL` means the column must be + // populated, which SQL spells IS NOT NULL -- so the two operators with a + // null-test reading are translated and the four without one are refused, an + // ordering comparison against NULL having no meaning to preserve. + const isNull = (operand: Operand) => + operand.kind === 'literal' && /^NULL$/i.test(operand.text); + if (isNull(left) || isNull(right)) { + if (operator !== '=' && operator !== '!=') { + return { + error: `'${text}' compares with NULL using '${operator}', which has ` + + `no meaning; write '= NULL' or '!= NULL' to ask whether the ` + + `field is set`, + }; + } + return { + left: isNull(left) ? right : left, + right: {kind: 'literal', text: 'NULL'}, + operator: operator === '=' ? 'IS' : 'IS NOT', + }; + } + + return {left, right, operator}; +} + + +function parseOperand( + text: string, + parameters: Map): Operand|{error: string} { + const field = text.match(/^([A-Za-z_]\w*)\.([A-Za-z_]\w*)$/); + if (field) return {kind: 'field', entity: field[1], field: field[2]}; + if (isLiteral(text)) return {kind: 'literal', text}; + if (/^[A-Za-z_]\w*$/.test(text)) { + if (parameters.has(text)) return {kind: 'parameter', name: text}; + return { + error: `'${text}' is not a parameter of this action; a field is ` + + `written .`, + }; + } + return {error: `'${text}' is not a field, a parameter or a literal`}; +} + + +// The first comparison operator in `text`, longest match first so `>=` is not +// read as `>` with a stray `=` after it. Read with the literals blanked, so an +// operator character inside a string -- `'a>b' = Order.tag` -- is not mistaken +// for the comparison. +function findOperator(text: string): {operator: string; index: number}|null { + let best: {operator: string; index: number}|null = null; + const masked = blankStringLiterals(text); + for (const operator of OPERATORS) { + const index = masked.indexOf(operator); + if (index < 0) continue; + if (!best || index < best.index || + (index === best.index && operator.length > best.operator.length)) { + best = {operator, index}; + } + } + return best; +} + + +// A literal the probe embeds verbatim, restricted to shapes with no quoting +// hazard: a number, a single-quoted string with no embedded quote or +// backslash, or one of the three keywords. Anything else is refused rather +// than escaped, because a constraint expression is model text and a surprising +// escape is harder to notice than a refusal. +function isLiteral(text: string): boolean { + if (/^[-+]?\d+(\.\d+)?$/.test(text)) return true; + if (/^'[^'\\]*'$/.test(text)) return true; + return /^(TRUE|FALSE|NULL)$/i.test(text); +} diff --git a/toolbox/mdcode/src/libts/semantic/runtime/constraints/index.ts b/toolbox/mdcode/src/libts/semantic/runtime/constraints/index.ts new file mode 100644 index 00000000..93d31083 --- /dev/null +++ b/toolbox/mdcode/src/libts/semantic/runtime/constraints/index.ts @@ -0,0 +1,142 @@ +// Checking an action's guards against a live deployment. +// +// A constraint is a logical invariant over the ontology (`Order.total >= 0`, +// `amount <= Order.total`). An action that names one in `guards` claims the +// rule holds of every call it makes, and this package is what makes the claim +// true: each named constraint becomes one check, run inside the action's own +// transaction, so a rule that does not hold rolls the write back and no +// violating state is ever visible to another reader. +// +// The package is laid out along the two ways a check varies, and they are +// independent: +// +// * WHAT SETTLES THE RULE. A constraint declares exactly one body and the +// body says who answers it. An `expression` is answered by the store +// (sql_check.ts); a `judgment` is answered by a language model reading the +// proposed change (judgment.ts). CHECKERS below maps one to the other, +// keyed by the IR's own CONSTRAINT_EVALUATIONS, so a third body cannot be +// added to the model without this map failing to compile. +// * HOW A STORE IS ASKED. One rule, one meaning, more than one language to +// write it in: GoogleSQL over the entity's own table today, GQL over the +// pushed property graph next, whatever a future engine reads after that. +// dialect.ts holds that axis alone. Nothing outside it -- not the grammar, +// not the timing, not the refusals -- changes when a dialect is added. +// +// Everything a check needs to be DECIDED is settled before a session opens and +// with no argument values in hand (analysis.ts, bind.ts), which is what lets +// one call answer both "may this action be offered at all" and "what shall I +// run". The advertised verdict and the real one cannot drift, because they are +// the same answer. +// +// The property that outranks expressive power: planning FAILS CLOSED. A +// constraint this package cannot turn into a check does not quietly pass. It +// returns a reason, and the runtime refuses the action rather than running it +// unchecked, because a gate that lets writes through is worse than no gate -- +// it is believed. + +import { + Action, + Constraint, + ConstraintEvaluation, + constraintEvaluation, + SemanticModel, +} from '../../ir'; + +import { + CheckPlan, + ConstraintChecker, + ConstraintCheck, + effectOf, + UncheckedRule, +} from './check'; +import {GOOGLE_SQL, SqlDialect} from './dialect'; +import {judgedCheck} from './judgment'; +import {sqlCheck} from './sql_check'; + + +// One checker per way a constraint can be settled. Total over the IR's own +// enumeration rather than a lookup with a fallback: a body the model can +// declare and this package has no answer for is a rule that would be silently +// skipped, and the compiler is a better place to find that out than a run. +const CHECKERS: Record = { + deterministic: sqlCheck, + judged: judgedCheck, +}; + + +/** + * Plans every constraint `action` names in `guards`. + * + * Returns the checks it could build, the reasons for the ones it could not, + * and the advisory rules it had to leave unevaluated. A non-empty `errors` + * means the action cannot be run: the model says the call is checked, and a + * check that cannot be performed is not one. + * + * An ADVISORY rule -- `on_violation: warn` -- is the exception, and it is + * `unchecked` rather than an error. It reports and lets the write through, so + * being unable to evaluate it costs the caller a report and stops nothing; + * refusing over it would turn a rule the author wrote as advice into the one + * thing that makes the action unrunnable. It is still named, because a report + * that was owed and not made is news in its own right. + */ +export function planGuards( + model: SemanticModel, action: Action, + dialect: SqlDialect = GOOGLE_SQL): { + checks: ConstraintCheck[]; + errors: string[]; + unchecked: UncheckedRule[]; +} { + const checks: ConstraintCheck[] = []; + const errors: string[] = []; + const unchecked: UncheckedRule[] = []; + for (const name of action.guards ?? []) { + const constraint = (model.constraints ?? []).find(c => c.name === name); + if (!constraint) { + // Not classifiable as advisory: the model declares nothing by this name, + // so there is no `on_violation` to read, and guessing is not on offer. + errors.push( + `action '${action.name}' is guarded by '${name}', which this model ` + + `does not declare`); + continue; + } + const planned = planGuard(model, action, constraint, dialect); + if (planned.ok) { + checks.push(planned.check); + } else if (effectOf(constraint) === 'warn') { + unchecked.push({constraint: name, reason: planned.reason}); + } else { + errors.push(planned.reason); + } + } + return {checks, errors, unchecked}; +} + + +/** Plans one constraint as a gate on `action`, or says why it cannot be. */ +export function planGuard( + model: SemanticModel, action: Action, constraint: Constraint, + dialect: SqlDialect = GOOGLE_SQL): CheckPlan { + return CHECKERS[constraintEvaluation(constraint)]( + {model, action, constraint, dialect}); +} + + +export { + checkStatement, + effectOf, + strictestEffect, + violationFrom, +} from './check'; +export type { + CheckerContext, + CheckPlan, + CheckTiming, + ConstraintCheck, + ConstraintChecker, + ConstraintViolation, + StoreQuery, + StoreStatement, + UncheckedRule, +} from './check'; +export {GOOGLE_SQL, violating} from './dialect'; +export type {EntityProbe, SqlDialect} from './dialect'; diff --git a/toolbox/mdcode/src/libts/semantic/runtime/constraints/judgment.ts b/toolbox/mdcode/src/libts/semantic/runtime/constraints/judgment.ts new file mode 100644 index 00000000..0c5cc6b5 --- /dev/null +++ b/toolbox/mdcode/src/libts/semantic/runtime/constraints/judgment.ts @@ -0,0 +1,29 @@ +// Checking a constraint that is settled by judgment. +// +// A `judgment` states the rule in words, for the rules no expression decides: +// *the credit memo must name a specific service failure* is a real requirement +// with a real owner, and no arithmetic settles it. The store cannot answer +// one, so no dialect helps; what answers it is a language model reading the +// proposed change against the rule's own text. +// +// This runtime calls no model, and the honest report of that is a refusal. The +// alternative is an action whose model says it is judged running unjudged, +// which is the failure the whole evaluator exists to prevent. A `warn` rule +// still passes, because index.ts reports an advisory rule it cannot check +// rather than refusing over it. +// +// A judge lands here and nowhere else. What it needs beyond this file is a +// model client threaded to the point of use, since a judgment is answered +// outside the store and therefore outside the action's transaction -- which +// makes WHEN it runs a decision of its own rather than one read off the +// expression, and is why the timing analysis is not shared with it. + +import {cannotCheck, CheckPlan, ConstraintChecker} from './check'; + + +/** The checker for a constraint stated as a judgment. */ +export const judgedCheck: ConstraintChecker = ({constraint}): CheckPlan => + cannotCheck( + constraint, + `it is settled by judgment rather than by an expression, and this ` + + `runtime runs no judge`); diff --git a/toolbox/mdcode/src/libts/semantic/runtime/constraints/sql_check.ts b/toolbox/mdcode/src/libts/semantic/runtime/constraints/sql_check.ts new file mode 100644 index 00000000..595186ee --- /dev/null +++ b/toolbox/mdcode/src/libts/semantic/runtime/constraints/sql_check.ts @@ -0,0 +1,151 @@ +// Checking a constraint by asking the store. +// +// Given a rule the analysis accepted, this builds the query returning the rows +// that BREAK it -- a probe. No rows means the rule holds. Which table and +// columns it reads comes from bind.ts, how the query is written comes from the +// dialect, and what the rule is allowed to say was settled in analysis.ts. +// What is left here is the wiring between them, and the refusals that arise +// only once a real table is involved. +// +// Two properties shape everything below: +// +// * The probe needs no argument VALUES. It is a query with the action's own +// parameters left unbound, so the call that decides whether an action is +// runnable at all -- asked before any argument arrives, to decide whether +// to offer an agent the tool -- produces the very query that later runs. +// One answer, so the advertised verdict and the real one cannot drift. +// * The probe is SCOPED to the rows the call touches. An action writes a +// handful of rows, and a gate whose cost grows with the table is a gate +// that gets switched off. + +import {analyze} from './analysis'; +import {columnsFor, keyColumns, rowsTouchedBy, tableFor} from './bind'; +import {cannotCheck, CheckPlan, ConstraintChecker} from './check'; +import {SqlDialect} from './dialect'; +import {Operand, ParsedExpression} from './expression'; + + +// How many violating rows a probe returns. A gate needs enough to explain +// itself, not the whole violation set. +const PROBE_LIMIT = 5; + + +/** + * The checker for a constraint stated as an expression. + * + * Every refusal it gives names something about this model, this action or this + * store; nothing here refuses on the grammar, which `analyze` has already + * passed on. + */ +export const sqlCheck: ConstraintChecker = + ({model, action, constraint, dialect}): CheckPlan => { + const fail = (reason: string) => cannotCheck(constraint, reason); + + const rule = analyze(action, constraint); + if ('error' in rule) return fail(rule.error); + + // A probe reads one table, so a rule naming two entities has no shape in + // this dialect. A dialect that traverses a relationship rather than + // scanning a table can answer it, and would lift this. + if (rule.entities.length > 1) { + return fail( + `it spans ${rule.entities.join(' and ')}; a probe reads ` + + `one entity's table, so write one constraint per entity and list ` + + `them together in 'guards'`); + } + + if (!rule.entities.length) { + // No table to read: the rule is entirely about the call's arguments. + const predicate = renderPredicate(rule.parsed, new Map(), dialect); + return { + ok: true, + check: { + constraint, + timing: rule.timing, + query: { + text: dialect.argumentProbe(predicate), + parameters: rule.parameters, + }, + columns: ['violated'], + }, + }; + } + + const entityName = rule.entities[0]; + const entity = (model.entities ?? []).find(e => e.name === entityName); + if (!entity) return fail(`'${entityName}' is not an entity of this model`); + if (entity.abstract) { + return fail(`'${entityName}' is abstract, so it has no table to read`); + } + + const bound = columnsFor(entity, rule.fields); + if ('error' in bound) return fail(bound.error); + + const touched = rowsTouchedBy(action, entity); + if ('error' in touched) return fail(touched.error); + + const keys = keyColumns(entity); + if ('error' in keys) return fail(keys.error); + + const table = tableFor(entity); + if ('error' in table) return fail(table.error); + + const predicate = renderPredicate(rule.parsed, bound.columns, dialect); + return { + ok: true, + check: { + constraint, + timing: rule.timing, + entity: entityName, + query: { + text: dialect.entityProbe({ + table: table.table, + keys: keys.columns, + scope: renderScope(touched, dialect), + predicate, + limit: PROBE_LIMIT, + }), + // The scope reads the entity-typed parameters whether or not the + // rule mentions them, so both sets are named. + parameters: [...new Set([...rule.parameters, ...touched.parameters])], + }, + columns: keys.columns, + }, + }; + }; + + +// Restricts the probe to the rows this call touches. See rowsTouchedBy. +function renderScope( + touched: {key: string; parameters: string[]}, dialect: SqlDialect): string { + const key = dialect.columnRef(touched.key); + const refs = touched.parameters.map(p => dialect.parameterRef(p)); + return refs.length === 1 ? `${key} = ${refs[0]}` : + `${key} IN (${refs.join(', ')})`; +} + + +// Renders the parsed expression against the physical columns. Each comparison +// is parenthesized, so a mixed AND/OR expression keeps the precedence the +// engine gives it rather than one this module invents. +function renderPredicate( + parsed: ParsedExpression, columns: Map, + dialect: SqlDialect): string { + const render = (operand: Operand): string => { + switch (operand.kind) { + case 'field': + return dialect.columnRef(columns.get(operand.field)!); + case 'parameter': + return dialect.parameterRef(operand.name); + case 'literal': + return operand.text; + } + }; + const parts = parsed.comparisons.map( + c => `(${render(c.left)} ${c.operator} ${render(c.right)})`); + let out = parts[0]; + for (let i = 1; i < parts.length; i++) { + out = `${out} ${parsed.joiners[i - 1]} ${parts[i]}`; + } + return out; +} diff --git a/toolbox/mdcode/src/libts/semantic/runtime/run_action.ts b/toolbox/mdcode/src/libts/semantic/runtime/run_action.ts index c52e47f8..18845e76 100644 --- a/toolbox/mdcode/src/libts/semantic/runtime/run_action.ts +++ b/toolbox/mdcode/src/libts/semantic/runtime/run_action.ts @@ -24,11 +24,11 @@ // the caller supplies a handler that produces the statements. // // Where the model's rules come in. A constraint takes effect here through -// `guards` on the action: each rule the action names is lowered to a probe and +// `guards` on the action: each rule the action names becomes one check and is // run inside this same transaction -- before the write when it reads one of the // call's arguments, after the write when it reads only stored state -- and a // violation rolls the whole thing back and reports the rule's own words. See -// constraint_eval.ts. +// ./constraints. // // A rule that cannot be lowered REFUSES the action rather than letting it run // unchecked. A model that declares a rule and a runtime that quietly ignores it @@ -56,15 +56,15 @@ import { import {quoteIfReserved, referencedParameters} from '../sql_identifiers'; import { + CheckTiming, + checkStatement, ConstraintViolation, effectOf, - lowerGuards, - UncheckedRule, - probeStatement, - ProbeTiming, + planGuards, strictestEffect, + UncheckedRule, violationFrom, -} from './constraint_eval'; +} from './constraints'; import {runtimeClient, SemanticRuntime} from './runtime'; @@ -179,14 +179,14 @@ export async function runAction(opts: RunActionOptions): const refusal = whyRefusedWithoutRunning(model, action, opts.handler); if (refusal) return {status: 'error', message: refusal}; - // Lowered before anything opens, and by the same call the refusal check just - // made: the probes that run are the ones it proved buildable, so an action + // Planned before anything opens, and by the same call the refusal check just + // made: the checks that run are the ones it proved buildable, so an action // reported as runnable cannot then meet a rule that turns out to be // uncheckable. - const lowered = lowerGuards(model, action); - const probes = lowered.probes; - // Grows during the run: a probe the store refuses joins the rules that could - // not be lowered in the first place, since both leave a rule the model named + const lowered = planGuards(model, action); + const checks = lowered.checks; + // Grows during the run: a check the store refuses joins the rules that could + // not be planned in the first place, since both leave a rule the model named // unevaluated and both are worth reporting under the same heading. const unchecked: UncheckedRule[] = [...lowered.unchecked]; @@ -256,41 +256,41 @@ export async function runAction(opts: RunActionOptions): } const refs = resolved.refs; - // A probe binds the action's own parameters, so a guarded action is + // A check binds the action's own parameters, so a guarded action is // bound here even when a handler is what supplies the writes. let probeValues: Bindings|undefined; - if (probes.length) { + if (checks.length) { const bound = bindArguments(model, action, args, refs); if ('error' in bound) { return await rollback({status: 'error', message: bound.error}); } probeValues = bound; } - // A probe that the store refuses is the same situation as a rule that - // could not be lowered, and it is answered the same way: an advisory + // A check the store refuses is the same situation as a rule that + // could not be planned, and it is answered the same way: an advisory // rule is reported as unchecked and the write goes on, anything - // stricter stops the call. Letting a `warn` probe's StoreError escape + // stricter stops the call. Letting a `warn` check's StoreError escape // would roll the transaction back over a rule whose whole contract is - // that it stops nothing -- the carve-off `lowerGuards` makes, undone + // that it stops nothing -- the carve-off `planGuards` makes, undone // one layer down. - const check = async (timing: ProbeTiming) => { + const violationsAt = async (timing: CheckTiming) => { const violations: ConstraintViolation[] = []; - for (const probe of probes) { - if (probe.timing !== timing) continue; + for (const check of checks) { + if (check.timing !== timing) continue; let rows; try { - rows = await query(probeStatement( - probe, probeValues!.params, probeValues!.types)); + rows = await query(checkStatement( + check, probeValues!.params, probeValues!.types)); } catch (err) { - if (effectOf(probe.constraint) !== 'warn') throw err; + if (effectOf(check.constraint) !== 'warn') throw err; unchecked.push({ - constraint: probe.constraint.name, + constraint: check.constraint.name, reason: `its probe could not be run (${ err instanceof Error ? err.message : String(err)})`, }); continue; } - if (rows.length) violations.push(violationFrom(probe, rows)); + if (rows.length) violations.push(violationFrom(check, rows)); } return violations; }; @@ -298,7 +298,7 @@ export async function runAction(opts: RunActionOptions): // Before the write, because a rule that reads an argument is asking // whether this call may proceed at all, and a call that may not should // cost the store no writes. - const beforeWrite = await check('before'); + const beforeWrite = await violationsAt('before'); const refusedBefore = refusedBy(action, beforeWrite, refs); if (refusedBefore) return await rollback(refusedBefore); @@ -325,7 +325,7 @@ export async function runAction(opts: RunActionOptions): // After the write and still inside the transaction, which is the one // moment the post-state both exists and can still be undone. - const afterWrite = await check('after'); + const afterWrite = await violationsAt('after'); const refusedAfter = refusedBy(action, afterWrite, refs); if (refusedAfter) return await rollback(refusedAfter); const warnings = @@ -541,7 +541,7 @@ function unbindableByThisRuntime( // write safe; it is the model saying no rule gates the call. What the write // does is the author's, which is what `affects` describes. function guardsNotCheckable(model: SemanticModel, action: Action): string|null { - const errors = lowerGuards(model, action).errors; + const errors = planGuards(model, action).errors; if (!errors.length) return null; return `Action '${action.name}' cannot be run: ${errors.join('; ')}. ` + `Running it would apply a write the model says is checked first, so ` + diff --git a/toolbox/mdcode/tests/libts/semantic/runtime/constraint_eval.test.ts b/toolbox/mdcode/tests/libts/semantic/runtime/constraints.test.ts similarity index 82% rename from toolbox/mdcode/tests/libts/semantic/runtime/constraint_eval.test.ts rename to toolbox/mdcode/tests/libts/semantic/runtime/constraints.test.ts index 31d36616..7157e527 100644 --- a/toolbox/mdcode/tests/libts/semantic/runtime/constraint_eval.test.ts +++ b/toolbox/mdcode/tests/libts/semantic/runtime/constraints.test.ts @@ -15,15 +15,18 @@ import {describe, expect, test} from 'bun:test'; import {Action, Constraint, Entity, SemanticModel} from '../../../../src/libts/semantic/ir'; import { - ConstraintProbe, + CheckPlan, + checkStatement, + ConstraintCheck, effectOf, - lowerGuard, - lowerGuards, - Lowering, - probeStatement, + GOOGLE_SQL, + planGuard, + planGuards, + SqlDialect, strictestEffect, + violating, violationFrom, -} from '../../../../src/libts/semantic/runtime/constraint_eval'; +} from '../../../../src/libts/semantic/runtime/constraints'; const ORDER: Entity = { @@ -119,21 +122,25 @@ function modelWith(over: Partial = {}): SemanticModel { function lowerRule( constraint: Constraint, action: Action = ISSUE_CREDIT, - model: SemanticModel = modelWith()): Lowering { - return lowerGuard(model, action, constraint); + model: SemanticModel = modelWith()): CheckPlan { + return planGuard(model, action, constraint); } -function lower(expression: string, action?: Action): Lowering { +function lower(expression: string, action?: Action): CheckPlan { return lowerRule({name: 'Rule', expression}, action); } -function probeOf(lowered: Lowering): ConstraintProbe { +// The probe's query text is read often enough here to be worth flattening onto +// the check, so an assertion about the SQL reads as one. +function probeOf(lowered: CheckPlan): ConstraintCheck&{sql: string} { if (!lowered.ok) throw new Error(`expected a probe: ${lowered.reason}`); - return lowered.probe; + return {...lowered.check, sql: lowered.check.query.text}; } -function reasonOf(lowered: Lowering): string { - if (lowered.ok) throw new Error(`expected a refusal: ${lowered.probe.sql}`); +function reasonOf(lowered: CheckPlan): string { + if (lowered.ok) { + throw new Error(`expected a refusal: ${lowered.check.query.text}`); + } return lowered.reason; } @@ -155,7 +162,7 @@ describe('what the probe reads and when it runs', () => { to: {code: 'INT64'}, amount: {code: 'NUMERIC'}, }; - const statement = probeStatement( + const statement = checkStatement( probeOf(lower('Order.total >= 0', MOVE_CREDIT)), params, types); expect(statement.params).toEqual({from: 1, to: 2}); }); @@ -411,7 +418,7 @@ describe('lowering the guards of one action', () => { }; const guardedBy = (constraints: Constraint[], guards: string[]) => - lowerGuards( + planGuards( modelWith({constraints}), {...ISSUE_CREDIT, guards}); test('a guard naming a constraint the model does not declare is an error', @@ -423,8 +430,8 @@ describe('lowering the guards of one action', () => { }); test('a guard it cannot check stops the action', () => { - const {probes, errors} = guardedBy([judged], ['Justified']); - expect(probes).toEqual([]); + const {checks, errors} = guardedBy([judged], ['Justified']); + expect(checks).toEqual([]); expect(errors).toHaveLength(1); }); @@ -441,15 +448,15 @@ describe('lowering the guards of one action', () => { }); test('the checkable guards are still lowered alongside the rest', () => { - const {probes, errors} = + const {checks, errors} = guardedBy([positive, judged], ['Positive', 'Justified']); - expect(probes.map(p => p.constraint.name)).toEqual(['Positive']); + expect(checks.map(c => c.constraint.name)).toEqual(['Positive']); expect(errors).toHaveLength(1); }); test('an action naming no guard produces nothing to run', () => { - const {probes, errors, unchecked} = guardedBy([positive], []); - expect(probes).toEqual([]); + const {checks, errors, unchecked} = guardedBy([positive], []); + expect(checks).toEqual([]); expect(errors).toEqual([]); expect(unchecked).toEqual([]); }); @@ -468,23 +475,26 @@ describe('binding a probe to the call', () => { // An action's parameter list is wider than any one rule, and a statement // carrying a parameter it never reads is one the store may refuse. const statement = - probeStatement(probeOf(lower('amount <= 25')), params, types); + checkStatement(probeOf(lower('amount <= 25')), params, types); expect(statement.params).toEqual({amount: 30}); expect(statement.paramTypes).toEqual({amount: {code: 'NUMERIC'}}); }); test('carries the scope parameter too, when the probe reads a table', () => { const statement = - probeStatement(probeOf(lower('Order.total >= 0')), params, types); + checkStatement(probeOf(lower('Order.total >= 0')), params, types); expect(statement.params).toEqual({order: '12345'}); }); test('a probe naming no parameter carries none at all', () => { - const statement = probeStatement( + const statement = checkStatement( { constraint: {name: 'Rule', expression: 'TRUE = TRUE'}, timing: 'before', - sql: 'SELECT 1 AS violated FROM UNNEST([1]) WHERE FALSE', + query: { + text: 'SELECT 1 AS violated FROM UNNEST([1]) WHERE FALSE', + parameters: [], + }, columns: ['violated'], }, params, types); @@ -493,6 +503,66 @@ describe('binding a probe to the call', () => { }); +describe('what the dialect decides, and what it does not', () => { + // A second dialect, standing in for one that reads the pushed property graph + // rather than the table: the same column of the same table, reached through + // a pattern variable instead of named bare. + const THROUGH_A_VARIABLE: SqlDialect = { + name: 'test', + columnRef: (column) => `o.${column}`, + parameterRef: (name) => `@${name}`, + entityProbe: ({table, keys, scope, predicate, limit}) => + `MATCH (o:${table}) WHERE ${scope} AND ${violating(predicate)} ` + + `RETURN ${keys.map(k => `o.${k}`).join(', ')} LIMIT ${limit}`, + argumentProbe: (predicate) => + `RETURN 1 AS violated WHERE ${violating(predicate)}`, + }; + + const inDialect = (expression: string, dialect: SqlDialect) => planGuard( + modelWith(), ISSUE_CREDIT, {name: 'Rule', expression}, dialect); + + test('the same rule is written differently and resolved the same', () => { + // Binding is what the two share: both read column Total of table Orders, + // and only the way the reference is written down differs. + expect(probeOf(inDialect('Order.total >= 0', GOOGLE_SQL)).sql) + .toBe( + 'SELECT OrderId FROM Orders WHERE OrderId = @order AND ' + + 'NOT COALESCE((Total >= 0), FALSE) LIMIT 5'); + expect(probeOf(inDialect('Order.total >= 0', THROUGH_A_VARIABLE)).sql) + .toBe( + 'MATCH (o:Orders) WHERE o.OrderId = @order AND ' + + 'NOT COALESCE((o.Total >= 0), FALSE) RETURN o.OrderId LIMIT 5'); + }); + + test('everything but the writing is settled before a dialect is asked', () => { + // The grammar, the entity a rule reads and the moment it runs are decided + // in the analysis, so a dialect can neither widen what the runtime agrees + // to check nor move when it checks it. + for (const dialect of [GOOGLE_SQL, THROUGH_A_VARIABLE]) { + expect(reasonOf(inDialect('SUM(Order.total) > 0', dialect))) + .toContain('parentheses or a function call'); + expect(reasonOf(inDialect('Order.total >= 0 AND amount > 0', dialect))) + .toContain('about stored data'); + expect(probeOf(inDialect('Order.total >= 0', dialect)).timing) + .toBe('after'); + expect(probeOf(inDialect('amount <= Order.total', dialect)).timing) + .toBe('before'); + } + }); + + test('a check reports the parameters it wrote, whatever it wrote them as', + () => { + // The emitter names them rather than the caller scanning the text + // back out of it, so a dialect with another sigil binds correctly + // without anyone teaching the binder about it. + const check = probeOf(inDialect('amount <= Order.total', GOOGLE_SQL)); + expect([...check.query.parameters].sort()).toEqual([ + 'amount', 'order' + ]); + }); +}); + + describe('reporting a violation', () => { test("the author's own words lead, and the citation follows", () => { // The description is written as the instruction to the caller who was