diff --git a/CHANGELOG.md b/CHANGELOG.md index 86fcac26..85540e5a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,62 @@ ## Unreleased +- **The questionnaire asks the unread questions first.** The declaration + questionnaire promised an order — "by how much answering can move the + verdict" — and delivered the opposite of it. It ranked each question by the + effect the scan had *already inferred* for the action, and a pre-filled + proposal is offered on exactly the same condition, so the two mechanisms ran + off one signal: **every question that arrived with a draft answer outranked + every question that arrived blank.** On the fifth `adk-samples#1745` walk + that put three already-drafted mail tools at Q2–Q4 and the financial write — + the single question that produces both `critical` blockers once answered — at + Q6, behind three drafts a reader had to confirm first + ([#419](https://github.com/ThreeMoonsLab/agents-shipgate/issues/419)). + + *Rank by the ceiling, not by the floor.* Observed risk and "how much can + answering this move the verdict" are not the same quantity, and the header + claimed the second. An action nothing has bounded is not a low-risk action; + it is an unmeasured one, its answer can still turn out to be `destructive`, + and it is exactly where a human answer carries new information. A question + about an unbounded action now sorts above every bounded one, and the bounded + ones keep their old order among themselves — strongest first. On the same + walk the financial write moves from Q6 to Q3 and all three drafts move to the + end. + + *Bounded is not the same test as draftable.* A reviewed declaration and + policy-eligible source evidence bound an action even when what they establish + is `read`, and the rule that decides whether to pre-fill a value cannot say + so: it refuses to draft `effect: read` from anything, because a confirmed + guess of `read` is the one direction that loses safety. Ranking on that rule + would send an OpenAPI `GET` named `delete_account` to the top of the file + with its name breaking the tie — the same defect inverted — so ordering asks + its own question. A heuristic reading of `read` still bounds nothing: this + resolver may not act on it, so the answer remains open. + + *And a name breaks the tie among blanks.* Where nothing was observed there is + nothing to rank by, so the questionnaire falls back to the shape of the + action's name — mutating, neutral, retrieving — using the keyword vocabulary + the scanner already owns. This needs no trust and is given none: it is + consulted only among actions the scan measured nothing about, it cannot + reorder an action the scan did read, and it never reaches a claim, an issue, + or a verdict. Getting it wrong costs a reader one place in a list they have + to finish either way. + + The header sentence now states the order the file actually uses — including + the heuristic-read case, where a block prints a reading and is still + unbounded — and a test renders the file and checks the two against each + other. A blank with no reading at all now says so at the block, since the + header explains that the top of the file is the unbounded half and silence + read as "nothing to see here". + + *Published contract.* `report.json` / + `semantic_coverage.declaration_questions.open_questions[]` documented itself + as "highest-acting action first", which is no longer what it is. The model + docstring is emitted verbatim into the report, packet, and verifier schemas, + so it and `docs/agent-contract-current.md` now describe the ranking above and + say plainly that position is not severity: the action at the top is the one + *least* is known about. Field shapes are unchanged. + - **One action, one permission list, with no reviewed authority either.** A manifest row that listed `scopes:` and declared no `authority:` block at either site turned `verify --base` into `Internal error` (exit 4) on a legal @@ -169,9 +225,9 @@ would let the scanner establish what only a human may (#357, #268). *The file is numbered and counted.* Blocks carry `Question 3 of 5` banners - ordered by how much answering them can move the verdict — money, outward - communication, and destruction first, which is what reached a verdict in two - answers on the walk — and both the file header and the CLI print + ordered by how much answering them can move the verdict — two answers were + enough to reach one on the walk, and the entry above says which two the + order now leads with — and both the file header and the CLI print `Declaration questions: 1 of 2 answered; 1 open (1 authority).` from one rendering, so they cannot describe the same state two ways. An open question with no blank to fill (a conflict whose repair is in the source) is still diff --git a/docs/agent-contract-current.md b/docs/agent-contract-current.md index 6d6d4f3b..7e086da4 100644 --- a/docs/agent-contract-current.md +++ b/docs/agent-contract-current.md @@ -415,7 +415,7 @@ In `agents-shipgate-reports/report.json`: - `release_decision.{blockers,review_items}[].capability_refs` (v0.24+) — stable capability IDs copied from the originating finding when a policy or policy-pack rule matched a `CapabilityFactV1`. Empty for findings that are not capability-policy matches. This is audit metadata only; `release_decision.decision` remains the gate. - `release_decision.{blockers,review_items}[].capability_trace_refs` (v0.25+) — stable local trace-evidence IDs copied from the originating finding when an existing trace/evidence check used declared local trace artifacts. Empty when no local trace row is relevant. This is audit metadata only; `release_decision.decision` remains the gate. - `release_decision.evidence_coverage.semantic_coverage` (v0.29+) — `{total_actions, pass_eligible_actions, gap_count, review_concern_count, reason_counts}`. A non-zero semantic `gap_count` prevents `passed`; a non-zero `review_concern_count` prevents an automatic pass and routes known review concerns to human review — unscoped/ambient authority, and (v0.36+) `acknowledged_effect_override`, a declared effect a reviewer acknowledged as weaker than the evidence inferred for it. Read `reason_counts` for which; the count is of concerns, so one action can contribute more than one. Semantic gaps are not Findings and cannot be suppressed, baselined, severity-overridden, waived by `--no-heuristics`, or satisfied by `human_ack`. -- `release_decision.evidence_coverage.semantic_coverage.declaration_questions` (v0.37+) — the same action surface counted as a questionnaire: `{total, answered, open, open_by_dimension, open_questions[]}`. A *question* is one `(action, dimension)` a reviewed `action_surface.actions` row has to answer, and only `effect` and `authority` are counted — an action whose effect the scan established by itself (an OpenAPI method, an MCP annotation) was never asked and is not in `total`, and an inventory or `agent_bindings` declaration has no per-action counterfactual to score against. `answered` is exact rather than optimistic: it counts dimensions that gap when the same action is re-resolved *without* its declaration. `total == answered + open`, and `open_by_dimension` sums to `open`. `open_questions[]` is the answer order — highest-acting action first, `effect` before `authority` — and joins to `evidence_gaps[].subject_id`. Nothing here gates; it is a projection of counts the decision already made, published so a coding agent (and the generated questionnaire) can report progress instead of a gap tally. +- `release_decision.evidence_coverage.semantic_coverage.declaration_questions` (v0.37+) — the same action surface counted as a questionnaire: `{total, answered, open, open_by_dimension, open_questions[]}`. A *question* is one `(action, dimension)` a reviewed `action_surface.actions` row has to answer, and only `effect` and `authority` are counted — an action whose effect the scan established by itself (an OpenAPI method, an MCP annotation) was never asked and is not in `total`, and an inventory or `agent_bindings` declaration has no per-action counterfactual to score against. `answered` is exact rather than optimistic: it counts dimensions that gap when the same action is re-resolved *without* its declaration. `total == answered + open`, and `open_by_dimension` sums to `open`. `open_questions[]` is the answer order and joins to `evidence_gaps[].subject_id`. v0.38 ranks it by the ceiling of what an answer can establish: the actions nothing has bounded first — no effect evidence, a protocol default standing in for its absence, or only a heuristic reading the scan may not act on — then the actions a reviewed declaration or policy-eligible source evidence established, strongest-acting first, with `effect` before `authority` within one action. Position is not severity: the action at the top is the one *least* is known about. Nothing here gates; it is a projection of counts the decision already made, published so a coding agent (and the generated questionnaire) can report progress instead of a gap tally. - `release_decision.evidence_coverage.policy_gap_count` and top-level `policy_evidence_gaps[]` (v0.33+) — policy applicability that is heuristic-only, mixed, unknown, or conflicting. These rows are outside Findings and cannot be suppressed, baselined, severity-overridden, acknowledged, or removed by `--no-heuristics`; any row prevents `passed`. - `release_decision.evidence_coverage.identity_coverage` (v0.30+) — `{total_observations, canonical_tools, bound_tools, pass_eligible_tools, ambiguous_name_count, gap_count, reason_counts}`. Provider-scoped observations remain separate unless an exact reviewed `tool_identity.bindings[]` entry joins them. Any ambiguous selector, invalid binding, or conflicting identity prevents `passed`. - `release_decision.evidence_coverage.evidence_gaps[]` (v0.26+; semantic kinds added v0.29) — one structured row per measurable gap: `{kind, subject, source_type, source_ref, why, next_action}`. In addition to `low_confidence_tool` and `source_warning`, v0.29 adds `incomplete_surface`, `missing_effect_evidence`, `inferred_effect_only`, `conflicting_effect_evidence`, `missing_authority_evidence`, `partial_authority_evidence`, `conflicting_authority_evidence`, and `invalid_semantic_annotation`. v0.36 adds `declaration_below_inferred_evidence`: the declared effect is weaker than evidence this scan inferred for the same action. The declaration still stands as the operative effect — heuristics never drive a verdict — but the action is not evidence-backed-pass until a reviewer raises the declared effect or adds `action_surface.actions[].override` with the `evidence` they checked and the `reason` it does not apply. An acknowledged override keeps the action pass-eligible and is reported as one semantic review concern, so the run can never read `passed`. Each acknowledgement is also emitted as a row in `release_decision.evidence_coverage.semantic_coverage.acknowledged_overrides[]` (v0.36+) naming the action, both readings, the hint source, any source evidence that agrees, and the reviewer's evidence and reason — the packet's §1 and the PR comment render it, because a count is not a review surface. The acknowledgement is consumed by policy applicability as well, so applying it reaches the review route rather than trading one gap for another. Semantic next actions use `declare_action_effect`, `declare_action_authority`, `declare_tool_inventory`, `provide_complete_inventory`, or `resolve_semantic_conflict`, include accepted values and exact source/manifest pointers, and are always human-routed. v0.37 adds `next_action.observed_readings[]` on effect rows — `{effect, sources[], observed}`, the distinct readings this scan's non-declaration evidence supports, so the row can be answered without opening `action_surface_facts`. Where those readings support one conservative answer, `next_action.declaration_template` carries it **pre-filled** instead of a `` blank. That value is a proposal, not an assertion: it is drawn from the closed `ActionEffect` vocabulary (never from source content), it is never weaker than any reading, and it is offered only where something was observed — a protocol default standing in for the absence of evidence, or a heuristic reading of `read`, keeps the blank. Their declaration placeholders carry `suggested_patch_kind="manual"`, `auto_apply=false`, and `requires_human_review=true`; they are not Patch objects, and a pre-filled template is still operative only once a human merges it into the manifest. v0.37 also re-routes `partial_authority_evidence`: it is raised when the *source's* authority evidence is ambiguous or incomplete, and the resolver preserves it whatever the manifest declares ("reviewed authority cannot replace ambiguous or incomplete source authority alternatives"). Its `next_action.kind` is therefore `provide_source` with no declaration template, rather than a `declare_action_authority` block that could not close the row it was printed on. It is excluded from `declaration_questions` for the same reason. Work the rows in order instead of guessing; Agents Shipgate never auto-asserts effect or authority. diff --git a/docs/engineering/insufficient-evidence-cold-start.md b/docs/engineering/insufficient-evidence-cold-start.md index 00ca6d75..c42ef793 100644 --- a/docs/engineering/insufficient-evidence-cold-start.md +++ b/docs/engineering/insufficient-evidence-cold-start.md @@ -536,3 +536,146 @@ deliberately outside the denominator — there is no counterfactual for them, because removing an inventory does not re-run extraction. Counting what can only be measured on one side is how a progress bar starts lying, and a lying finish line is worse than none. + +## Fourth round (2026-08-25): the order was ranking the wrong quantity + +The questionnaire told the reader what its order meant — "ordered by how much +answering can move the verdict, so the first questions are the ones about +money, outward communication, and destruction" — and then delivered the inverse +of it. + +The fifth `adk-samples#1745` walk asked 13 questions. Three already-drafted mail +tools sat at Q2–Q4. `create_sap_sales_order`, the one question that produces +both `critical` blockers the moment it is answered `financial_write`, sat at +**Q6**, behind three drafts a reader working top to bottom confirms first. + +### One signal, two mechanisms, opposite directions + +Each question was ranked by `effect_evidence_rank(conservative_effect)` — the +effect the scan had already inferred for the action. A pre-filled proposal is +offered *only where something was observed*, which is the right rule for a +proposal and was settled in the round above. But the two mechanisms then ran +off one signal, and nothing said so: + +| action | evidence | old rank | proposal | +|---|---|---|---| +| `send_email` | `risk_hint:keyword` → external_communication | 6 | offered | +| `create_sap_sales_order` | none | 1 (the `write` fallback) | blank | + +So the questions that arrived with a draft answer systematically outranked the +questions that arrived blank: the cheapest questions first, the most valuable +ones last, and a header sentence promising the reverse. + +The ranking was faithful to observed risk. **Observed risk is not the quantity +the header names.** An action nothing was observed about is not a low-risk +action; it is an unmeasured one. Its answer can still turn out to be +`destructive`, and it is exactly where a human answer carries new information. + +### Rank by the ceiling + +A question is now ranked by the *ceiling* of what its answer can establish +rather than by the floor the scan already inferred. Where something already +bounds the effect, the answer cannot go below it and the old rank still +applies. Where nothing does, there is no bound at all, so the question sorts +above every bounded one (`UNBOUNDED_EFFECT_RANK`). On the same walk the +financial write moves Q6 → Q3, and all three drafts move to the end. + +### "Bounded" is not "draftable", and the difference is a whole class of bug + +The first draft of this used `effect_is_measured` — the gate +`propose_effect_declaration` already applies — on the reasoning that "measured" +and "could be drafted" are one fact. They are, and that is exactly why it was +the wrong predicate to rank with. + +`effect_is_measured` is a **proposal-safety** rule. It returns `False` for every +read-only reading, however authoritative, because a pre-filled `effect: read` +is the one direction where a reviewer confirming without thinking loses safety. +An OpenAPI `GET`, an MCP `readOnlyHint: true` the manifest trusts, and a +reviewed `effect: read` are all *proven* reads — and all three read as "nothing +to propose". + +Ranked with it, they went to the **ceiling**, and the name band then broke the +tie among them. A structural `GET` named `delete_account` led the questionnaire, +ahead of a genuinely unknown effect — this round's own defect inverted, with a +repository-chosen name ordering something the scan had established. Found in +review, not by the tests, because every fixture had been built from the walk, +where nothing is a proven read. + +`effect_is_bounded` is the ordering question, and it is two clauses because the +resolver records the two cases differently: + +- an effect status of `declared`, `structural`, or `conflicting` — the resolver + established it (or established that its sources disagree, which is still a + bound). This clause is what catches a reviewed `effect: read`, which leaves + *no reading at all* behind: declaration claims are excluded from readings on + purpose, because a row is not evidence about itself; +- otherwise, an observed side effect — the `inferred` action whose keyword hint + reads `external_communication`. + +A heuristic reading of `read` is deliberately in neither: this resolver refuses +to establish a read-only action from a heuristic, so such an action is exactly +as unproven as one with no reading at all. It is the one tier member that +*prints* a reading above its block, which is why the header describes the tier +as what nothing has pinned down rather than as what the scan "read nothing +about". + +### A name may order what it may not judge + +Within the unmeasured group nothing was observed, so alphabetical order is no +order at all — and that group is the whole questionnaire on a repository that +has just adopted. The tiebreaker is the shape of the action's *name*, in three +coarse bands (mutating, neutral, retrieving), read with the keyword vocabulary +the scanner already owns rather than a second one. + +The scanner deliberately does **not** consult those keywords for evidence on +most source types: a Python function called `create_sap_sales_order` is not +proof that it writes anything, and `google_adk` is not in +`_KEYWORD_GATED_SOURCE_TYPES` for precisely that reason. Ordering needs no such +trust, and is given none: + +- it is consulted only among actions the scan measured nothing about — never to + reorder an action it did read; +- the band is inert (`0`) for every measured action, so it cannot cross the + boundary even by accident; +- a source-level test pins the call sites, because the failure worth guarding + against is a *future* one: the moment a name-shaped reading is consulted + where a claim, an issue, or a verdict can see it, an unreviewed reading of a + repository-chosen string has become evidence. + +The `delete_account` GET above is what that second point is worth. The band was +correctly scoped to one tier the whole time; what put a name in charge of a +proven action was the *tier* being wrong, one predicate away. A guard on the +band alone would not have caught it. + +Getting a band wrong costs a reader one place in a list they must finish +either way. + +### What "never precedes" can and cannot mean + +The obvious statement of the fix — *a question carrying a proposal never +precedes a blank one* — is not quite the property to enforce, and enforcing it +literally would break two things worth more. + +An action's own `authority` question always arrives blank, and it follows that +action's `effect` question by design: they are one `action_surface.actions` row +and one block in the generated file. Sorting every blank first would separate +them, and the block that answers both would be announced as "Questions 1 and 7" +with no block numbered 7 anywhere in the flow. + +The property that *is* enforced is the one the mechanism actually has: **every +question the scan could draft an answer for sorts after every question about an +action it measured nothing about.** Since drafting and measuring are one +condition, that is the same statement with the ambiguity removed — and it is +what the header now says. + +**The rule this adds:** *rank by what an answer can still establish, not by what +you already inferred.* Where those two differ, the second one ranks a +questionnaire by how much it already knows, which is the reverse of what the +reader is there to do. + +**And the one it adds twice:** *a safety rule is not a general-purpose +predicate.* `effect_is_measured` is exactly right for deciding what may be +pre-filled and exactly wrong for deciding what is known, and the two read as +the same sentence right up until an OpenAPI `GET` sorts above an unknown. When +a predicate's name describes its answer rather than its question, borrowing it +is how the borrower inherits a rule it never wanted. diff --git a/docs/packet-schema.v0.15.json b/docs/packet-schema.v0.15.json index f8c95826..7814ea1f 100644 --- a/docs/packet-schema.v0.15.json +++ b/docs/packet-schema.v0.15.json @@ -547,7 +547,7 @@ }, "DeclarationQuestionRow": { "additionalProperties": false, - "description": "v0.37: one open declaration question, in the order to answer it.\n\nOrdered strongest-acting action first. Answering the two that move money\nand communicate outward is what reached a verdict on the fourth\n``adk-samples#1745`` walk, so the list leads with them rather than with\nwhatever sorts first alphabetically. Ordering is ranking only \u2014 it decides\nwhat to read first and can never change a verdict.\n\nv0.38: a question is one blank a reviewer fills, so ``answer_path`` \u2014 the\nmanifest block that blank lives in \u2014 is its identity. Actions answered by\nthe same block are one question: a source of 117 actions with no authority\nevidence owes one ``tool_sources[].authority`` block, and counting that as\n117 questions describes one edit as a backlog (#410 increment 3).", + "description": "v0.37: one open declaration question, in the order to answer it.\n\nOrdered by how much answering can move the verdict, rather than by what\nsorts first alphabetically. Answering the two actions that moved money and\ncommunicated outward is what reached a verdict on the fourth\n``adk-samples#1745`` walk. Ordering is ranking only \u2014 it decides what to\nread first and can never change a verdict.\n\nv0.38: a question is one blank a reviewer fills, so ``answer_path`` \u2014 the\nmanifest block that blank lives in \u2014 is its identity. Actions answered by\nthe same block are one question: a source of 117 actions with no authority\nevidence owes one ``tool_sources[].authority`` block, and counting that as\n117 questions describes one edit as a backlog (#410 increment 3).\n\nv0.38 also ranks by the **ceiling** of what an answer can establish rather\nthan by the effect already inferred. The actions nothing has bounded lead \u2014\nno effect evidence at all, a protocol default standing in for its absence,\nor only a heuristic reading this scan may not act on \u2014 because an unbounded\naction is unmeasured rather than safe, and its answer can still turn out to\nbe anything. Actions whose effect a reviewed declaration or policy-eligible\nsource evidence established keep their evidence rank behind them, strongest\nfirst. Position is not severity: the action at the top is the one *least*\nis known about.", "properties": { "answer_path": { "default": "", diff --git a/docs/report-schema.v0.38.json b/docs/report-schema.v0.38.json index 57d18bd0..c23be9ec 100644 --- a/docs/report-schema.v0.38.json +++ b/docs/report-schema.v0.38.json @@ -3033,7 +3033,7 @@ }, "DeclarationQuestionRow": { "additionalProperties": false, - "description": "v0.37: one open declaration question, in the order to answer it.\n\nOrdered strongest-acting action first. Answering the two that move money\nand communicate outward is what reached a verdict on the fourth\n``adk-samples#1745`` walk, so the list leads with them rather than with\nwhatever sorts first alphabetically. Ordering is ranking only \u2014 it decides\nwhat to read first and can never change a verdict.\n\nv0.38: a question is one blank a reviewer fills, so ``answer_path`` \u2014 the\nmanifest block that blank lives in \u2014 is its identity. Actions answered by\nthe same block are one question: a source of 117 actions with no authority\nevidence owes one ``tool_sources[].authority`` block, and counting that as\n117 questions describes one edit as a backlog (#410 increment 3).", + "description": "v0.37: one open declaration question, in the order to answer it.\n\nOrdered by how much answering can move the verdict, rather than by what\nsorts first alphabetically. Answering the two actions that moved money and\ncommunicated outward is what reached a verdict on the fourth\n``adk-samples#1745`` walk. Ordering is ranking only \u2014 it decides what to\nread first and can never change a verdict.\n\nv0.38: a question is one blank a reviewer fills, so ``answer_path`` \u2014 the\nmanifest block that blank lives in \u2014 is its identity. Actions answered by\nthe same block are one question: a source of 117 actions with no authority\nevidence owes one ``tool_sources[].authority`` block, and counting that as\n117 questions describes one edit as a backlog (#410 increment 3).\n\nv0.38 also ranks by the **ceiling** of what an answer can establish rather\nthan by the effect already inferred. The actions nothing has bounded lead \u2014\nno effect evidence at all, a protocol default standing in for its absence,\nor only a heuristic reading this scan may not act on \u2014 because an unbounded\naction is unmeasured rather than safe, and its answer can still turn out to\nbe anything. Actions whose effect a reviewed declaration or policy-eligible\nsource evidence established keep their evidence rank behind them, strongest\nfirst. Position is not severity: the action at the top is the one *least*\nis known about.", "properties": { "answer_path": { "default": "", diff --git a/docs/verifier-schema.v0.12.json b/docs/verifier-schema.v0.12.json index bc62e629..c4f904c4 100644 --- a/docs/verifier-schema.v0.12.json +++ b/docs/verifier-schema.v0.12.json @@ -863,7 +863,7 @@ }, "DeclarationQuestionRow": { "additionalProperties": false, - "description": "v0.37: one open declaration question, in the order to answer it.\n\nOrdered strongest-acting action first. Answering the two that move money\nand communicate outward is what reached a verdict on the fourth\n``adk-samples#1745`` walk, so the list leads with them rather than with\nwhatever sorts first alphabetically. Ordering is ranking only \u2014 it decides\nwhat to read first and can never change a verdict.\n\nv0.38: a question is one blank a reviewer fills, so ``answer_path`` \u2014 the\nmanifest block that blank lives in \u2014 is its identity. Actions answered by\nthe same block are one question: a source of 117 actions with no authority\nevidence owes one ``tool_sources[].authority`` block, and counting that as\n117 questions describes one edit as a backlog (#410 increment 3).", + "description": "v0.37: one open declaration question, in the order to answer it.\n\nOrdered by how much answering can move the verdict, rather than by what\nsorts first alphabetically. Answering the two actions that moved money and\ncommunicated outward is what reached a verdict on the fourth\n``adk-samples#1745`` walk. Ordering is ranking only \u2014 it decides what to\nread first and can never change a verdict.\n\nv0.38: a question is one blank a reviewer fills, so ``answer_path`` \u2014 the\nmanifest block that blank lives in \u2014 is its identity. Actions answered by\nthe same block are one question: a source of 117 actions with no authority\nevidence owes one ``tool_sources[].authority`` block, and counting that as\n117 questions describes one edit as a backlog (#410 increment 3).\n\nv0.38 also ranks by the **ceiling** of what an answer can establish rather\nthan by the effect already inferred. The actions nothing has bounded lead \u2014\nno effect evidence at all, a protocol default standing in for its absence,\nor only a heuristic reading this scan may not act on \u2014 because an unbounded\naction is unmeasured rather than safe, and its answer can still turn out to\nbe anything. Actions whose effect a reviewed declaration or policy-eligible\nsource evidence established keep their evidence rank behind them, strongest\nfirst. Position is not severity: the action at the top is the one *least*\nis known about.", "properties": { "answer_path": { "default": "", diff --git a/llms-full.txt b/llms-full.txt index cba2b0c3..5709a3fa 100644 --- a/llms-full.txt +++ b/llms-full.txt @@ -1774,7 +1774,7 @@ In `agents-shipgate-reports/report.json`: - `release_decision.{blockers,review_items}[].capability_refs` (v0.24+) — stable capability IDs copied from the originating finding when a policy or policy-pack rule matched a `CapabilityFactV1`. Empty for findings that are not capability-policy matches. This is audit metadata only; `release_decision.decision` remains the gate. - `release_decision.{blockers,review_items}[].capability_trace_refs` (v0.25+) — stable local trace-evidence IDs copied from the originating finding when an existing trace/evidence check used declared local trace artifacts. Empty when no local trace row is relevant. This is audit metadata only; `release_decision.decision` remains the gate. - `release_decision.evidence_coverage.semantic_coverage` (v0.29+) — `{total_actions, pass_eligible_actions, gap_count, review_concern_count, reason_counts}`. A non-zero semantic `gap_count` prevents `passed`; a non-zero `review_concern_count` prevents an automatic pass and routes known review concerns to human review — unscoped/ambient authority, and (v0.36+) `acknowledged_effect_override`, a declared effect a reviewer acknowledged as weaker than the evidence inferred for it. Read `reason_counts` for which; the count is of concerns, so one action can contribute more than one. Semantic gaps are not Findings and cannot be suppressed, baselined, severity-overridden, waived by `--no-heuristics`, or satisfied by `human_ack`. -- `release_decision.evidence_coverage.semantic_coverage.declaration_questions` (v0.37+) — the same action surface counted as a questionnaire: `{total, answered, open, open_by_dimension, open_questions[]}`. A *question* is one `(action, dimension)` a reviewed `action_surface.actions` row has to answer, and only `effect` and `authority` are counted — an action whose effect the scan established by itself (an OpenAPI method, an MCP annotation) was never asked and is not in `total`, and an inventory or `agent_bindings` declaration has no per-action counterfactual to score against. `answered` is exact rather than optimistic: it counts dimensions that gap when the same action is re-resolved *without* its declaration. `total == answered + open`, and `open_by_dimension` sums to `open`. `open_questions[]` is the answer order — highest-acting action first, `effect` before `authority` — and joins to `evidence_gaps[].subject_id`. Nothing here gates; it is a projection of counts the decision already made, published so a coding agent (and the generated questionnaire) can report progress instead of a gap tally. +- `release_decision.evidence_coverage.semantic_coverage.declaration_questions` (v0.37+) — the same action surface counted as a questionnaire: `{total, answered, open, open_by_dimension, open_questions[]}`. A *question* is one `(action, dimension)` a reviewed `action_surface.actions` row has to answer, and only `effect` and `authority` are counted — an action whose effect the scan established by itself (an OpenAPI method, an MCP annotation) was never asked and is not in `total`, and an inventory or `agent_bindings` declaration has no per-action counterfactual to score against. `answered` is exact rather than optimistic: it counts dimensions that gap when the same action is re-resolved *without* its declaration. `total == answered + open`, and `open_by_dimension` sums to `open`. `open_questions[]` is the answer order and joins to `evidence_gaps[].subject_id`. v0.38 ranks it by the ceiling of what an answer can establish: the actions nothing has bounded first — no effect evidence, a protocol default standing in for its absence, or only a heuristic reading the scan may not act on — then the actions a reviewed declaration or policy-eligible source evidence established, strongest-acting first, with `effect` before `authority` within one action. Position is not severity: the action at the top is the one *least* is known about. Nothing here gates; it is a projection of counts the decision already made, published so a coding agent (and the generated questionnaire) can report progress instead of a gap tally. - `release_decision.evidence_coverage.policy_gap_count` and top-level `policy_evidence_gaps[]` (v0.33+) — policy applicability that is heuristic-only, mixed, unknown, or conflicting. These rows are outside Findings and cannot be suppressed, baselined, severity-overridden, acknowledged, or removed by `--no-heuristics`; any row prevents `passed`. - `release_decision.evidence_coverage.identity_coverage` (v0.30+) — `{total_observations, canonical_tools, bound_tools, pass_eligible_tools, ambiguous_name_count, gap_count, reason_counts}`. Provider-scoped observations remain separate unless an exact reviewed `tool_identity.bindings[]` entry joins them. Any ambiguous selector, invalid binding, or conflicting identity prevents `passed`. - `release_decision.evidence_coverage.evidence_gaps[]` (v0.26+; semantic kinds added v0.29) — one structured row per measurable gap: `{kind, subject, source_type, source_ref, why, next_action}`. In addition to `low_confidence_tool` and `source_warning`, v0.29 adds `incomplete_surface`, `missing_effect_evidence`, `inferred_effect_only`, `conflicting_effect_evidence`, `missing_authority_evidence`, `partial_authority_evidence`, `conflicting_authority_evidence`, and `invalid_semantic_annotation`. v0.36 adds `declaration_below_inferred_evidence`: the declared effect is weaker than evidence this scan inferred for the same action. The declaration still stands as the operative effect — heuristics never drive a verdict — but the action is not evidence-backed-pass until a reviewer raises the declared effect or adds `action_surface.actions[].override` with the `evidence` they checked and the `reason` it does not apply. An acknowledged override keeps the action pass-eligible and is reported as one semantic review concern, so the run can never read `passed`. Each acknowledgement is also emitted as a row in `release_decision.evidence_coverage.semantic_coverage.acknowledged_overrides[]` (v0.36+) naming the action, both readings, the hint source, any source evidence that agrees, and the reviewer's evidence and reason — the packet's §1 and the PR comment render it, because a count is not a review surface. The acknowledgement is consumed by policy applicability as well, so applying it reaches the review route rather than trading one gap for another. Semantic next actions use `declare_action_effect`, `declare_action_authority`, `declare_tool_inventory`, `provide_complete_inventory`, or `resolve_semantic_conflict`, include accepted values and exact source/manifest pointers, and are always human-routed. v0.37 adds `next_action.observed_readings[]` on effect rows — `{effect, sources[], observed}`, the distinct readings this scan's non-declaration evidence supports, so the row can be answered without opening `action_surface_facts`. Where those readings support one conservative answer, `next_action.declaration_template` carries it **pre-filled** instead of a `` blank. That value is a proposal, not an assertion: it is drawn from the closed `ActionEffect` vocabulary (never from source content), it is never weaker than any reading, and it is offered only where something was observed — a protocol default standing in for the absence of evidence, or a heuristic reading of `read`, keeps the blank. Their declaration placeholders carry `suggested_patch_kind="manual"`, `auto_apply=false`, and `requires_human_review=true`; they are not Patch objects, and a pre-filled template is still operative only once a human merges it into the manifest. v0.37 also re-routes `partial_authority_evidence`: it is raised when the *source's* authority evidence is ambiguous or incomplete, and the resolver preserves it whatever the manifest declares ("reviewed authority cannot replace ambiguous or incomplete source authority alternatives"). Its `next_action.kind` is therefore `provide_source` with no declaration template, rather than a `declare_action_authority` block that could not close the row it was printed on. It is excluded from `declaration_questions` for the same reason. Work the rows in order instead of guessing; Agents Shipgate never auto-asserts effect or authority. diff --git a/src/agents_shipgate/cli/scan/declarations.py b/src/agents_shipgate/cli/scan/declarations.py index ccab374f..d3d19c86 100644 --- a/src/agents_shipgate/cli/scan/declarations.py +++ b/src/agents_shipgate/cli/scan/declarations.py @@ -28,6 +28,13 @@ ``semantic_coverage.declaration_questions``, so the file and the report cannot disagree about how much work is left. +That order leads with the actions nothing has *bounded* — no evidence, a +protocol default standing in for its absence, or only a heuristic reading the +scan may not act on — rather than with the ones it read as risky (#419); see +``core.declaration_questions``. The header says so, and a block with no +reading at all to print says so too, because a blank that printed nothing let +its silence read as "nothing to see here". + **Where evidence supports one conservative answer, it is filled in.** The scan already read ``request_refund_approval`` as a financial write; asking a human to retype that is the cost that stalls adoption, and the readings behind it are @@ -433,9 +440,13 @@ def _header(questions: DeclarationQuestionCoverage | None) -> list[str]: lines.append("#") lines.extend( _wrapped_comment( - "Ordered by how much answering can move the verdict, so the " - "first questions are the ones about money, outward " - "communication, and destruction.", + "Ordered by how much answering can move the verdict. First, " + "the actions nothing has pinned down: no effect evidence at " + "all, or only a reading this scan is not allowed to act on. An " + "action nothing has bounded is unmeasured, not safe, and its " + "answer can still turn out to be anything. Then the ones the " + "scan did establish, strongest first: money, outward " + "communication, destruction.", "", ) ) @@ -554,7 +565,16 @@ def _emit_block( entry["dimensions"], ) ) - out.extend(_reading_lines(entry["readings"], entry["template"])) + out.extend( + _reading_lines( + entry["readings"], + entry["template"], + asks_effect=any( + DIMENSION_BY_GAP_KIND.get(kind) == "effect" for kind in entry["kinds"] + ), + ordered=bool(entry["numbers"]), + ) + ) out.append("---") out.append(f"# closes: {', '.join(entry['kinds'])}") out.append(f"# merge into: {entry['path']}") @@ -593,7 +613,15 @@ def _emit_unfillable( # The readings too. This row has no blank to fill precisely *because* its # sources disagree, so what they each say is the thing the reviewer has to # go and reconcile. - out.extend(_reading_lines(getattr(gap.next_action, "observed_readings", ()) or (), {})) + # Never the "nothing was read" note: this row has no blank to fill, and + # its silence is a conflict between sources rather than an absence. + out.extend( + _reading_lines( + getattr(gap.next_action, "observed_readings", ()) or (), + {}, + asks_effect=False, + ) + ) out.extend(_wrapped_comment(one_line(gap.why), "")) out.append("#") out.extend( @@ -624,6 +652,9 @@ def _proposed_fields(template: dict[str, Any]) -> frozenset[str]: def _reading_lines( readings: Sequence[EvidenceReading], template: dict[str, Any], + *, + asks_effect: bool = False, + ordered: bool = False, ) -> list[str]: """What the scan read this action's effect as, above the value it proposes. @@ -632,10 +663,30 @@ def _reading_lines( presenting it beside a keyword match as though both were evidence about this action would misrepresent the weaker one — and it is exactly the reading nothing is ever proposed from. + + An effect question with *no* readings at all says so (#419). The header + explains why the top of the file is the unbounded half; a block that + printed nothing left a reader to read the silence as "nothing to see + here", which is the reading this whole ordering exists to correct. + + ``ordered`` gates the half of that note which claims a *position*. A report + written before ``declaration_questions`` existed carries no coverage to + number the blocks from, so they keep gap emission order and a blank can + follow a bounded question — where "it is asked before" would be a sentence + the file itself disproves two lines up. """ if not readings: - return [] + if not asks_effect: + return [] + note = ( + "This scan read nothing about this action's effect — an absence of " + "evidence, not evidence that it is safe. Your answer is the only " + "thing that bounds it." + ) + if ordered: + note += " It is asked before the ones the scan could read for itself." + return _wrapped_comment(note, "") observed = [reading for reading in readings if reading.observed] defaults = [reading for reading in readings if not reading.observed] lines: list[str] = [] diff --git a/src/agents_shipgate/core/declaration_questions.py b/src/agents_shipgate/core/declaration_questions.py index 78e75aae..f9f41a3a 100644 --- a/src/agents_shipgate/core/declaration_questions.py +++ b/src/agents_shipgate/core/declaration_questions.py @@ -33,12 +33,33 @@ repaired by a tool inventory, and ``invalid_semantic_annotation`` is a defect in the source, not a blank in the manifest. -Ordering is by how much answering can move the verdict, strongest action -first. The fourth adoption walk of ``adk-samples#1745`` reached ``blocked`` -after declaring 2 of 12 tools — the two that moved money and communicated -outward — so a questionnaire that leads with them reaches the same verdict in -two answers instead of twelve. Ordering is *ranking only*: it decides what to -read first, never what the verdict is. +Ordering is by how much answering can move the verdict. The fourth adoption +walk of ``adk-samples#1745`` reached ``blocked`` after declaring 2 of 12 tools +— the two that moved money and communicated outward — so a questionnaire that +leads with them reaches the same verdict in two answers instead of twelve. + +The quantity that ranks a question is the **ceiling** of what its answer can +establish, not the floor the scan already inferred (#419). Those are not the +same number, and ranking by the second inverted the promise: a proposal is +offered only where something was observed, so ranking by the observation put +every question that arrives with a proposed answer above every question that +arrives blank — the cheapest questions first and the most valuable ones last. +An action nothing has bounded is not a low-risk action; it is an unmeasured +one, its answer can still turn out to be anything in the vocabulary, and it is +exactly where a human answer carries new information. So an unbounded action +outranks every bounded one, and among the bounded the strongest reading leads. + +*Bounded* is :func:`effect_is_bounded`, and it is deliberately not the gate +that decides whether a value may be pre-filled. That one refuses to draft +``effect: read`` from anything, however authoritative, because a confirmed +guess of ``read`` is the one direction that loses safety (#357) — so an +OpenAPI ``GET`` reads as "nothing to propose" while being entirely proven. +Ranking on it sent a structural ``GET`` named ``delete_account`` to the top of +the file with its name breaking the tie, which is this issue's own defect +inverted. + +Ordering is *ranking only*: it decides what to read first, never what the +verdict is. """ from __future__ import annotations @@ -54,9 +75,12 @@ Tool, ToolSemanticAssessment, ) +from agents_shipgate.core.risk_hints import name_shape_band from agents_shipgate.core.semantic_assessment import ( + UNBOUNDED_EFFECT_RANK, assess_tool_semantics, effect_evidence_rank, + effect_is_bounded, ) from agents_shipgate.schemas.report import DeclarationQuestionCoverage @@ -277,26 +301,39 @@ class DeclarationQuestion: dimension: DeclarationDimension answered: bool rank: int + shape: int class _PendingQuestion: """One question under construction, folding in every action that asks it.""" - def __init__(self, target: DeclarationTarget, dimension: DeclarationDimension) -> None: + def __init__( + self, + target: DeclarationTarget, + dimension: DeclarationDimension, + reach: tuple[int, int], + ) -> None: self.target = target self.dimension = dimension self.answered = True - self.rank = 0 + # Seeded from the first action rather than from a zero floor. A bounded + # ``read`` reaches ``(0, 0)`` exactly, so a floor would have tied with a + # real value instead of losing to it. + self.rank, self.shape = reach - def absorb(self, *, answered: bool, rank: int) -> None: + def absorb(self, *, answered: bool, rank: int, shape: int) -> None: # Open wins. A block that answers eleven of its twelve actions and # leaves the twelfth gapping is not an answered question: the reviewer # still has an edit to make, and a counter that said otherwise would # report a finish line the scan does not agree has been reached. self.answered = self.answered and answered # Ranked by the strongest action it covers, so a source carrying one - # financial write is asked about before a source of readers. - self.rank = max(self.rank, rank) + # action nothing was read about is asked before a source the scan read + # end to end. The band travels with the rank rather than being + # maximised on its own: it describes the same action the rank came + # from, and taking the two from different actions would order a + # question by a name nothing else about it refers to. + self.rank, self.shape = max((self.rank, self.shape), (rank, shape)) def build(self) -> DeclarationQuestion: return DeclarationQuestion( @@ -307,16 +344,18 @@ def build(self) -> DeclarationQuestion: dimension=self.dimension, answered=self.answered, rank=self.rank, + shape=self.shape, ) def declaration_questions(tools: Iterable[Tool]) -> list[DeclarationQuestion]: """Every declaration question this scan asks, answered ones included. - Deterministic and total over the catalog: highest-risk action first, then - by subject, then effect before authority. A tool with no semantic - assessment contributes nothing — it has not been resolved, so nothing is - known about what it owes. + Deterministic and total over the catalog: the actions nothing was read + about first, then the ones that were read, strongest first — see + :func:`_reach` — then by subject, then effect before authority. A tool + with no semantic assessment contributes nothing: it has not been resolved, + so nothing is known about what it owes. Actions asking the same question are folded together. That is not a display convenience: the unit of the counter is the unit of the work, and @@ -337,7 +376,7 @@ def declaration_questions(tools: Iterable[Tool]) -> list[DeclarationQuestion]: undeclared = ( assess_tool_semantics(tool, None) if _has_declaration(assessment) else assessment ) - rank = effect_evidence_rank(assessment.conservative_effect) + rank, shape = _reach(tool, assessment) for dimension in DECLARATION_DIMENSIONS: asked = _asked(dimension, assessment, undeclared) if asked is None: @@ -347,14 +386,40 @@ def declaration_questions(tools: Iterable[Tool]) -> list[DeclarationQuestion]: key = (target.kind, target.id, dimension) slot = pending.get(key) if slot is None: - slot = _PendingQuestion(target, dimension) + slot = _PendingQuestion(target, dimension, (rank, shape)) pending[key] = slot - slot.absorb(answered=answered, rank=rank) + slot.absorb(answered=answered, rank=rank, shape=shape) questions = [slot.build() for slot in pending.values()] questions.sort(key=_ordering) return questions +def _reach(tool: Tool, assessment: ToolSemanticAssessment) -> tuple[int, int]: + """``(rank, name band)`` — how far an answer about this action can reach. + + The ceiling, not the floor (#419). Where something already bounds the + effect — a reviewed declaration, policy-eligible structural evidence, or an + observed side effect — the answer cannot go below that, so the action keeps + its evidence rank. Where nothing does, the scan holds no bound at all: the + answer can still turn out to be ``destructive``, so the question sorts + above every bounded one. + + :func:`effect_is_bounded`, not :func:`effect_is_measured`. The second is a + proposal-safety rule and reads ``False`` for a proven read-only action; it + ranked an OpenAPI ``GET`` at the ceiling and then let its name break the + tie, which is this issue's own defect inverted. + + The band is the tiebreaker among the unbounded, and it is ``0`` — inert — + for every bounded action, so a name can never reorder an action the scan + established. See :func:`name_shape_band` for why an unbounded action may be + ordered by something no verdict is allowed to touch. + """ + + if effect_is_bounded(assessment.effect): + return effect_evidence_rank(assessment.conservative_effect), 0 + return UNBOUNDED_EFFECT_RANK, name_shape_band(tool) + + def _target_for( tool: Tool, dimension: DeclarationDimension, @@ -499,8 +564,14 @@ def _subject(tool: Tool) -> str: return f"{tool.name} [{tool.provider or tool.source_id or tool.source_type}]" -def _ordering(question: DeclarationQuestion) -> tuple[int, str, str, str, int]: - """Risk, then subject, then **subject id**, then dimension. +def _ordering(question: DeclarationQuestion) -> tuple[int, int, str, str, str, int]: + """Reach, then name band, then subject, then **subject id**, then dimension. + + Reach is the ceiling of what an answer can establish — see + :func:`_reach` — so the questions nothing has bounded lead, and the band + orders those among themselves. The band is ``0`` for every bounded action, + so that second component only ever separates questions the first one has + already tied. Subject id before dimension is what keeps one subject's questions contiguous. Two canonical tools can render the same display subject, and @@ -517,6 +588,7 @@ def _ordering(question: DeclarationQuestion) -> tuple[int, str, str, str, int]: return ( -question.rank, + -question.shape, question.subject, question.subject_kind, question.subject_id, diff --git a/src/agents_shipgate/core/risk_hints.py b/src/agents_shipgate/core/risk_hints.py index b07b6c7b..005a575b 100644 --- a/src/agents_shipgate/core/risk_hints.py +++ b/src/agents_shipgate/core/risk_hints.py @@ -233,6 +233,42 @@ def is_policy_eligible_write_tool(tool: Tool) -> bool: return bool(policy_eligible_effects(tool) - {"read"}) +def name_shape_band(tool: Tool) -> int: + """A **display-order** hint read off an action's name. Never an effect. + + ``1`` mutating, ``0`` neither, ``-1`` retrieving. Deliberately three coarse + bands: this is a tiebreaker, and a finer scale would invite reading it as a + measurement. + + The keyword classifier above is gated: ``WRITE_KEYWORDS`` are consulted + only for the source types in :data:`_KEYWORD_GATED_SOURCE_TYPES`, because a + Python function called ``create_sap_sales_order`` is not evidence that it + writes anything — the name is the author's, and this scanner will not put a + verdict on it. + + Ordering needs no such trust (#419). A questionnaire has to ask its + questions in *some* order, and where the scan observed nothing at all the + name is the only thing left to prefer one blank over another; getting it + wrong costs a reader one place in a list they must finish either way. So + this reuses the classifier's own vocabulary rather than inventing a second + one, and it is consulted **only** among actions whose effect nothing + measured — never to reorder actions the scan did read, and never on any + path that can reach a claim, an issue, or a verdict. + + The name alone, not the description the classifier also tokenizes. Three + bands is a coarse signal and a paragraph of repository prose dilutes it: + one sentence of a docstring warning against deleting anything would band a + ``get_`` tool as mutating. + """ + + tokens = _tokenize(tool.name) + if tokens & (WRITE_KEYWORDS | DESTRUCTIVE_KEYWORDS | FINANCIAL_KEYWORDS): + return 1 + if tokens & READ_ONLY_KEYWORDS: + return -1 + return 0 + + def _tokenize(text: str) -> set[str]: return set(re.findall(r"[a-z]+", text.lower())) diff --git a/src/agents_shipgate/core/semantic_assessment.py b/src/agents_shipgate/core/semantic_assessment.py index 9d278f29..32a86c33 100644 --- a/src/agents_shipgate/core/semantic_assessment.py +++ b/src/agents_shipgate/core/semantic_assessment.py @@ -1095,6 +1095,94 @@ def effect_evidence_rank(effect: str) -> int: return _EFFECT_RANK.get(cast(ActionEffect, effect), 0) +#: Where an action nothing has bounded sorts on that table. +#: +#: Above every effect in it, deliberately. An action nothing established is not +#: a low-risk action, it is an unbounded one: the strongest thing its answer +#: could turn out to be is the top of the vocabulary, and that is the quantity +#: a questionnaire ordered by "how much can answering this move the verdict" +#: has to rank by (#419). Ranking it by the inferred floor instead put every +#: unread action below every action the scan had already read for itself. +#: +#: Not an ``ActionEffect`` and never converted to one. Nothing but display +#: order reads it, and :func:`effect_evidence_rank` cannot return it. +UNBOUNDED_EFFECT_RANK = max(_EFFECT_RANK.values()) + 1 + + +def effect_is_measured(readings: Sequence[EffectReading]) -> bool: + """Did this scan observe a side effect for this action at all? + + The one gate, asked by two surfaces for opposite purposes and therefore + spelled once. :func:`propose_effect_declaration` asks it to decide whether + a pre-filled answer may be offered at all; the questionnaire asks it to + decide where the question sorts. They are the same fact read from both + ends — what the scan measured is what it may propose, and what it did not + measure is what it most needs a human for — and a second spelling is how + the two would start disagreeing about which questions are the cheap ones + (#419). + + ``read`` alone is **not** a measurement here. This resolver refuses to + establish a read-only action from a heuristic (#357), so an action whose + only *heuristic* reading is ``read`` is exactly as unproven as one with no + reading at all, and its answer can still be anything. + + That last rule is about heuristics, and it makes this predicate wrong for + any question other than "may a value be pre-filled" — an OpenAPI ``GET`` + and a trusted ``readOnlyHint`` are read-only and *proven*, and this still + returns ``False`` for them. :func:`effect_is_bounded` is the one to ask + about what the scan established. + """ + + return any(reading.observed and reading.effect != "read" for reading in readings) + + +#: Effect statuses that hold an action down without a human answering anything. +#: +#: ``declared`` is a reviewed declaration and ``structural`` is policy-eligible +#: source evidence; both establish the effect, ``read`` included. +#: ``conflicting`` is evidence disagreeing with itself, which is still +#: evidence — the conservative reading is a real bound, and the reviewer is +#: being asked to reconcile a known disagreement rather than to name an +#: unknown. +#: +#: Deliberately not ``protocol_default``, which is what the protocol assumes +#: when a server publishes nothing, and not ``inferred``, which is a heuristic +#: this resolver will not let establish a read-only action at all (#357). Both +#: leave the answer unbounded. +_BOUNDED_EFFECT_STATUSES: frozenset[str] = frozenset( + {"declared", "structural", "conflicting"} +) + + +def effect_is_bounded(effect: EffectSemanticAssessment) -> bool: + """Is anything other than a human's answer already holding this effect down? + + The **ordering** question, and deliberately not + :func:`effect_is_measured`. That one answers "may a non-read effect be + pre-filled here", which is a proposal-safety rule: it returns ``False`` for + every read-only reading however authoritative, because a pre-filled + ``effect: read`` is the one direction where a confirmed guess loses safety. + + Reusing it to rank questions put a structurally proven read at the *top* of + the questionnaire — an OpenAPI ``GET`` named ``delete_account`` outranking a + genuinely unknown effect, with a repository-chosen name breaking the tie. + That is the defect #419 exists to fix, wearing the other hat: something the + scan established being ordered as though it had not been. + + Two ways to be bounded, because the resolver records them differently. A + status in :data:`_BOUNDED_EFFECT_STATUSES` means the resolver established + the effect, and it holds for a bounded ``read`` that has no reading at all + behind it — a reviewed ``effect: read`` is the manifest speaking, and + declaration claims are excluded from readings on purpose. Otherwise, an + observed side effect is a bound even where nothing established it: that is + the ``inferred`` action whose keyword hint reads ``external_communication``. + """ + + if effect.status in _BOUNDED_EFFECT_STATUSES: + return True + return effect_is_measured(effect_readings(effect)) + + def effect_readings(effect: EffectSemanticAssessment) -> list[EffectReading]: """Group this action's non-declaration effect claims into readings. @@ -1139,8 +1227,9 @@ def propose_effect_declaration( ) -> EffectProposal | None: """The weakest declaration that accounts for every reading, or ``None``. - ``None`` — keep the blank — in exactly two cases, and both are the point of - an evidence-first proposal rather than a guess: + ``None`` — keep the blank — in exactly the two cases + :func:`effect_is_measured` rules out, and both are the point of an + evidence-first proposal rather than a guess: * **Nothing was observed.** Only a protocol default stands here, and a default is an absence of evidence (see @@ -1160,7 +1249,7 @@ def propose_effect_declaration( The gate above only decides whether to propose at all. """ - if not any(reading.observed and reading.effect != "read" for reading in readings): + if not effect_is_measured(readings): return None values = sorted( {reading.effect for reading in readings}, diff --git a/src/agents_shipgate/schemas/report.py b/src/agents_shipgate/schemas/report.py index 265041e3..006ca9a0 100644 --- a/src/agents_shipgate/schemas/report.py +++ b/src/agents_shipgate/schemas/report.py @@ -535,17 +535,27 @@ class AcknowledgedEffectOverride(BaseModel): class DeclarationQuestionRow(BaseModel): """v0.37: one open declaration question, in the order to answer it. - Ordered strongest-acting action first. Answering the two that move money - and communicate outward is what reached a verdict on the fourth - ``adk-samples#1745`` walk, so the list leads with them rather than with - whatever sorts first alphabetically. Ordering is ranking only — it decides - what to read first and can never change a verdict. + Ordered by how much answering can move the verdict, rather than by what + sorts first alphabetically. Answering the two actions that moved money and + communicated outward is what reached a verdict on the fourth + ``adk-samples#1745`` walk. Ordering is ranking only — it decides what to + read first and can never change a verdict. v0.38: a question is one blank a reviewer fills, so ``answer_path`` — the manifest block that blank lives in — is its identity. Actions answered by the same block are one question: a source of 117 actions with no authority evidence owes one ``tool_sources[].authority`` block, and counting that as 117 questions describes one edit as a backlog (#410 increment 3). + + v0.38 also ranks by the **ceiling** of what an answer can establish rather + than by the effect already inferred. The actions nothing has bounded lead — + no effect evidence at all, a protocol default standing in for its absence, + or only a heuristic reading this scan may not act on — because an unbounded + action is unmeasured rather than safe, and its answer can still turn out to + be anything. Actions whose effect a reviewed declaration or policy-eligible + source evidence established keep their evidence rank behind them, strongest + first. Position is not severity: the action at the top is the one *least* + is known about. """ model_config = ConfigDict(extra="forbid") diff --git a/tests/test_declaration_questionnaire.py b/tests/test_declaration_questionnaire.py index 3dd263cd..6a9f0984 100644 --- a/tests/test_declaration_questionnaire.py +++ b/tests/test_declaration_questionnaire.py @@ -36,6 +36,8 @@ from agents_shipgate.core.semantic_assessment import ( assess_tool_semantics, declaration_covers, + effect_is_bounded, + effect_is_measured, effect_readings, propose_effect_declaration, ) @@ -326,9 +328,15 @@ def test_a_manifest_cannot_be_the_source_that_contradicts_itself() -> None: # -------------------------------------------------------------------------- -def _mcp_workspace(tmp_path: Path, *, tools: list[dict], actions: list[dict]) -> Path: +def _mcp_workspace( + tmp_path: Path, + *, + tools: list[dict], + actions: list[dict], + risk_overrides: dict | None = None, +) -> Path: (tmp_path / "tools.json").write_text(json.dumps({"tools": tools}), encoding="utf-8") - manifest = { + manifest: dict = { "version": "0.1", "project": {"name": "questionnaire"}, "agent": {"name": "asst", "declared_purpose": ["test the questionnaire"]}, @@ -350,6 +358,8 @@ def _mcp_workspace(tmp_path: Path, *, tools: list[dict], actions: list[dict]) -> } if actions: manifest["action_surface"] = {"actions": actions} + if risk_overrides: + manifest["risk_overrides"] = risk_overrides config = tmp_path / "shipgate.yaml" config.write_text(yaml.safe_dump(manifest, sort_keys=False), encoding="utf-8") return config @@ -800,6 +810,311 @@ def test_questions_lead_with_the_action_that_can_move_the_verdict() -> None: ] +# -------------------------------------------------------------------------- +# 4. The order ranks by the ceiling, not by what was already inferred (#419) +# -------------------------------------------------------------------------- +# +# A proposal is offered only where something was observed, and the order used +# to be the strength of that same observation — so the questions that arrived +# with a draft answer systematically outranked the questions that arrived +# blank. On the reference walk that put three already-drafted mail tools at +# Q2-Q4 and the financial write, the single question that produced both +# ``critical`` blockers, at Q6. + + +def _walk_shaped_catalog() -> list[Tool]: + """The fifth ``adk-samples#1745`` walk in miniature. + + Three tools the scan read as outward communication — the ones that arrive + with a proposed answer — and four it read nothing about at all, including + the financial write that produces every blocker once it is answered. + """ + + tools = [ + _observing("external_communication", id="t_send", name="send_email"), + _observing("external_communication", id="t_list", name="list_messages"), + _observing("external_communication", id="t_mgr", name="get_manager_email"), + _tool(id="t_order", name="create_sap_sales_order"), + _tool(id="t_status", name="update_opportunity_status"), + _tool(id="t_map", name="map_salesforce_account_to_sap_bp"), + _tool(id="t_items", name="get_opportunity_line_items"), + ] + for tool in tools: + tool.semantic_assessment = assess_tool_semantics(tool, None) + return tools + + +def _has_proposal(tool: Tool) -> bool: + assert tool.semantic_assessment is not None + return propose_effect_declaration(_readings(tool)) is not None + + +def test_the_first_effect_question_is_one_the_scan_could_not_draft() -> None: + """The reference walk's headline symptom, asserted (#419). + + An action nothing was observed about is not a low-risk action; it is an + unmeasured one, and it is exactly where a human answer carries the most new + information. A reader working top to bottom must not have to confirm three + drafts before reaching it. + """ + + tools = _walk_shaped_catalog() + by_id = {tool.id: tool for tool in tools} + + effect_questions = [ + question + for question in declaration_questions(tools) + if question.dimension == "effect" + ] + + first = effect_questions[0] + assert not _has_proposal(by_id[first.subject_id]), ( + f"the questionnaire leads with {first.subject!r}, which already carries " + "a proposed answer" + ) + # And the money question is reached before any of the drafted ones. + order = [question.subject_id for question in effect_questions] + assert order.index("t_order") < min( + order.index(subject_id) for subject_id in ("t_send", "t_list", "t_mgr") + ) + + +def test_a_drafted_question_never_precedes_an_unbounded_one() -> None: + """The invariant, stated on the mechanism rather than on the symptom. + + Ordering asks :func:`effect_is_bounded` — has anything other than a human's + answer already held this effect down. Where nothing has, the question sorts + first, and on a catalog like this one that half is also exactly the half no + proposal is offered for, which is the correlation the issue reported. + + Note what is deliberately *not* asserted: that a blank question precedes a + drafted one full stop. An action's own authority question arrives blank and + still follows its effect question — they are one manifest row and one + block, and separating them would number that block "Questions 1 and 7". + """ + + tools = _walk_shaped_catalog() + bounded = { + tool.id: effect_is_bounded(tool.semantic_assessment.effect) # type: ignore[union-attr] + for tool in tools + } + questions = declaration_questions(tools) + # Every question here is about one action, so ``subject_id`` is a tool id. + assert {question.subject_kind for question in questions} == {"action"} + + last_unbounded = max( + index + for index, question in enumerate(questions) + if not bounded[question.subject_id] + ) + first_bounded = min( + index for index, question in enumerate(questions) if bounded[question.subject_id] + ) + assert last_unbounded < first_bounded, [ + (question.subject, question.dimension, bounded[question.subject_id]) + for question in questions + ] + # On this catalog the bounded half is exactly the drafted half, which is + # what makes the statement above about proposals at all. + for question in questions: + if question.dimension != "effect": + continue + tool = next(item for item in tools if item.id == question.subject_id) + assert _has_proposal(tool) is bounded[question.subject_id] + + +def test_an_unbounded_action_outranks_every_bounded_one() -> None: + """Even a measured ``destructive``: the ceiling is above the whole table. + + The unbounded action is named as harmlessly as the vocabulary allows — it + lands in the band's *lowest* tier — so nothing but "nothing bounds it" can + be putting it first. + """ + + razed = _observing("destructive", id="t_razed", name="drop_all_tables") + unread = _tool(id="t_unread", name="get_status") + for tool in (razed, unread): + tool.semantic_assessment = assess_tool_semantics(tool, None) + + questions = declaration_questions([razed, unread]) + + assert [question.subject_id for question in questions][:2] == [ + "t_unread", + "t_unread", + ] + + +def test_unbounded_questions_are_ordered_by_what_their_names_suggest() -> None: + """The tiebreaker, and the only place a name is allowed to decide anything. + + Every action here is equally unbounded, so without the band the order is + alphabetical — which is no order at all for a reader with limited + attention. The band never establishes an effect; it decides which blank a + reader sees first among blanks they have to fill either way. + """ + + tools = [ + _tool(id="t_get", name="get_account"), + _tool(id="t_handle", name="handle_account"), + _tool(id="t_delete", name="delete_account"), + ] + for tool in tools: + tool.semantic_assessment = assess_tool_semantics(tool, None) + + ordered = [ + question.subject_id + for question in declaration_questions(tools) + if question.dimension == "effect" + ] + + assert ordered == ["t_delete", "t_handle", "t_get"] + + +def test_a_name_cannot_reorder_an_action_the_scan_actually_read() -> None: + """The negative control for the band. + + A repository names its own tools. If the band applied to measured actions, + calling a financial write ``get_status`` would push the question that fires + both blockers below one the scan read as a plain write. + """ + + money = _observing("financial_write", id="t_money", name="get_status") + plain = _observing("write", id="t_plain", name="create_everything") + for tool in (money, plain): + tool.semantic_assessment = assess_tool_semantics(tool, None) + + ordered = [ + question.subject_id + for question in declaration_questions([money, plain]) + if question.dimension == "effect" + ] + + assert ordered == ["t_money", "t_plain"] + + +def test_the_band_reaches_nothing_but_the_questionnaire_order() -> None: + """A name-shaped hint may order questions and nothing else (#419). + + Stated as a source fact rather than a behavioural one, because the failure + it guards against is a *future* call site: the moment this is read + somewhere a claim, an issue, or a verdict can see, an unreviewed reading of + a repository-chosen string becomes evidence. + """ + + root = Path(__file__).resolve().parents[1] / "src" / "agents_shipgate" + callers = { + path.relative_to(root).as_posix() + for path in root.rglob("*.py") + if "name_shape_band" in path.read_text(encoding="utf-8") + } + + assert callers == {"core/risk_hints.py", "core/declaration_questions.py"}, ( + "a name-shaped reading reached a new module; it may order questions and " + "nothing else" + ) + + +def test_the_proposal_gate_says_exactly_what_is_drafted() -> None: + """Exhaustive: ``effect_is_measured`` agrees with what is actually drafted. + + It is the proposal gate and nothing else. Ordering asks + :func:`effect_is_bounded` instead — see the test below for the shapes where + the two deliberately disagree. + """ + + checked = 0 + for count in (0, 1, 2): + for effects in itertools.combinations(sorted(_TAG_FOR_EFFECT), count): + readings = _readings(_observing(*effects)) + checked += 1 + assert effect_is_measured(readings) is ( + propose_effect_declaration(readings) is not None + ), f"{effects}: the proposal gate disagrees with the proposal" + assert checked > 40, "the sweep stopped exercising the gate" + + +#: The three ways an action's effect can be **read** and still be *proven*. +#: +#: Each is bounded — the answer cannot come out anything but ``read`` — and +#: each is invisible to :func:`effect_is_measured`, which returns ``False`` for +#: every read-only reading so that ``effect: read`` is never pre-filled from a +#: heuristic (#357). Every one is named to look as mutating as the name band +#: can score, so a test that passes cannot be passing by accident. +_BOUNDED_READS: dict[str, dict[str, object]] = { + "openapi_get": { + "name": "delete_account", + "source_type": "openapi", + "annotations": {"httpMethod": "GET"}, + }, + "mcp_read_only_hint": { + "name": "drop_everything", + "source_type": "mcp", + "annotations": {"mcp_server": True, "readOnlyHint": True}, + }, +} + + +@pytest.mark.parametrize("shape", sorted(_BOUNDED_READS)) +def test_a_proven_read_is_not_treated_as_unread(shape: str) -> None: + """A structural ``read`` is bounded, however mutating its name looks. + + ``effect_is_measured`` is a proposal-safety rule: it says ``False`` for an + OpenAPI ``GET`` and a trusted ``readOnlyHint`` because a pre-filled + ``effect: read`` is the one direction a confirmed guess loses safety in. + Ranking questions with it put those actions at the *ceiling* and then let a + repository-chosen name break the tie — this issue's own defect inverted. + """ + + proven = _tool(id="t_proven", **_BOUNDED_READS[shape]) + unknown = _tool(id="t_unknown", name="get_status") + for tool in (proven, unknown): + tool.semantic_assessment = assess_tool_semantics(tool, None) + # The premise: the scan established one and knows nothing about the other, + # and neither can be drafted from. + assert effect_is_bounded(proven.semantic_assessment.effect) # type: ignore[union-attr] + assert not effect_is_bounded(unknown.semantic_assessment.effect) # type: ignore[union-attr] + assert not _has_proposal(proven) and not _has_proposal(unknown) + + questions = declaration_questions([proven, unknown]) + + assert questions[0].subject_id == "t_unknown", [ + (question.subject, question.dimension, question.rank, question.shape) + for question in questions + ] + # And nothing about the proven action is ordered by its name. + assert [question.shape for question in questions if question.subject_id == "t_proven"] == [0] + + +def test_a_partly_answered_action_keeps_the_rank_its_answer_gave_it() -> None: + """A reviewed ``effect: read`` bounds the action; its authority still gaps. + + The declaration leaves no reading behind — declaration claims are excluded + from readings on purpose, a row is not evidence about itself — so a rule + written on readings alone reads this action as unread and floats its + remaining authority question to the top of the file, above questions nobody + has answered anything about. + """ + + declared = _tool(id="t_declared", name="purge_records") + declared.semantic_assessment = assess_tool_semantics( + declared, ActionDeclarationConfig(tool="purge_records", effect="read") + ) + unknown = _tool(id="t_unknown", name="get_status") + unknown.semantic_assessment = assess_tool_semantics(unknown, None) + + questions = declaration_questions([declared, unknown]) + still_open = [question for question in questions if not question.answered] + + # The effect question was answered; the authority question was not. + assert {(q.dimension, q.subject_id) for q in still_open} == { + ("authority", "t_declared"), + ("effect", "t_unknown"), + ("authority", "t_unknown"), + } + assert [question.subject_id for question in still_open][0] == "t_unknown" + assert [q.shape for q in questions if q.subject_id == "t_declared"] == [0, 0] + + # -------------------------------------------------------------------------- # The rendered questionnaire # -------------------------------------------------------------------------- @@ -957,6 +1272,219 @@ def test_a_default_reading_is_shown_but_marked_as_not_evidence() -> None: assert yaml.safe_load(scaffold)["effect"] == REVIEW_REQUIRED_SENTINEL +def test_a_blank_the_scan_read_nothing_for_says_so() -> None: + """Silence at the block is what the header exists to correct (#419). + + The header explains that the top of the file is the unread half. A block + that printed nothing at all left the reader to read that silence as + "nothing to see here" — the reading the whole ordering exists to correct. + """ + + note = "This scan read nothing about this action's effect" + + blank = build_declaration_scaffold( + [ + _gap( + "missing_effect_evidence", + subject_id="t1", + template={"tool": "send_email", "effect": REVIEW_REQUIRED_SENTINEL}, + ) + ], + questions=_coverage_rows(("t1", "effect")), + ) + assert blank is not None + assert note in " ".join(blank.split()) + + # Not where the scan did read something, however weak the reading. + drafted = build_declaration_scaffold( + [ + _gap( + "inferred_effect_only", + subject_id="t1", + template={"tool": "send_email", "effect": "external_communication"}, + readings=[ + EvidenceReading( + effect="external_communication", sources=["risk_hint:keyword"] + ) + ], + ) + ], + questions=_coverage_rows(("t1", "effect")), + ) + assert drafted is not None + assert note not in " ".join(drafted.split()) + + # And not on a block that never asked about an effect at all. + authority = build_declaration_scaffold( + [ + EvidenceGap( + kind="missing_authority_evidence", + subject="src [tool_source]", + subject_id="src", + subject_kind="tool_source", + source_type="mcp", + why="test", + next_action=EvidenceGapAction( + kind="declare_action_authority", + path="shipgate.yaml#tool_sources[id='src'].authority", + why="test", + expects="Declare reviewed authority, then rerun verification.", + declaration_template={ + "id": "src", + "authority": {"mode": REVIEW_REQUIRED_SENTINEL}, + }, + ), + ) + ], + questions=DeclarationQuestionCoverage( + total=1, + answered=0, + open=1, + open_by_dimension={"authority": 1}, + open_questions=[ + DeclarationQuestionRow( + subject="src [tool_source]", + subject_id="src", + subject_kind="tool_source", + dimension="authority", + ) + ], + ), + ) + assert authority is not None + assert note not in " ".join(authority.split()) + + +def test_the_note_claims_a_position_only_when_it_has_one() -> None: + """``questions=None`` is supported, and it numbers nothing (#419 review). + + A report written before ``declaration_questions`` existed carries no + coverage, so the blocks keep gap emission order and a blank can follow a + bounded question. "It is asked before the ones the scan could read for + itself" is then a sentence the file itself disproves two lines up. + """ + + placement = "It is asked before the ones the scan could read for itself." + gaps = [ + _gap( + "inferred_effect_only", + subject_id="t1", + name="send_email", + template={"tool": "send_email", "effect": "external_communication"}, + readings=[ + EvidenceReading( + effect="external_communication", sources=["risk_hint:keyword"] + ) + ], + ), + _gap( + "missing_effect_evidence", + subject_id="t2", + name="create_order", + template={"tool": "create_order", "effect": REVIEW_REQUIRED_SENTINEL}, + ), + ] + + unnumbered = build_declaration_scaffold(gaps) + assert unnumbered is not None + unnumbered_prose = " ".join(unnumbered.split()) + # The blank still says what it is; it just does not claim a place in a + # queue that was never built. + assert "This scan read nothing about this action's effect" in unnumbered_prose + assert placement not in unnumbered_prose + # The premise: emission order really does put the drafted block first here. + assert unnumbered.index("create_order") > unnumbered.index("send_email") + + numbered = build_declaration_scaffold( + gaps, questions=_coverage_rows(("t2", "effect"), ("t1", "effect")) + ) + assert numbered is not None + assert placement in " ".join(numbered.split()) + + +def test_the_header_sentence_and_the_realised_order_agree(tmp_path: Path) -> None: + """The file's own promise, checked against the file (#419). + + Rendered end to end, because the defect this closes was precisely a header + describing an order the questionnaire did not use: it claimed to lead with + what could move the verdict while leading with what the scan had already + read for itself. + + All three shapes the header has to describe are present, because the + unbounded half is not only the actions with nothing printed above them: + + * ``create_sap_sales_order`` — nothing read at all; + * ``fetch_thing`` — only the MCP protocol default, an absence of evidence; + * ``sync_ledger`` — a heuristic reading of ``read``, printed above the + block, which this resolver may not act on (#357) and which therefore + bounds nothing. A header saying the top of the file is what the scan + "could read nothing about" was false for exactly this one. + """ + + config = _mcp_workspace( + tmp_path, + tools=[ + { + "name": "send_email", + "description": "Send an email to the customer.", + "auth": {"type": "oauth2", "scopes": ["mail:send"]}, + }, + {"name": "create_sap_sales_order", "description": "Create the order in SAP."}, + {"name": "fetch_thing", "description": "A tool."}, + {"name": "sync_ledger", "description": "A tool."}, + ], + actions=[], + risk_overrides={ + "tools": {"sync_ledger": {"tags": ["read_only"], "reason": "reviewed"}} + }, + ) + run_scan( + config_path=config, + output_dir=tmp_path / "out", + formats=["json"], + ci_mode="advisory", + packet_enabled=False, + ) + + scaffold = (tmp_path / "out" / "suggested-declarations.yaml").read_text( + encoding="utf-8" + ) + + # Unwrapped: the header is prose wrapped into comment lines, and asserting + # on the wrapping instead of the sentence is a test that fails on a rename. + prose = " ".join( + " ".join( + line.lstrip("#").strip() + for line in scaffold.splitlines() + if line.startswith("#") + ).split() + ) + assert "the actions nothing has pinned down" in prose + assert "Then the ones the scan did establish, strongest first" in prose + + # Every blank effect block precedes every drafted one, in the file itself. + drafted: list[int] = [] + blank: list[int] = [] + for index, line in enumerate(scaffold.splitlines()): + stripped = line.strip() + if not stripped.startswith("effect:"): + continue + value = stripped.split(":", 1)[1].strip() + (blank if value == REVIEW_REQUIRED_SENTINEL else drafted).append(index) + + assert blank and drafted, "the fixture stopped carrying both kinds" + assert max(blank) < min(drafted), scaffold + + # The third shape specifically: a heuristic ``read`` is printed, and the + # block still sits in the half the header says comes first. + heuristic_read = next( + index + for index, line in enumerate(scaffold.splitlines()) + if "read — risk_hint:manual" in line + ) + assert heuristic_read < min(drafted), scaffold + + def test_repository_controlled_text_cannot_forge_a_reading_line() -> None: """A claim source embeds repository-controlled names (``risk_hint:``).