feat(evidence): authority follows credentials, not functions (#410 increment 3) - #417
Conversation
…crement 3)
Every action a tool source contributes normally runs with the same credential,
and asking for it once per action asks the same infrastructure question N
times. That is not merely tedious — it is what breeds the copy-paste that
breeds wrong answers, and a wrong authority declaration is the one that makes
an unscoped production credential read as `mode: none`. Section C of the
evidence-first declaration RFC moves the claim to where the fact lives.
* **A new `tool_sources[].authority` block** — `{mode, auth_type,
credential_mode, scopes, reason}`, with exactly the mode co-requirements an
action row already obeys. They are now one shared rule rather than two
copies, so a manifest one site rejects and the other accepts is not
reachable. The only difference is where `scopes` lives: an action row keeps
its permission list in the sibling `actions[].scopes` field, and a source,
having no such sibling, carries its scopes inside the block.
* **Additive, and never a weaker statement.** An `action_surface.actions[]` row
declaring its own `authority` still wins for that action; a source with no
block resolves exactly as before. Both spellings are normalized into one
record before anything judges them, so the source block is held to the same
conflict rule: declaring `mode: none` across a source whose actions publish
an OAuth scope raises `conflicting_authority_evidence` on each action that
disagrees, and it still cannot stand in for authority a source publishes
ambiguously.
* **One blank is one question.** The questionnaire's unit was
`(action, dimension)`, which counted one edit as N things to do. A question
is now identified by the manifest block that answers it (`answer_path`), so
a source of N actions with no authority evidence is one question, one
numbered block, and one `evidence_gaps[]` row naming the source and how many
actions wait on it. Nothing above the published rows changed: every action
still carries the issue and still fails pass eligibility for it. Conflicts
stay per action, because each asks a reader of *that* action which claim is
wrong.
* **Nothing is prescribed where nothing can be written.** The source route is
offered only for a `source_id` the manifest configures; a per-scan adapter's
source id keeps being asked on its own row.
Also fixes a divergence the change surfaced: two derivations of "which
manifest site is operative" gave two answers to what an action is granted. An
action declaring `mode: none` under a scoped source published the source's
scopes as its `required_scopes` while its assessment reported none. Both
surfaces now read `reviewed_authority`.
Report schema 0.37 → 0.38, packet 0.14 → 0.15, verifier 0.11 → 0.12; all
additive, all prior versions frozen, hash-pinned, and read forward.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…#410 review) Review pass on this branch. Three findings, and the first is the one that matters: the change had two derivations of *what is this action granted?* **Two answers to one question.** The first draft published a source's grant as each action's `required_scopes` without reading the same list as effect evidence, so a source granting `crm.delete` beside a declared `effect: read` had nothing objecting — the #409 failure mode with the assertion moved four lines up the file. Keeping the grant off the action instead turned out to be worse than a style choice: `CapabilityFactV1` *requires* `authority.scopes` to equal the semantic authority's, and one of its two builders reconstructs the fact from `required_scopes`, so the divergence raised `CapabilityFactV1.authority.scopes must project semantic authority` and turned adding the block into an internal error on the base-vs-head path. Whichever site is operative now supplies the whole record, permission list included, and that one list feeds the action fact, the authority dimension, the capability fact, and the effect evidence — so a write-verb permission the manifest says an action requires bounds that action's effect whichever site asserted it. Resolved once per call and passed to both dimensions rather than derived twice. Guarded by the invariant asserted directly across every reviewed-authority shape, and by a real `verify --base` run over a commit that adds the block, because the unit-level builder raises identically on `main` and would not have distinguished new from pre-existing. **A fallback that could never reach its fallback.** A source-wide row's `source_ref` was meant to name the file the source is read from rather than the JSON pointer of whichever action happened to build the row — but the chain began with the caller's `source_ref`, which is the issue's own per-action pointer and is always set for this kind. The branch was dead on arrival; found by reading the rendered rows, not the code. **Surface with no reader, and two stale references.** The normalizer was exported while nothing outside the module read it; a comment named a test that does not exist under that name; and nothing pinned the source-answerable kinds to be a subset of the kinds a declaration can close at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The adversarial declaration from #409, with the grant moved to the source block: `effect: read` on an action the manifest says requires `crm.delete`. Written on the action row this has always been a blocking conflict, and the whole point of reading one resolved permission list is that moving the grant four lines up the file does not make it go quiet. Verdict `blocked`, `0/1` pass-eligible, `conflicting_effect_evidence` raised. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…pped A paragraph survived from a draft this branch abandoned — it said the source's grant is deliberately kept off the action's `required_scopes`, which is the opposite of what the code does and of why. `CapabilityFactV1` requires the two to agree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
pengfei-threemoonslab
left a comment
There was a problem hiding this comment.
Blocking findings found. GitHub is receiving this as a comment review because the authenticated account authored the PR and cannot request changes on its own PR.
I reviewed the source-authority resolver, canonical identity behavior, policy matching, schema validators, generated contracts, documentation, and tests. The first two inline findings can turn incomplete or stronger credential evidence into a clean pass, or suppress credential-mode policy matching; they should be fixed before merge. The remaining findings close a contradictory manifest state and public schema-contract drift.
Validation: the targeted authority/semantic/effect/questionnaire/packet tests passed and all current PR checks are green. Local Agents Shipgate verification returned control_state: review_publishable with release decision review_required; it permits updating the PR, but not merge or completion.
| tool = original.model_copy() if copy_tools else original | ||
| declaration = by_tool.get(tool.id) | ||
| tool.semantic_assessment = assess_tool_semantics(tool, declaration) | ||
| tool_source = by_source.get(tool.source_id) if tool.source_id else None |
There was a problem hiding this comment.
[P1] Bind authority to configured-source provenance, not raw IDs
Tool.source_id is not a foreign key to manifest.tool_sources: configured IDs are arbitrary, while per-scan adapters mint fixed IDs in the same namespace. I reproduced an unrelated MCP row with id: openai_api and authority: {mode: none} being applied to OpenAI API tools, clearing missing_authority_evidence and changing pass_eligible from false to true. The inverse also fails: a configured codex_config source does not match emitted codex_config_mcp:<path> tools; canonical identity bindings can additionally contain multiple contributing sources while this keeps only the primary ID. Please carry explicit, possibly plural, configured-source provenance through loading/identity resolution and resolve divergent declarations on that relationship. The fallback lookup in core/lenses/action_surface.py:507 needs the same fix.
|
|
||
| source_credential_mode = (tool.auth.credential_mode or "").strip().lower() | ||
| declared_credential_mode = (authority.credential_mode or "").strip().lower() | ||
| declared_credential_mode = (reviewed.credential_mode or "").strip().lower() |
There was a problem hiding this comment.
[P1] Do not erase a published credential mode by omission
The comparison below rejects only when both credential modes are present. If a tool publishes credential_mode: service_account and the new source-wide block omits this optional field, the result becomes declared and pass-eligible while replacing the published value with None, with no issue. Capability policies then treat credential_modes: [service_account] as not matched. Preserve the concrete source value or emit conflicting_authority_evidence, and add a source-wide omission test that exercises policy applicability.
| normalized_reason = (reason or "").strip() | ||
| has_scopes = bool(list(scopes)) | ||
|
|
||
| if mode == "none": |
There was a problem hiding this comment.
[P2] Reject credential metadata for mode: none
This validator defines none as "no credential at all", but it never receives or checks credential_mode. Both authority sites therefore accept {mode: none, credential_mode: service_account}; on a structurally complete read action that contradictory fact is pass-eligible. Pass credential_mode into this shared validator, reject nonblank values for none, and cover both declaration sites in the mode matrix.
| - §10 ("What this packet did NOT prove") — always lists prompt robustness, runtime behavior, model correctness, adversarial resistance. | ||
|
|
||
| The packet schema is `0.14`; full schema at [`docs/packet-schema.v0.14.json`](packet-schema.v0.14.json). It projects current report binding and semantic coverage plus gap remediation; v0.13 and older versions are frozen references. | ||
| The packet schema is `0.14`; full schema at [`docs/packet-schema.v0.15.json`](packet-schema.v0.15.json). It projects current report binding and semantic coverage plus gap remediation; v0.13 and older versions are frozen references. |
There was a problem hiding this comment.
[P2] Keep public schema-version statements synchronized
This sentence still calls the packet schema 0.14 while linking v0.15; the table at lines 292-293 likewise calls report/packet 0.37/0.14 while linking v0.38/v0.15 and omits the newly frozen versions. The same partial bump remains in AGENTS.md (stable cells), docs/overview.md, docs/architecture.md, and the current-version headline in STABILITY.md. Strict consumers use these as the integration contract, so update the labels/frozen lists together and add a parity test against the emitted contract constants.
…ion (#410 review) Four review findings on this branch. The first is the one that matters: the join was wrong, in both directions. **A source id is not a foreign key.** `Tool.source_id` is minted by the adapter, and configured ids share that namespace. A `tool_sources` row of type `mcp` calling itself `openai_api` had its reviewed authority applied to the OpenAI API surface — clearing `missing_authority_evidence` for actions nobody declared anything about, and moving them to eligible on that dimension — while a `codex_config` row, whose adapter emits ids derived from the file it read, applied to nothing at all. The dispatcher now records which configured entry each loaded result was produced *for* (exactly, in pass 1; by the `(adapter type, minted id)` pair in pass 2, where the four manifest-only adapters are rejected in `tool_sources` outright and so cannot collide), identity resolution carries it onto the canonical action, and the declaration joins on that. A reviewed `tool_identity` binding merging observations from two configured sources answers for neither — their credentials are separate facts — and the question stays on the action row. **An omitted optional field is not a claim of absence.** A declaration that leaves out `credential_mode` was overwriting a published `service_account` with nothing, leaving the dimension `declared` and pass-eligible while capability policies matching `credential_modes: [service_account]` silently stopped matching. The published value is preserved where the declaration states none; a *different* stated value is still a conflict. **`mode: none` means no credential, including its mode.** Both declaration sites accepted `{mode: none, credential_mode: service_account}` — a fact about a credential the same block says does not exist — and on a structurally complete read action that pair was pass-eligible. **A bump moves the labels, not only the filenames.** Table cells reading `0.37` beside a v0.38 link, "The packet schema is `0.14`" above a v0.15 link, and a `verifier_schema_version: "0.7"` in README and the Claude Code skill that had been stale for several releases. Two parity tests hold them together now: a line linking the current schema must also name its version, and a quoted `<kind>_schema_version` must equal what the engine emits unless the line or its section marks it as history. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A joined string needs a separator, and the id half is repository-chosen — one more thing that can appear inside it. The tuple is the key. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
#415 landed on main and touches the same loading and identity modules. The only textual conflict was an import line in `schemas/manifest/tool_sources.py` where both sides added to `from typing import …`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`configured_source_ids` was a local holding the ids adapters *minted*, which is what a binding member selector names. Since the provenance fix that spelling means something else and more precise — the `tool_sources` entry a result was produced *for* — and the two sets differ wherever an adapter derives its own ids. The local is `loaded_source_ids` now; behaviour is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Increment 3 (section C) of the evidence-first declaration RFC, #410. Increment 1 shipped in #411/#413; increment 2 in #416.
The problem this closes
Authority is a fact about a deployment, not about a function. Every action a tool source contributes normally runs with the same credential, so asking for it once per action asks the same infrastructure question N times. The RFC measures the cost:
smart_closer48 hand-typed values;github-mcp-server117 tools that owe one authority answer. And the cost is not only tedium — it is what breeds the copy-paste that breeds wrong answers, and a wrong authority declaration is the one that makes an unscoped production credential read asmode: none.What this adds
tool_sources[].authority—{mode, auth_type, credential_mode, scopes, reason}:One rule, two sites. The mode co-requirements an action row already obeys are now a single shared validator rather than two copies, so a manifest one site rejects and the other accepts is not reachable. The only difference is where
scopeslives: an action row keeps its permission list in the siblingactions[].scopesfield (one canonical list per action); a source has no such sibling, so its scopes go inside the block.One permission list, everywhere. Whichever site is operative supplies the whole record, permission list included, and that one list is what every surface reports and judges: the action's
required_scopes, the authority dimension'sscopes, and the capability fact's — whichCapabilityFactV1requires to agree — and the list the effect evidence reads, so a write-verb permission the manifest says an action requires still bounds that action's effect whichever site asserted it. An action needing a narrower list declares its ownauthorityblock beside itsscopes.Additive, and never a weaker statement. An
action_surface.actions[]row that declares its ownauthoritystill wins for that action; a source with noauthorityblock resolves exactly as before. The resolver normalizes both spellings into one record before it judges anything, so the source block is held to the same rule as the action row:mode: noneacross a source whose actions publish an OAuth scope raisesconflicting_authority_evidenceon each action that disagrees, naming the block to correct;partial_authority_evidenceis preserved whatever the manifest says, and it is not counted as a question.One blank is one question. The questionnaire's unit was
(action, dimension), which counted one edit as N things to do. A question is now identified by the manifest block that answers it, published asopen_questions[].answer_path. So a source of N actions with no authority evidence is one question, one numbered block insuggested-declarations.yaml, and oneevidence_gaps[]row —subject_kind: tool_source, subjectcrm [tool_source], and awhythat says how many actions are waiting on it. Nothing above the published rows changed: every action still carries the issue and still fails pass eligibility for it. Conflicts stay per action, because each one asks a reader of that action which of the two claims is wrong.Nothing is prescribed where nothing can be written. The source route is offered only for a
source_idthe manifest actually configures — a per-scan adapter stamps a source idtool_sourcesdoes not accept, and those actions keep being asked on their own row rather than sent to a manifest key the schema rejects.Measured on a synthetic 117-tool MCP source, the RFC's
github-mcp-servershape: 234 declaration questions → 118, of which exactly one is the authority question, andsuggested-declarations.yamlcontains exactly onetool_sourcesblock. Semantic evidence gaps drop the same way — one merged row instead of 117 identical ones, reading "117 actions from tool source 'github' have no explicit or structural authority evidence." On a 4-tool fixture:8questions →5,8gaps →5.Safety boundaries, checked rather than assumed
_SAFE_SOURCE_KEYS) is an allowlist, so atool_sourcesrow carryingauthorityis refused as "authority-bearing" — pinned by a test, because that allowlist is now load-bearing in a new way.doctorwill not publish an executable edit for it.authorityis already a human-owned placeholder leaf matched at every path segment; pinned.next_action.kindstaysdeclare_action_effect/declare_action_authority— the kind names the claim being asked for, which the contract'sdo_not_auto_assert[]is keyed on. No contract bump.shipgate.yaml, the trust root; no source content can self-declare it.effect: readon a tool taggedexternal_writebecomes pass-eligible with zero findings #409 method). Declaringmode: noneacross a source whose tools publish OAuth scopes:pass_eligible_actions: 0, twoconflicting_authority_evidencerows, verdict notpassed. There is no quiet path.Two defects found while reviewing this branch
Two answers to "what is this action granted?" The first draft published the source's grant as the action's
required_scopeswithout reading the same list as effect evidence, so a source grantingcrm.deletebeside a declaredeffect: readhad nothing objecting. Trying the opposite — keeping the grant off the action entirely — turned out to be worse than a style choice:CapabilityFactV1requiresauthority.scopesto equal the semantic authority's, and one of its two builders reconstructs the fact fromrequired_scopes, so the divergence raisedCapabilityFactV1.authority.scopes must project semantic authorityand turned adding the block into an internal error on the base-vs-head path. One resolved list now feeds all three surfaces and the effect evidence. Guarded four ways: the invariant asserted directly across every reviewed-authority shape, a realverify --baserun over a commit that adds the block, and an exhaustive sweep over 294 combinations of (source authority × action declaration × published evidence) showing that no reviewed record at either site can weaken published evidence and still reach an eligible authority dimension — 57 combinations resolve eligible, 107 are weakenings, and the two sets do not intersect.A fallback that could never reach its fallback. A source-wide row's
source_refwas meant to name the file the source is read from rather than the JSON pointer of whichever action happened to build the row — but the chain began with the caller'ssource_ref, which is the issue's own per-action pointer and is always set for this kind. The branch was dead on arrival; found by reading the rendered rows, not the code.Surface discipline
Per
CONTRIBUTING.md§ Surface discipline:count-of-human-inputs-to-verdict(48 → ≤ 7).next_action.kind, no contract bump. One manifest field, plus the additive report-schema bump and the packet/verifier bumps that follow from embedding the same rows; everything else extendsscan,verify, and the existing questionnaire.Schemas
Report
0.37 → 0.38(subject_kindonevidence_gaps[];subject_kind/answer_pathondeclaration_questions.open_questions[]). Packet0.14 → 0.15and verifier0.11 → 0.12follow because they embed the same rows. Prior versions keep their published bytes, are pinned by hash, and are read forward defaulting to the action-scoped reading — which is exactly what those builds could produce. The safety-qualification gate'srequired_report_schema_versionmoves with them.The published JSON Schema deliberately does not encode the mode co-requirements, matching what
ActionAuthorityConfigalready does; the runtime validator is the authority at both sites, and expressing one rule as two different JSON Schema shapes is the second-implementation trap this PR spent its effort removing.Not in this PR
Section F of increment 3 (policy packs as the rules layer, pack selection at
init) is not here. It reshapesSHIP-POLICY-APPROVAL-MISSINGinto per-pack statements, which touches shipped check ids and deserves its own review; C is independently shippable and is the half that moves the activation metric.Review round — four findings, all addressed
[P1] Bind authority to configured-source provenance, not raw ids. The join was wrong in both directions, and the reviewer's reproduction is now a test.
Tool.source_idis minted by the adapter and configured ids share that namespace: atool_sourcesrow of typemcpcalling itselfopenai_apihad its reviewed authority applied to the OpenAI API surface — clearingmissing_authority_evidencefor actions nobody declared anything about — while acodex_configrow, whose adapter emits ids derived from the file it read, applied to nothing at all. The dispatcher now records which configured entry each loaded result was produced for — exactly in pass 1, where it holds the config object; by the(adapter type, minted id)pair in pass 2, where the four manifest-only adapters are rejected intool_sourcesoutright and so cannot collide. Identity resolution carries it onto the canonical action as a list, so a reviewedtool_identitybinding merging observations from two configured sources answers for neither: their credentials are separate facts, and the question stays on the action row. Verified that a per-scan framework source (google_adk, the RFC's motivating case) still resolves through its configured entry.[P1] Do not erase a published credential mode by omission.
credential_modeis optional, and a declaration leaving it out was overwriting a publishedservice_accountwith nothing —declared, pass-eligible, and capability policies matchingcredential_modes: [service_account]silently no longer matching. The published value is preserved where the declaration states none; a different stated value is still a conflict. Pinned by a test that exercises the assessment and the policy-visible value.[P2] Reject credential metadata for
mode: none.credential_modenow reaches the shared validator, and both sites reject{mode: none, credential_mode: …}— a fact about a credential the same block says does not exist.[P2] Keep public schema-version statements synchronized. Fixed the label/link splits in
AGENTS.md,STABILITY.md,docs/{overview,architecture,report-reading-for-agents,passed-verdict-contract}.md, including the frozen lists. The parity test the reviewer asked for is two rules: a line that links the current schema must also name its version, and a quoted<kind>_schema_versionmust equal what the engine emits unless the line or its section marks it as history. Both are section-aware rather than allowlisted. They immediately found averifier_schema_version: "0.7"inREADME.mdand the Claude Code skill that had been stale for several releases — fixed too.Each fix was verified by perturbation: reverting it makes the new tests fail (3, 1, 1, and 1 failures respectively).
#415 landed on
mainduring the review and touches the same loading and identity modules; it is merged in here. The only textual conflict was an import line, but the semantic overlap is real, so the suite was re-run against the merge rather than against the pre-merge tree. One thing the merge surfaced:tool_identity.pyhad a local namedconfigured_source_idsholding the ids adapters minted, which is what a binding-member selector names — the provenance fix gives that exact spelling a different and more precise meaning, and the two sets diverge wherever an adapter derives its own ids. The local isloaded_source_idsnow.Found in passing, not fixed here
verify --basealready fails withinternal_erroronmainfor a manifest that listsactions[].scopeswithout also declaring that action'sauthority: the same two fields diverge, and the sameCapabilityFactV1validator raises. Reproduced on a cleanorigin/mainworktree, so it is neither caused nor widened by this change — under this PR the source route keeps the two in agreement. It needs its own fix and its own decision about which of the two fields should move, so it is not bundled here.🤖 Generated with Claude Code