From 9f325c3b23afd1d6c5536c39c159377cde327ab2 Mon Sep 17 00:00:00 2001 From: Pengfei Hu Date: Fri, 25 Sep 2026 14:43:27 -0700 Subject: [PATCH 1/2] fix: compare application wiring past unrelated links and submodules `diff --application` at the root scope took the unscoped archive route, which refuses every symlink and gitlink. On 2026-09-25, 15 of the first 27 runs against open third-party SDK/ADK PRs exited 2 on a path the reader never opens: `CLAUDE.md -> AGENTS.md`, a linked skill directory, a `VERSION` link leaving the tree, a vendored submodule. - Materialize every scope, the root included, through the scoped verified materializer, which recreates links rather than refusing them and packs the tree instead of the history. - Never read a Python input through a link: a target this scope already reads is compared at its own path, and any other in-scope target is a gap over the link's path. Reading the alias made one agent two ambiguous ones and hid the real file's change. - Record gitlinks behind an opt-in `archive_tree(record_gitlinks=True)`, materialized as the empty directory an unpopulated checkout leaves. An unchanged gitlink commit is named in limits; any other is a coverage gap over its path. Other archive callers still refuse. - Stop turning the host-configuration census's link count into application coverage gaps. - Read `google.adk.Agent`, the package-root re-export, as an ADK agent constructor (`from google.adk import Agent`). Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 6 + docs/application-comparison.md | 34 ++- docs/distribution-surfaces.md | 2 +- src/agents_shipgate/cli/application_diff.py | 108 +++++++-- src/agents_shipgate/cli/verify/git.py | 40 +++- src/agents_shipgate/inputs/google_adk.py | 2 + tests/test_adapter_static_only.py | 4 +- tests/test_application_diff.py | 16 +- tests/test_application_diff_reach.py | 249 ++++++++++++++++++++ 9 files changed, 428 insertions(+), 33 deletions(-) create mode 100644 tests/test_application_diff_reach.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 35db9261..4fa8bf60 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ ### Application comparison without prior setup - Add `diff --application` for OpenAI Agents SDK and Google ADK source-observed per-agent wiring, with exact base/head refs and independently selected scopes. No manifest, saved baseline or authored declarations are needed. The advisory `application_comparison_schema_version: "0.1"` result records before/after evidence, scoped coverage gaps and explicit uncertain candidates; it supplies no release verdict or merge permission. Includes scoped Git materialization, partial-clone recovery, and definition lookup using reader-resolved Python symbols and locations. See [application comparison](docs/application-comparison.md). (#871) +- `diff --application` no longer refuses a repository over a link or submodule its application reader never opens, and reads a Google ADK agent imported from the package root. (#871 follow-up, measured for #868) + - **The problem.** On 2026-09-25, against open third-party pull requests that edit SDK/ADK tool wiring, 15 of the first 27 runs at the default root scope exited 2 with `Git tree contains unsupported external binding`. The path named was never application source: `CLAUDE.md -> AGENTS.md` (dlt-hub/dlt#4417, jaegertracing/jaeger#9636, omnigent-ai/omnigent#6611, wandb/weave#7948), a linked `.claude/skills/…` or `.agents/skills/…` directory (asterinas#3834, vllm#57322, kagent-dev/kagent#2788), `agent/VERSION -> ../../VERSION` (TencentCloud/CubeSandbox#1508), `.pylintrc` (tensorflow#128063), and a vendored submodule (temporalio/sdk-python#1868). The root scope took the unscoped archive route, which refuses every link and gitlink. `from google.adk import Agent` was not read as an agent, so CubeSandbox#1508's `root_agent` gave `not_established` ("No supported application agents were established"). + - **Links.** The root scope now uses the scoped materializer a named scope already used, which recreates each link as a link. Python discovery never walks through a link, in a checkout or here, so a link that is not a Python input changes nothing. A Python file that is a link is never read through: when its target is a Python input the scope already reads, that file is compared at its own path (reading the alias as well made one agent two ambiguous ones and hid the real file's change, as a named scope did on main); any other in-scope target is a coverage gap over the link's own path. A scope that is itself a link is still refused. The host-configuration census, which counts every link that could conceal a *host* path, no longer adds application coverage gaps. + - **Submodules.** A gitlink under the scope is materialized as the empty directory a checkout without `--recurse-submodules` leaves; its content is never fetched. The same gitlink commit on both sides is named in `limits` as unchanged and does not make the comparison `partial`, since identical content cannot carry a change. An added, removed or moved gitlink is a coverage gap over its path on each side that has it, so the result is `partial`, never `compared`. A gitlink was refused (exit 2) before. + - **Google ADK.** `google.adk.Agent`, the package-root re-export of `google.adk.agents.llm_agent.Agent`, is read as an agent constructor by the ADK reader, for `from google.adk import Agent` and `import google.adk as adk` alike. CubeSandbox#1508 now establishes `cube_code_agent` and is `partial`, naming the tool it imports from a sibling module (#864), instead of `not_established`. + - **Re-measured.** At the root scope, main `a430e81a` refused CubeSandbox#1508, dlt#4417 and asterinas#3834; this change compares each, with its own limits (a Python census past `--max-python-files`, an unsupported framework, an unresolved import). No application comparison schema change; `application_comparison_schema_version` stays `"0.1"`. ### Changes diff --git a/docs/application-comparison.md b/docs/application-comparison.md index 527cfa0d..599474cc 100644 --- a/docs/application-comparison.md +++ b/docs/application-comparison.md @@ -57,7 +57,9 @@ requested and compared refs/tree IDs, per-side scope/coverage, rows, source correspondence, and a deterministic `comparison_id`. This is a separate advisory artifact from the existing host diff JSON and verifier receipt. -- `compared`: the selected supported source observations were compared. +- `compared`: the selected supported source observations were compared. An + unchanged submodule may still be named in `limits` (see below); it cannot + carry a change, so it does not make the comparison `partial`. - `partial`: a parse/discovery/binding gap remains. Gaps identify their source and agent where known. Only affected candidates become `change: not_established` rows, carrying `candidate_change` and per-side `uncertainty`; independent known @@ -91,7 +93,35 @@ object with that key removed, encoded as UTF-8 JSON with sorted keys, `separators=(",", ":")` and `ensure_ascii=True`; readers can recompute it. Each side materializes only its selected scope through the existing verified -Git materializer, which retains symlinks and containment checks. Partial clones +Git materializer, which retains symlinks and containment checks. The default +root scope goes through the same scoped materializer, so it packs the tree +rather than the history. + +### Links and submodules + +A symlink anywhere in the tree is recreated as the link it is, never refused +and never followed by the comparison. Python discovery does not walk through a +link, here or in a checkout, so a link that is not a Python input changes +nothing: `CLAUDE.md -> AGENTS.md`, a linked `.claude/skills/…` directory, or a +`VERSION` link that leaves the repository reads as it would without the link. +A Python file that is a link is never read through. When its target is a +Python input the scope already reads, that file is compared at its own path; +a link whose target leaves the scope is outside discovery, as in a checkout; +any other target is a coverage gap over the link's own path +(`Linked Python input: tools.py`). A selected scope that is itself a link is +refused (exit 2). + +A submodule's content is in another repository. The comparison never fetches +it and materializes its gitlink as the empty directory a checkout without +`--recurse-submodules` leaves. The same gitlink commit on both sides is the +same content, so it is named in both sides' `limits` — +`Submodule content is not read (unchanged commit 85b71d7ecd4f): vendor/core` — +and nothing more. An added, removed or moved gitlink is a coverage gap over its +path on each side that has it, so the comparison is `partial`, and a binding +the other side holds at that path is `not_established` rather than added or +removed. + +Partial clones with unfetched objects exit 2 with `objects_missing`, name the affected side, and provide the existing `git fetch --refetch --no-filter ` recovery. The comparison never runs that fetch. Other configuration/materialization errors diff --git a/docs/distribution-surfaces.md b/docs/distribution-surfaces.md index 73ee6e7e..df12123e 100644 --- a/docs/distribution-surfaces.md +++ b/docs/distribution-surfaces.md @@ -75,7 +75,7 @@ and this document are checked against each other by | `human_review_decision` | `docs/human-review-decision.md` | `release_decision_vocabulary` | `test_surface_enumerations_match_the_engine_vocabulary` | Host-neutral read-only evaluator; no GitHub acquisition, persistence or operation authority. | | `github_action` | `action.yml`, `scripts/github_action_outputs.py` | `merge_verdict_vocabulary` | `test_action_input_enumerates_engine_merge_verdicts`, `test_action_output_script_shares_the_engine_merge_verdicts` | The paired `shipgate_wheel`/`shipgate_wheel_sha256` inputs install a caller-supplied local wheel instead of a published version, so that route names no channel and claims no `executable_pin`; it is refused unless both halves are given, and it installs `--no-deps`. `tests/test_action_engine_install.py` proves the refusals. Every `python` the Action starts in the workspace runs with `-P` or as a script path, so a pull request's `pip/` or `agents_shipgate/` package cannot stand in for pip or the engine; the same file executes the install and merge-verdict steps against such a checkout. The `v1.0.0` tag predates that fix; the published `v1.1.0` carries it. | | `capability_diff` | `src/agents_shipgate/cli/diff.py`, `src/agents_shipgate/core/capability_diff_rows.py`, `src/agents_shipgate/core/host_comparison.py`, `src/agents_shipgate/report/host_comparison.py`, `src/agents_shipgate/core/unread_inputs.py`, `src/agents_shipgate/cli/verify/changed_inputs.py` | — | — | Answers no question the engine answers: it emits no verdict, no release decision and no pin. Every field is read from the drift payload the engine already produces — `risk` is the engine's severity and `expansion_signals` is the engine's word on widening — so there is no second implementation to drift. A `permission_mode` or `sandbox` row names the setting and its value as the file spells it (`enableAllProjectMcpServers: true`, `defaultMode: dontAsk`), recovered from the grant's published value and digest, and a Claude Code setting's `why` is the basis the engine's one setting table (`core/host_settings.py`) records for the value; that table also rates the grant and `check`'s violation, so a row's severity and the violation's risk give one answer (#827, `tests/test_prompt_disabling_settings.py`). `verify`/PR and `check` reuse the host comparator (#684, `tests/test_manifest_free_pr_rows.py`), and the source name each named reusable-workflow secret refers to, also non-widening, with a redacting name or target refused rather than compared, and an unreadable value neither compared nor named on this surface — only the host inventory and `audit --host` name its `job/destination`, as on `1.0.0` (#693, `tests/test_reusable_workflow_secret_mappings.py`); check retains argument redaction and its existing local-policy control. Missing comparison evidence never supplies empty comparable rows. Default host mode only (`--application` is registered separately below); workflow rows compare effective writes and reusable secret recipients (#685, `tests/test_workflow_capability_diff.py`) and each job's remote step action references, as a non-widening change (#771, `tests/test_workflow_step_action_references.py`); and each job's agent launches — a documented agent action's permission inputs, the permission flags of a `run:` that is one plain `claude -p` / `codex exec` command — and checkout refs, compared as text, as a change unless the job gains a documented widening rule, which the engine names in `expansion_signals` (`workflow_agent_widened_*`) — a rule read only from text the engine reads exactly (no shell is parsed; an argument input that is not a plain list of words is compared by a digest and read for no rule), and a gain the engine does not claim (a rule moved in from a job the launch left, one an unread step of the job rewritten as a read launch may already have met, or one the job's launch held before in an expression or an unread argument input) named in the `why` from the same engine function, never counted — with a note on a workflow row naming the untrusted-input trigger, write scopes, secrets and pull request checkout beside each agent step, read off the grant the engine published and moving no direction; an unread `run:` agent step (never compared, so never a row), an unreadable value or a setting published redacted is named only by the host inventory and `audit --host`, as for an unread secret value, an unread argument input or an unresolved launch is named there and in the `why` of a row reporting its launch, and a checkout ref holding credential-shaped text is refused as a redacting step reference is (#823, `tests/test_workflow_agent_launches.py`); every job id, step label, trigger and scope name those rows print is the label the engine published once where it built the grant, redacted, never re-derived here; `check`'s workflow evidence is derived from the raw declarations, which it still compares, and redacts job and scope names by the same rule; two distinct job ids or triggers in one workflow, or scope names in one `permissions` mapping, that publish alike are refused rather than compared, so while such a workflow exists `check` refuses on every run even when it is unchanged (#802, `tests/test_workflow_label_redaction.py`); artifact-only edits remain separate evidence. Tool-source subjects are #655. Where a partial or experimental surface is byte-identical on both sides, `diff` and `verify` compare the rest and name it in `unchanged_limits`; `check` keeps refusing, because its boundary result cannot carry a limit yet (#721). A surface the reader reaches through an in-tree link it reads through qualifies only when that link, a link with the same text at each link on the way, and the file it lands on, the same blob at the same path, are both unchanged, read from the base's Git tree entries against a commit's or, without following any link, the working tree's; any change to either is treated as before. The same proof decides which shared plugin-reference limits `check` leaves out, so behind such a link `check` compares, and publishes the rows it finds, exactly as for a limit at its own path, and a comparison `partial` only because of such a limit is `comparable` with it in `unchanged_limits` (#822, `tests/test_linked_unchanged_limits.py`). A hook row's `why` states the grant's loading basis, read from its published `source`, `access` and `risk` by the engine's `hook_loading_basis`; only a hook the host loads for this project earns an expansion signal — one a settings layer declares, or one a plugin selects that the repository's project settings enable from an in-repository marketplace — so a declared-only hook, or one a plugin selects without that enablement, is a row and never an expansion, and a removal names no basis (#714). `check` compares without a plugin-reference limit both sides share on an untouched source, which it cannot name and, untouched, does not route; a limit only one side carries makes its comparison incomparable. Those rows are not what routes a change: `check`, and the boundary check a manifest-backed `verify` runs, route a changed hook declaration of a plugin the project settings enable through the existing protected-surface rule, from the plugin hook reader's selection on both compared sides, and count a changed hook file such a plugin selects that the reader does not open as incomplete input; the rows beside either are unchanged (#809, `tests/test_enabled_plugin_hook_routing.py`). A partial clone that never fetched the base's objects is refused as `objects_missing`, exit `2`, never compared and never fetched; the refusal ends with the remediation sentence `verify` reports for the same reason, produced by the same function (#817, `tests/test_capability_diff_partial_clone.py`). The text of `diff`, `verify`, the PR comment and `check` reads the rows through one function, `review_changes`, and adds no row and changes no row value in any JSON projection (#795, `tests/test_host_diff_review_changes.py`): a permission rule is named with its disposition; an MCP server with the command name (never its path) or redacted URL, its package and argument digest (#819) and the env and header key names its grant already publishes, a URL printing only in the engine's sanitized scheme-and-host form and otherwise as `url not shown`, or, when none of those differ, a sentence naming what was compared and that the change is in a detail not shown, such as the command's path or another setting; a hook with each handler field that changed — its group's matcher, its command as its executable's name and digest, its timeout — before and after, a handler only one side declares, or the published handlers in a different order with a detail not shown that may also differ, and past the handler bound the same kind of sentence naming a handler past it, all read from the handlers its host-grants `0.7` grant publishes, which hold no command or argument text, and never re-derived here, and a declaration outside the documented hooks shape named as not shown rather than guessed (#819, `tests/test_hook_mcp_detail_fields.py`); those hook and MCP members display what `config_sha256` already binds, so grant equality and every inventory digest leave them out, a saved baseline holds none of them, and no row, row value, reason, digest or control answer moves; the PR comment gives the lines 1.1.0 printed their room first, the coverage block included, and prints an entry whole when the whole comment fits, otherwise cut to the widest length of at least 60 characters at which it does, or else in its shortest form (a difference cut after its name, an added or removed grant as its row), never longer than the entry 1.1.0 printed, with one line naming `verifier.json`, so no long entry hides a row, the coverage block, the change count, the review question, the reproduction or the advisory that 1.1.0 kept (#819 review, cycles 4 and 6); an allow rule the permission lattice decided another replaced (`widened` or `narrowed`), or the exact rule text that moved between dispositions in one host and source (`moved`), is one entry, never on the routes that redact rule arguments; and `diff` counts entries `from N rows` when one joins rows. Comparable results with entries end with one review question, naming the row count when an entry joins rows, and every result whose comparison names a base commit and a commit or working-tree head — a zero-row result and a refusal included (#812 follow-up, `tests/test_host_comparison_coverage.py`) — ends with the compared commits, the tool version and an `agents-shipgate diff --base ` reproduction, labelled `Inputs:` rather than `Compared:` where the comparison was refused, since that run compared nothing — and a refused comparison publishes no `review` object at all, so those two lines are the only place that run states its provenance, built from the `base_commit` it publishes beside the refusal; `check` and a provided diff print the question alone, and no result without a change asks a question. Every one of those facts is published beside the rows, so a machine consumer reads what a human reads (#795 slice 2, same test file): a row adds `disposition`, the `allow`/`ask`/`deny` list a permission rule is declared under and `null` for any other kind, on every route that publishes rows; and `review` in `diff --json` (capability diff `0.3`) and `host_comparison.review` in `verifier.json` (verifier `0.20`) — one object for one comparison — carry the presented changes, each naming the `row_indexes` it stands for, the `direction` the text uses (`widened`, `narrowed` and `moved` included, which no single row can carry), its cells, its `why` and one `expands`, plus a `summary` of `{rows, changes, widenings}` equal to `diff`'s summary line, the review question and the reproduction command. The block is refused unless its changes stand for every published row exactly once, its counters match and no joined change's two sides read alike, so the routes that redact rule arguments publish their rows alone and never a pair that reads `X → X`; `check`'s boundary result carries rows, with their dispositions, and no block. It is presentation, not a second opinion: it is the one `review_changes` projection the text prints, so the rows, their values, their count and every control answer are what they were. A comparison read back from JSON prints the changes it published, and one whose rows a caller sliced falls back to those rows. Each comparison also says what it established (#812, `tests/test_host_comparison_coverage.py`): `coverage` in `diff --json` (capability diff `0.3`) and `host_comparison.coverage` in `verifier.json` (verifier `0.20`) are the same object, printed as `What this run established` by `diff`, `verify` text and the PR comment. It is read off the grant changes, artifact changes, observed sources and blocking issues the comparator already computed: a file's rows, counting a source inside it (`#profiles.`, `#plugins.`); a file with no row and no artifact change called unchanged (`compared`, `0` rows) only when Git proves its blob identical, as the check `unchanged_limits` uses does, asked privately in one bounded batch and never published, because the artifact digest redacts `env` values and `apiKeyHelper`; a file that changed with no compared grant moving (`changed_without_grant_change`), whose artifact differs only in its digest or whose content Git shows differs while its artifact did not (never a difference a checkout line-ending conversion or a converting attribute explains, and no filter is run), never a plugin manifest or marketplace, a retargeted link or project settings while a hook's loading basis moved, worded as no compared grant changing and never as which fields changed; any other changed file with no row (`changed_without_rows`); a file Git neither proves identical nor shows differs — a provided diff, a link read, a redacted path, a working-tree file a checkout wrote with `CRLF` that Git reports unchanged — as `unchanged_not_proven`, never no change and never a change (#812 review cycle 3); the side that published a source, worded `published by` rather than `read in` for a plugin manifest or marketplace, which is published only while it declares hooks; and on a refused comparison each blocking source and its kind. Outside the bounded candidate rules below, a file no inventory observed is never an item and its absence is no claim, which the block states where it is read — one line under the heading and `read_sources_only` in the JSON — so a true list cannot be taken for the account of the change (#812 follow-up); a source already in `unchanged_limits` is not repeated; the list is capped at ten with `omitted_items`, ordered so what no row shows precedes a file's rows and, among blocking limits, by kind (`unreadable`, `parse_failed`, `unresolved_precedence`, then `unsupported`, `dynamic_source_excluded`, `remote_source_excluded`) — order, not severity, and a ranking of kinds rather than of items, since `unsupported` carries both a file this entry merely does not accept and one whose own text would not parse, so an item behind the count may still be one to repair; total down to every field an item is keyed by, the source name and then its side, limit and status — and counted in text as items not listed, ranked below those listed, and the PR comment lists only what fits in the room its entries, review question, reproduction, advisory, next action and evidence leave, at most 2000 characters, so the block never pushes out a line the comment prints without it (a row list that fills the comment by itself still truncates it, as on `1.0.0`); an instruction file's line carries no redacted-values note; sources are the inventory's redacted paths; `null` means not recorded, which is how a `0.19` verifier reads. It moves no row, reason, digest, baseline, control state or next action, and `check`'s boundary result and text carry none, so neither `check` nor a provided diff asks Git anything for it. The same list names the changed inputs this entry does not read (#821, `tests/test_unread_changed_inputs.py`): capability diff `0.4` and verifier `0.21` add a `changed_not_read` item, with the `candidate` rule that named it, for each path in the comparison's own changed-file set — the committed change, or the working tree's tracked and untracked changes — that a bounded, documented rule set recognises as plausibly agent configuration (`mcp.json` in a plugin directory, a plugin manifest's `mcpServers`, a Codex, Cursor or Copilot manifest's `hooks` and the hook files it names, a manifest or marketplace that does not parse, `.cursor/hooks.json`, host settings below the repository root, a marketplace entry's external `source`) and that no inventory published; a member is named whatever read its file, because no reader reads it. It is named from the path and, for a manifest or marketplace member, its text: nothing is fetched, run or read as a grant, so it is never a row, a widening, a `check` violation or a loading claim, and an external source is described redacted and never fetched. It ranks right after the blocking limits, inside the same cap; `read_sources_only` is `false` while one is named, and the first line says so instead; `unread_candidates` and `unread_candidates_not_examined` say whether the change set was examined and how many candidates were not — past the bound of 32, or because a file the rule needed was not read or did not parse, one count the text names both causes of. A manifest-free `verify` whose only host-relevant change is such an input, or a changed candidate it counts as not examined, publishes the comparison instead of the setup route, and `verify --preview` then names `audit --host` instead of `init --write`, in an agent-related workspace too; a `0.20` verifier reads with the search not recorded. A comparison refused only by plugin-reference limits, each bounded by its plugin directory, that no compared source depends on, is `partial` instead (#808, `tests/test_partial_host_comparison.py`); any other blocking limit it carries must be one both sides share on an unchanged source, named in `unchanged_limits` as on a comparable result. Capability diff `0.4` and verifier `0.21` publish `comparison_status: partial` with the refusal's `incomparable_reasons`, the rows, review and unchanged limits established outside those directories, and each directory (the outermost, where one holds another) as the reserved `coverage.items[].scope` on the `blocking_limit` items it bounds, and never call a changed project settings file without a row `changed_without_grant_change`, since the hooks whose loading basis it decides are not all compared; `diff`, `verify` text and the PR comment lead with `Partial comparison against …` or `Host capability comparison partial: …` and `Not compared: , …` before any entry, and a partial result with no entry is never printed as no change. Independence is read off the reader's reference graph, never off directory names: any other limit that is not unchanged, a reference leaving its plugin, a plugin at the root or holding project settings, a marketplace elsewhere declaring inline hooks for it, or a directory that does not publish as itself refuses as before. It answers no engine question and moves no control: a partial comparison is not comparable, `verify`'s control and route are the refusal's, the control envelope projects it as `incomparable` with no rows, and `check`, whose boundary result cannot name a directory, refuses its comparison and decides exactly as before. A `0.20` verifier claiming a partial comparison or a scope is refused. | -| `application_diff` | `src/agents_shipgate/cli/application_diff.py` | — | — | Advisory SDK/ADK source-wiring comparison through `diff --application`. Reuses framework observations and the binding graph; its comparator answers no release or merge verdict, activation verdict, executable pin, or declared authority question. Unlike host mode it computes per-agent source differences, not a projection of host drift. Scoped gaps and `not_established` candidates bound the answer; no deployment-root reachability is asserted. `tests/test_application_diff.py` and `tests/test_application_diff_review.py` prove the advisory boundary, isolation, uncertainty and evidence identity. | +| `application_diff` | `src/agents_shipgate/cli/application_diff.py` | — | — | Advisory SDK/ADK source-wiring comparison through `diff --application`. Reuses framework observations and the binding graph; its comparator answers no release or merge verdict, activation verdict, executable pin, or declared authority question. Unlike host mode it computes per-agent source differences, not a projection of host drift. Scoped gaps and `not_established` candidates bound the answer; no deployment-root reachability is asserted. `tests/test_application_diff.py` and `tests/test_application_diff_review.py` prove the advisory boundary, isolation, uncertainty and evidence identity. Every scope, the root included, is materialized by the scoped verified materializer, so a link is recreated rather than refused, a link that is not a Python input changes nothing, and a Python file that is a link is never read through; a submodule is never read, named as a limit when its gitlink commit is unchanged and a coverage gap over its path otherwise; the host-configuration census adds no application gap (`tests/test_application_diff_reach.py`). | | `zero_install_detector` | `tools/shipgate-detect.py` | `agent_project_verdict` | `test_detector_verdict_matches_cli` | Emits no `diagnostics[]` and no `next_actions[]`; evidence strings and framework scores are simplified. See the script's own "Intentional simplifications". | | `emitted_ci_workflow` | `src/agents_shipgate/cli/discovery/ci_workflow.py` | `executable_pin` | `tests/test_adopter_pins_resolve.py::test_the_emitted_workflow_pins_the_release_and_not_the_source_tree`, `tests/test_release_source.py::test_candidate_workflow_uses_immutable_source_before_and_after_publication` | Ordinary/source/preview builds use the published fallback; a stamped candidate pins its verified Action SHA and package version. Before publication its smoke substitutes the exact local wheel inputs. Provenance asserts no qualification. | | `prompts` | `prompts/` | `contract_floor`, `executable_pin`, `placeholder_ownership`, `release_decision_vocabulary` | `test_executable_pin_resolves_in_a_published_channel`, `test_surface_enumerations_match_the_engine_vocabulary`, `test_surface_routes_human_owned_placeholders_to_a_human`, `tests/test_adopter_pins_resolve.py::test_every_pin_init_writes_into_an_adopter_repo_names_the_published_release`, `tests/test_adopter_pins_resolve.py::test_the_shipped_floor_is_decided_against_the_release_the_prompts_pin` | — | diff --git a/src/agents_shipgate/cli/application_diff.py b/src/agents_shipgate/cli/application_diff.py index ad9d8e37..73864322 100644 --- a/src/agents_shipgate/cli/application_diff.py +++ b/src/agents_shipgate/cli/application_diff.py @@ -63,6 +63,8 @@ class Observations: limits: list[str] = field(default_factory=list) sources: list[dict[str, str]] = field(default_factory=list) coverage_gaps: list[dict[str, Any]] = field(default_factory=list) + #: Unpopulated submodules under the scope, by scope-relative path. + submodules: dict[str, str] = field(default_factory=dict) def gap( self, @@ -183,8 +185,21 @@ def _definition(root: Path, tool: Any) -> dict[str, Any]: } -def observe(tree: Path, scope: str, *, max_python_files: int) -> Observations: +def observe( + tree: Path, + scope: str, + *, + max_python_files: int, + gitlinks: dict[str, str] | None = None, +) -> Observations: result = Observations(scope) + for path, commit in (gitlinks or {}).items(): + # A gitlink outside the scope arrived with an in-scope link's target, + # which discovery never walks into; it is not this scope's to name. + if scope == ".": + result.submodules[path] = commit + elif PurePosixPath(path).is_relative_to(scope): + result.submodules[PurePosixPath(path).relative_to(scope).as_posix()] = commit root = tree / scope if root.is_symlink(): raise ConfigError(f"Application scope is not a regular directory: {scope}") @@ -207,11 +222,20 @@ def observe(tree: Path, scope: str, *, max_python_files: int) -> Observations: if result.limits: result.status = "partial" return result + # A Python input that is a link is never read through. When its target is a + # Python input this scope already reads, it is compared at its own path, so + # the link adds nothing; reading it again made one agent two ambiguous ones + # and hid that file's changes. Any other target is named as a gap below. + linked = {p.relative_to(root).as_posix(): p for p in python_files if p.is_symlink()} + read_directly = {p.resolve() for p in python_files if not p.is_symlink()} detected = detect_workspace(root, max_python_files=max_python_files) if detected.python_parse_truncated: result.gap(f"Python discovery truncated at {max_python_files} files.") - for path in detected.host_discovery_incomplete_paths: - result.gap(f"Discovery could not read {path}.", source=path) + # `detected.host_discovery_incomplete_paths` is deliberately not a gap. It + # is the host-configuration census, which counts every link that could + # conceal a host path. Python discovery never walks through a link, here + # or in a checkout, so `CLAUDE.md -> AGENTS.md` or a linked skill directory + # hides no application source; a linked Python input is named below. for item in detected.excluded_sources: result.gap(f"Excluded candidate: {item}", source=item.get("path")) # Discovery omits malformed Python; preserve that gap rather than an empty @@ -219,11 +243,10 @@ def observe(tree: Path, scope: str, *, max_python_files: int) -> Observations: if len(python_files) > max_python_files: result.gap(f"Python input census exceeds {max_python_files} files.") for file in python_files[:max_python_files]: - if file.is_symlink(): - result.gap( - f"Linked Python input: {file.relative_to(root)}", - source=file.relative_to(root).as_posix(), - ) + relative = file.relative_to(root).as_posix() + if relative in linked: + if _resolved(file) not in read_directly: + result.gap(f"Linked Python input: {relative}", source=relative) continue try: ast.parse(file.read_bytes()) @@ -240,7 +263,13 @@ def observe(tree: Path, scope: str, *, max_python_files: int) -> Observations: source=path, ) entries = sorted( - {(f.type, p) for f in detected.frameworks if f.type in SUPPORTED for p in f.candidate_files} + { + (f.type, p) + for f in detected.frameworks + if f.type in SUPPORTED + for p in f.candidate_files + if p not in linked + } ) sources = [ ToolSourceConfig(id=f"{kind}:{path}", type=kind, path=path) for kind, path in entries @@ -251,6 +280,13 @@ def observe(tree: Path, scope: str, *, max_python_files: int) -> Observations: return result +def _resolved(path: Path) -> Path | None: + try: + return path.resolve(strict=True) + except (OSError, RuntimeError): + return None + + def _observe_source(result: Observations, root: Path, source: ToolSourceConfig) -> None: """Each reader's unlocated gaps cover its input file, not other sources. @@ -376,6 +412,31 @@ def _observe_source(result: Observations, root: Path, source: ToolSourceConfig) } +def _reconcile_submodules(base: Observations, head: Observations) -> None: + """Name each submodule the comparison did not read, by what it can hide. + + Discovery never reads into a submodule; a checkout without + ``--recurse-submodules`` leaves an empty directory, and so does the + materializer. The same gitlink commit on both sides is the same content, so + it cannot carry a change: it is named as a limit and nothing more. Any + other gitlink is a gap over its own path on each side that has it. + """ + + for path in sorted(base.submodules.keys() | head.submodules.keys()): + before, after = base.submodules.get(path), head.submodules.get(path) + if before == after: + message = f"Submodule content is not read (unchanged commit {before[:12]}): {path}" + base.limits.append(message) + head.limits.append(message) + continue + for side, commit in ((base, before), (head, after)): + if commit is not None: + side.gap( + f"Submodule content is not read (commit {commit[:12]}): {path}", + source=path, + ) + + def _meaning(binding: dict[str, Any]) -> dict[str, Any]: return { k: v @@ -575,30 +636,43 @@ def run_application_diff( engine = build_engine_requirement(plugins_enabled=False).model_dump(mode="json") with tempfile.TemporaryDirectory(prefix="shipgate-application-diff-") as raw: scratch = Path(raw) + gitlinks: dict[str, dict[str, str]] = {} for side, ref, commit, selected_scope in ( ("base", base_ref, base_commit, old_scope), ("head", head, head_commit, scope), ): def in_scope(path: str, selected: str = selected_scope) -> bool: - return path == selected or path.startswith(selected + "/") + return ( + selected == "." or path == selected or path.startswith(selected + "/") + ) + # Always scoped, the root included: the scoped materializer + # recreates links rather than refusing them, so an unrelated + # `CLAUDE.md -> AGENTS.md` meets the reader as the link it is, and + # the reader decides what it would follow. It also packs the tree + # instead of walking history. try: - archive_fetched_tree( - workspace, - commit, - scratch / side, - scope=None if selected_scope == "." else in_scope, + gitlinks[side] = archive_fetched_tree( + workspace, commit, scratch / side, scope=in_scope, record_gitlinks=True ) except PromisedObjectsMissingError: _refuse_objects_missing(workspace, ref, commit, side=side) - old = observe(scratch / "base", old_scope, max_python_files=max_python_files) - new = observe(scratch / "head", scope, max_python_files=max_python_files) + old = observe( + scratch / "base", + old_scope, + max_python_files=max_python_files, + gitlinks=gitlinks["base"], + ) + new = observe( + scratch / "head", scope, max_python_files=max_python_files, gitlinks=gitlinks["head"] + ) if old.status == new.status == "absent": raise ConfigError( f"Neither comparison tree contains the selected scopes: " f"base={old_scope!r}, head={scope!r}. Check --scope/--base-scope." ) + _reconcile_submodules(old, new) moves = _align_exact_moves(workspace, base_commit, head_commit, old, new) rows = compare(old, new, target_moves={m["base_source"]: m["head_source"] for m in moves}) for row in rows: diff --git a/src/agents_shipgate/cli/verify/git.py b/src/agents_shipgate/cli/verify/git.py index cdb13272..d7748e70 100644 --- a/src/agents_shipgate/cli/verify/git.py +++ b/src/agents_shipgate/cli/verify/git.py @@ -2326,7 +2326,8 @@ def archive_tree( destination: Path, *, scope: Callable[[str], bool] | None = None, -) -> None: + record_gitlinks: bool = False, +) -> dict[str, str]: """Materialize exact Git blobs without export-ignore or substitutions. ``scope`` narrows the materialized tree to the paths a reader will @@ -2340,6 +2341,13 @@ def archive_tree( Scoping changes what is *materialized*, never what is *verified*: every blob written is still checked against its object ID, and the isolated store is still fsck'd, on the same terms as an unscoped archive. + + A gitlink (a submodule's commit pointer) in scope is refused unless + ``record_gitlinks`` is set. Its content lives in another repository, so + no archive of this one can hold it. A caller whose reader treats an + unpopulated submodule as the empty directory a checkout leaves may opt in: + the gitlink is then materialized as that empty directory and returned + as ``{path: commit}``, so the caller can name what it did not read. """ destination.mkdir(parents=True, exist_ok=True) @@ -2360,8 +2368,12 @@ def archive_tree( git_dir=git_dir, tree=tree if scope is not None else None, ) - _materialize_isolated_tree( - git_dir, tree=tree, destination=destination, scope=scope + return _materialize_isolated_tree( + git_dir, + tree=tree, + destination=destination, + scope=scope, + record_gitlinks=record_gitlinks, ) @@ -2449,7 +2461,8 @@ def _materialize_isolated_tree( tree: str, destination: Path, scope: Callable[[str], bool] | None = None, -) -> None: + record_gitlinks: bool = False, +) -> dict[str, str]: listing_args = ["ls-tree", "-r", "-z"] if scope is not None: listing_args.append("-t") @@ -2458,6 +2471,7 @@ def _materialize_isolated_tree( entries: list[tuple[str, str, str]] = [] links: list[tuple[str, str]] = [] portable_paths: dict[str, str] = {} + gitlinks: dict[str, str] = {} #: Every entry's object type, in or out of scope, so a link's target type #: can be read from the tree rather than from what was materialized. tree_types: dict[str, str] = {} @@ -2504,6 +2518,16 @@ def _materialize_isolated_tree( raise ConfigError(f"Git tree path escapes destination: {path_text}") target.mkdir(parents=True, exist_ok=True) continue + if record_gitlinks and mode == "160000" and object_type == "commit": + # An unpopulated submodule: the directory a checkout leaves, with + # nothing written into it. What it would hold is not in this + # repository's object graph, and the caller names it as unread. + target = (root / path_text).resolve() + if target == root or root not in target.parents: + raise ConfigError(f"Git tree path escapes destination: {path_text}") + target.mkdir(parents=True, exist_ok=True) + gitlinks[path_text] = oid + continue if object_type != "blob" or mode == "160000" or ( mode == "120000" and scope is None ): @@ -2576,6 +2600,7 @@ def _materialize_isolated_tree( } if materialized != expected_digests: raise ConfigError("Materialized Git tree differs from the verified object graph") + return gitlinks #: How many links one resolution may pass through before it counts as unresolved. @@ -3095,7 +3120,8 @@ def archive_fetched_tree( destination: Path, *, scope: Callable[[str], bool] | None = None, -) -> None: + record_gitlinks: bool = False, +) -> dict[str, str]: """:func:`archive_tree`, naming a partial clone's unfetched objects as such. A blobless clone fails the object copy with a :class:`ConfigError`, and a @@ -3109,7 +3135,9 @@ def archive_fetched_tree( """ try: - archive_tree(workspace, commit, destination, scope=scope) + return archive_tree( + workspace, commit, destination, scope=scope, record_gitlinks=record_gitlinks + ) except (ConfigError, subprocess.CalledProcessError) as exc: if promised_objects_missing(workspace, commit): raise PromisedObjectsMissingError(commit) from exc diff --git a/src/agents_shipgate/inputs/google_adk.py b/src/agents_shipgate/inputs/google_adk.py index 1f8c3705..2c8d43a7 100644 --- a/src/agents_shipgate/inputs/google_adk.py +++ b/src/agents_shipgate/inputs/google_adk.py @@ -46,6 +46,8 @@ AGENT_CLASS_NAMES = { "Agent", "LlmAgent", + # The package root re-exports the agent: ``from google.adk import Agent``. + "google.adk.Agent", "google.adk.agents.Agent", "google.adk.agents.LlmAgent", "google.adk.agents.llm_agent.Agent", diff --git a/tests/test_adapter_static_only.py b/tests/test_adapter_static_only.py index 70990d87..7f7d86ee 100644 --- a/tests/test_adapter_static_only.py +++ b/tests/test_adapter_static_only.py @@ -257,7 +257,7 @@ class AllowedException: AllowedException( relative_path="cli/verify/git.py", surface="attr_call:subprocess.Popen", - line=2872, + line=2897, snippet=( "subprocess.Popen(cmd, env=env, stderr=subprocess.PIPE, " "stdin=subprocess.PIPE if input is not None else " @@ -280,7 +280,7 @@ class AllowedException: AllowedException( relative_path="cli/verify/git.py", surface="attr_call:subprocess.run", - line=3276, + line=3304, snippet=( "subprocess.run(cmd, capture_output=capture_output, check=check, " "env=env, input=input, stderr=stderr, stdin=stdin, stdout=stdout, " diff --git a/tests/test_application_diff.py b/tests/test_application_diff.py index 9878bf5f..212712c7 100644 --- a/tests/test_application_diff.py +++ b/tests/test_application_diff.py @@ -226,15 +226,21 @@ def test_application_code_is_never_executed(repo): assert not marker.exists() -def test_gitlink_refusal_is_not_a_successful_comparison(repo): +def test_gitlink_is_not_a_successful_comparison(repo): + # The submodule's content is in another repository. It is named as an + # unread gap over its own path rather than refusing the comparison, and + # never read as an empty directory that proves no change (#871 follow-up). base = commit(repo, {'agent.py': SDK.replace('TOOLS', '[]')}) git(repo, 'update-index', '--add', '--cacheinfo', f'160000,{base},external') git(repo, '-c', 'user.name=Test', '-c', 'user.email=test@example.com', '-c', 'commit.gpgsign=false', 'commit', '-qm', 'gitlink') - result = CliRunner().invoke(app, ['diff', '--application', '--workspace', str(repo), - '--base', base, '--head', 'HEAD', '--json']) - assert result.exit_code == 2 - assert '160000' in result.output + result = run(repo, base, 'HEAD') + assert result['comparison_status'] == 'partial' + assert [(g['source'], g['reason']) for g in result['head']['coverage_gaps']] == [ + ('external', f'Submodule content is not read (commit {base[:12]}): external')] + text = CliRunner().invoke(app, ['diff', '--application', '--workspace', str(repo), + '--base', base, '--head', 'HEAD']) + assert 'not a no-change result' in text.output def test_python_size_bound_precedes_discovery(repo, monkeypatch): diff --git a/tests/test_application_diff_reach.py b/tests/test_application_diff_reach.py new file mode 100644 index 00000000..06738fe8 --- /dev/null +++ b/tests/test_application_diff_reach.py @@ -0,0 +1,249 @@ +"""Reach regressions: inputs the application reader never follows cannot refuse it. + +Measured on 134 open third-party SDK/ADK PRs (2026-09-25). At the default +root scope, 15 of the first 27 exited 2 on a link or submodule unrelated to the +application: ``CLAUDE.md -> AGENTS.md``, a linked skill directory, a ``VERSION`` +link that leaves the tree, a vendored gitlink. ``from google.adk import Agent`` +read as "no supported agents". +""" + +import os + +import pytest +from test_application_diff import SDK, commit, git, run +from test_application_diff import repo as repo + +LINKS = { + # dlt-hub/dlt#4417, jaegertracing/jaeger#9636, omnigent-ai/omnigent#6611 + "in_tree_file": ("CLAUDE.md", "AGENTS.md"), + # asterinas#3834, vllm#57322, kagent-dev/kagent#2788 + "in_tree_directory": (".claude/skills/review", "../../.agents/skills/review"), + # TencentCloud/CubeSandbox#1508: agent/VERSION -> ../../VERSION + "leaves_tree": ("agent/VERSION", "../../VERSION"), + "absolute": (".pylintrc", "/etc/pylintrc"), + "dangling": ("docs/latest", "missing/latest"), +} + + +def link(repo, path, target): + (repo / path).parent.mkdir(parents=True, exist_ok=True) + os.symlink(target, repo / path) + git(repo, "add", path) + + +def gitlink(repo, path, commit_id): + # An unpopulated submodule's empty directory keeps `add -A` from staging + # the gitlink's deletion. + (repo / path).mkdir(parents=True, exist_ok=True) + git(repo, "update-index", "--add", "--cacheinfo", f"160000,{commit_id},{path}") + + +@pytest.mark.parametrize("shape", sorted(LINKS)) +def test_root_scope_reads_past_a_link_python_discovery_never_follows(repo, shape): + path, target = LINKS[shape] + commit(repo, {"AGENTS.md": "# notes\n", ".agents/skills/review/SKILL.md": "# review\n"}) + link(repo, path, target) + base = commit(repo, {"agent.py": SDK.replace("TOOLS", "[lookup]")}) + head = commit(repo, {"agent.py": SDK.replace("TOOLS", "[lookup, execute]")}) + result = run(repo, base, head) + assert result["comparison_status"] == "compared", result["head"]["limits"] + assert [(r["agent"], r["tool"], r["change"]) for r in result["rows"]] == [ + ("agent", "execute", "added") + ] + assert result["base"]["limits"] == result["head"]["limits"] == [] + + +def test_python_link_to_an_input_already_read_is_not_read_twice(repo): + # Read through, the alias made one agent two ambiguous ones and hid the + # real file's change: `partial` with no row, as a narrow scope on main did. + source = SDK.replace("agent = Agent", "helper = Agent").replace('name="assistant"', 'name="helper"') + commit(repo, {"real/helper.py": source.replace("TOOLS", "[lookup]")}) + link(repo, "helper.py", "real/helper.py") + base = commit(repo, {}) + head = commit(repo, {"real/helper.py": source.replace("TOOLS", "[lookup, execute]")}) + result = run(repo, base, head) + assert result["comparison_status"] == "compared", result["head"]["limits"] + assert [(r["agent_source"], r["tool"], r["change"]) for r in result["rows"]] == [ + ("real/helper.py", "execute", "added") + ] + assert result["head"]["sources"] == [{"type": "openai_agents_sdk", "path": "real/helper.py"}] + + +@pytest.mark.parametrize("target", ["leaves_scope", "not_python"]) +def test_python_link_to_anything_else_is_never_read_through(repo, target): + # Discovery drops a link whose target leaves the scope, in a checkout as + # here. One that stays in scope but lands on something this scope does not + # read as Python is a gap over the link's own path, and only that path. + if target == "leaves_scope": + commit(repo, {"shared/tools.py": SDK.replace("TOOLS", "[lookup]")}) + link(repo, "app/tools.py", "../shared/tools.py") + else: + commit(repo, {"app/tools_impl": SDK.replace("TOOLS", "[lookup]")}) + link(repo, "app/tools.py", "tools_impl") + base = commit(repo, {"app/agent.py": SDK.replace("TOOLS", "[lookup]")}) + head = commit(repo, {"app/agent.py": SDK.replace("TOOLS", "[lookup, execute]")}) + result = run(repo, base, head, "--scope", "app") + assert [(r["agent_source"], r["tool"], r["change"]) for r in result["rows"]] == [ + ("agent.py", "execute", "added") + ] + assert result["head"]["sources"] == [{"type": "openai_agents_sdk", "path": "agent.py"}] + if target == "leaves_scope": + assert result["comparison_status"] == "compared" + assert result["head"]["coverage_gaps"] == [] + else: + assert result["comparison_status"] == "partial" + assert [(g["source"], g["reason"]) for g in result["head"]["coverage_gaps"]] == [ + ("tools.py", "Linked Python input: tools.py") + ] + + +@pytest.mark.parametrize("scope", [".", "app"]) +def test_unchanged_submodule_is_named_without_refusing(repo, scope): + # temporalio/sdk-python#1868: temporalio/bridge/sdk-core is a gitlink the + # PR does not move. The same commit on both sides is the same content, so + # it cannot carry a change; it is named, not read and not refused. + vendored = commit(repo, {"README.md": "vendored"}) + gitlink(repo, "app/vendor/core", vendored) + base = commit(repo, {"app/agent.py": SDK.replace("TOOLS", "[lookup]")}) + head = commit(repo, {"app/agent.py": SDK.replace("TOOLS", "[lookup, execute]")}) + result = run(repo, base, head, "--scope", scope) + assert result["comparison_status"] == "compared" + assert [(r["tool"], r["change"]) for r in result["rows"]] == [("execute", "added")] + vendor = "app/vendor/core" if scope == "." else "vendor/core" + for side in ("base", "head"): + assert result[side]["coverage_gaps"] == [] + assert result[side]["limits"] == [ + f"Submodule content is not read (unchanged commit {vendored[:12]}): {vendor}" + ] + + +def test_submodule_behind_a_link_out_of_scope_is_not_the_scopes(repo): + # The scoped materializer brings the target of an in-scope directory link, + # gitlinks beneath it included. Discovery never walks through that link, + # so they are not this scope's submodules and name nothing. + vendored = commit(repo, {"README.md": "vendored"}) + gitlink(repo, "lib/vendor", vendored) + commit(repo, {"lib/README.md": "shared"}) + link(repo, "app/lib", "../lib") + base = commit(repo, {"app/agent.py": SDK.replace("TOOLS", "[lookup]")}) + head = commit(repo, {"app/agent.py": SDK.replace("TOOLS", "[lookup, execute]")}) + result = run(repo, base, head, "--scope", "app") + assert result["comparison_status"] == "compared" + assert [(r["tool"], r["change"]) for r in result["rows"]] == [("execute", "added")] + assert result["base"]["limits"] == result["head"]["limits"] == [] + + +@pytest.mark.parametrize("move", ["added", "bumped", "removed"]) +def test_moved_submodule_is_an_attributed_gap_not_a_refusal(repo, move): + first = commit(repo, {"README.md": "first"}) + second = commit(repo, {"README.md": "second"}) + if move != "added": + gitlink(repo, "vendor/core", first) + base = commit(repo, {"agent.py": SDK.replace("TOOLS", "[lookup]")}) + if move == "bumped": + gitlink(repo, "vendor/core", second) + elif move == "removed": + git(repo, "rm", "-q", "--cached", "vendor/core") + else: + gitlink(repo, "vendor/core", second) + head = commit(repo, {"agent.py": SDK.replace("TOOLS", "[lookup, execute]")}) + result = run(repo, base, head) + # Not `compared`: whatever moved inside the submodule is unobserved. + assert result["comparison_status"] == "partial" + # The known addition elsewhere stays visible (#871 fail-closed granularity). + assert [(r["tool"], r["change"]) for r in result["rows"]] == [("execute", "added")] + sides = {"added": ["head"], "bumped": ["base", "head"], "removed": ["base"]}[move] + for side in ("base", "head"): + gaps = result[side]["coverage_gaps"] + if side in sides: + assert [(g["source"], g["affects"]) for g in gaps] == [ + ("vendor/core", "binding_presence") + ] + assert gaps[0]["reason"].startswith("Submodule content is not read") + else: + assert gaps == [] + + +def test_adk_package_root_agent_import_is_an_agent(repo): + # TencentCloud/CubeSandbox#1508 imports the constructor from the package + # root, which re-exports google.adk.agents.llm_agent.Agent. + source = '''from google.adk import Agent + +def lookup(query: str) -> str: + return query + +def execute(code: str) -> str: + return code + +root_agent = Agent(name="helper", tools=TOOLS) +''' + base = commit(repo, {"agent.py": source.replace("TOOLS", "[lookup]")}) + head = commit(repo, {"agent.py": source.replace("TOOLS", "[lookup, execute]")}) + result = run(repo, base, head) + assert result["comparison_status"] == "compared" + assert [(r["agent"], r["tool"], r["change"]) for r in result["rows"]] == [ + ("helper", "execute", "added") + ] + + +def test_adk_package_module_alias_is_an_agent(repo): + source = '''import google.adk as adk + +def lookup(query: str) -> str: + return query + +root_agent = adk.Agent(name="helper", tools=TOOLS) +''' + base = commit(repo, {"agent.py": source.replace("TOOLS", "[]")}) + head = commit(repo, {"agent.py": source.replace("TOOLS", "[lookup]")}) + result = run(repo, base, head) + assert [(r["agent"], r["tool"], r["change"]) for r in result["rows"]] == [ + ("helper", "lookup", "added") + ] + + +def test_adk_package_root_agent_with_sibling_tool_is_partial_not_absent(repo): + # The CubeSandbox#1508 shape itself: the tool comes from a sibling module, + # which the reader does not resolve yet (#864). The agent is established, + # so the result names that gap rather than "no supported agents". + tool = "def run_python_in_cube(code: str) -> str:\n return code\n" + agent = ( + "from google.adk import Agent\n" + "from cube_code_tool import run_python_in_cube\n" + 'root_agent = Agent(name="cube_code_agent", tools=[run_python_in_cube])\n' + ) + base = commit(repo, {"README.md": "examples"}) + head = commit( + repo, {"examples/adk/agent.py": agent, "examples/adk/cube_code_tool.py": tool} + ) + result = run(repo, base, head) + assert result["comparison_status"] == "partial" + assert [a["name"] for a in result["head"]["agents"]] == ["cube_code_agent"] + assert [(g["source"], g["reason"]) for g in result["head"]["coverage_gaps"]] == [ + ( + "examples/adk/agent.py", + "Google ADK agent 'cube_code_agent' references unresolved tool " + "'run_python_in_cube'.", + ) + ] + + +def test_recording_gitlinks_is_opt_in_at_the_materializer(repo, tmp_path_factory): + # Only the application route opts in; every other archive caller still + # refuses a gitlink it would have to read through. + from agents_shipgate.cli.verify.git import archive_tree + from agents_shipgate.core.errors import ConfigError + + vendored = commit(repo, {"README.md": "vendored"}) + gitlink(repo, "vendor/core", vendored) + ref = commit(repo, {"agent.py": SDK.replace("TOOLS", "[lookup]")}) + with pytest.raises(ConfigError, match="unsupported external binding at vendor/core"): + archive_tree(repo, ref, tmp_path_factory.mktemp("refused"), scope=lambda _: True) + destination = tmp_path_factory.mktemp("recorded") + recorded = archive_tree( + repo, ref, destination, scope=lambda _: True, record_gitlinks=True + ) + assert recorded == {"vendor/core": vendored} + assert (destination / "vendor/core").is_dir() + assert not any((destination / "vendor/core").iterdir()) + assert (destination / "agent.py").read_text() == SDK.replace("TOOLS", "[lookup]") From 3a0c00c769d6d37aef0040b81ebc69fa47a3be8c Mon Sep 17 00:00:00 2001 From: Pengfei Hu Date: Fri, 25 Sep 2026 15:41:24 -0700 Subject: [PATCH 2/2] fix: keep unread links and scope-level submodules from reading as removals PR #877 review found two cases where unread input became conclusive negative evidence: - Discovery drops a path that does not resolve, so replacing `agent.py` with a dangling link read as a definite removal, `compared`, with no gap. The dropped host-census gap had been the only fallback, and it also covered a source directory replaced by an absolute or dangling link. Census every link under the scope directly, without following it: gap a `*.py` link that does not alias an input the scope already reads, a directory link holding Python outside the scope, and a link resolving to nothing in the tree where the other side reads source at or beneath it. An unchanged `agent/VERSION -> ../../VERSION` still changes nothing. - A gitlink at the selected scope itself gave a gap with source ".", which covers no relative binding path, so `agent.py` read as a definite addition or removal. Make that gap scope-wide. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 4 +- docs/application-comparison.md | 34 ++++-- docs/distribution-surfaces.md | 2 +- src/agents_shipgate/cli/application_diff.py | 103 +++++++++++++--- tests/test_application_diff_reach.py | 123 ++++++++++++++++++-- 5 files changed, 222 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4fa8bf60..af8b7b3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,8 +7,8 @@ - Add `diff --application` for OpenAI Agents SDK and Google ADK source-observed per-agent wiring, with exact base/head refs and independently selected scopes. No manifest, saved baseline or authored declarations are needed. The advisory `application_comparison_schema_version: "0.1"` result records before/after evidence, scoped coverage gaps and explicit uncertain candidates; it supplies no release verdict or merge permission. Includes scoped Git materialization, partial-clone recovery, and definition lookup using reader-resolved Python symbols and locations. See [application comparison](docs/application-comparison.md). (#871) - `diff --application` no longer refuses a repository over a link or submodule its application reader never opens, and reads a Google ADK agent imported from the package root. (#871 follow-up, measured for #868) - **The problem.** On 2026-09-25, against open third-party pull requests that edit SDK/ADK tool wiring, 15 of the first 27 runs at the default root scope exited 2 with `Git tree contains unsupported external binding`. The path named was never application source: `CLAUDE.md -> AGENTS.md` (dlt-hub/dlt#4417, jaegertracing/jaeger#9636, omnigent-ai/omnigent#6611, wandb/weave#7948), a linked `.claude/skills/…` or `.agents/skills/…` directory (asterinas#3834, vllm#57322, kagent-dev/kagent#2788), `agent/VERSION -> ../../VERSION` (TencentCloud/CubeSandbox#1508), `.pylintrc` (tensorflow#128063), and a vendored submodule (temporalio/sdk-python#1868). The root scope took the unscoped archive route, which refuses every link and gitlink. `from google.adk import Agent` was not read as an agent, so CubeSandbox#1508's `root_agent` gave `not_established` ("No supported application agents were established"). - - **Links.** The root scope now uses the scoped materializer a named scope already used, which recreates each link as a link. Python discovery never walks through a link, in a checkout or here, so a link that is not a Python input changes nothing. A Python file that is a link is never read through: when its target is a Python input the scope already reads, that file is compared at its own path (reading the alias as well made one agent two ambiguous ones and hid the real file's change, as a named scope did on main); any other in-scope target is a coverage gap over the link's own path. A scope that is itself a link is still refused. The host-configuration census, which counts every link that could conceal a *host* path, no longer adds application coverage gaps. - - **Submodules.** A gitlink under the scope is materialized as the empty directory a checkout without `--recurse-submodules` leaves; its content is never fetched. The same gitlink commit on both sides is named in `limits` as unchanged and does not make the comparison `partial`, since identical content cannot carry a change. An added, removed or moved gitlink is a coverage gap over its path on each side that has it, so the result is `partial`, never `compared`. A gitlink was refused (exit 2) before. + - **Links.** The root scope now uses the scoped materializer a named scope already used, which recreates each link as a link. A link is never read through. Every link under the scope is censused, a dangling one included, and gapped only where it can hide application source: a `*.py` link whose target is not a Python input the scope already reads (an alias of one is compared at the target's path; reading it too made one agent two ambiguous ones and hid the real file's change, as a named scope did on main); a link to a directory outside the scope that holds Python; and a link resolving to nothing in the repository where the other side reads source at or beneath its path. Replacing `agent.py` with a dangling link, or a source directory with an absolute link, is therefore `not_established`, never a removal. A link Python discovery would not read changes nothing, and a scope that is itself a link is still refused. The host-configuration census, which counts every link that could conceal a *host* path, no longer adds application coverage gaps. + - **Submodules.** A gitlink under the scope is materialized as the empty directory a checkout without `--recurse-submodules` leaves; its content is never fetched. The same gitlink commit on both sides is named in `limits` as unchanged and does not make the comparison `partial`, since identical content cannot carry a change. An added, removed or moved gitlink is a coverage gap over its path on each side that has it, and over the whole scope when it is the selected scope itself, so the result is `partial`, never `compared`. A gitlink was refused (exit 2) before. - **Google ADK.** `google.adk.Agent`, the package-root re-export of `google.adk.agents.llm_agent.Agent`, is read as an agent constructor by the ADK reader, for `from google.adk import Agent` and `import google.adk as adk` alike. CubeSandbox#1508 now establishes `cube_code_agent` and is `partial`, naming the tool it imports from a sibling module (#864), instead of `not_established`. - **Re-measured.** At the root scope, main `a430e81a` refused CubeSandbox#1508, dlt#4417 and asterinas#3834; this change compares each, with its own limits (a Python census past `--max-python-files`, an unsupported framework, an unresolved import). No application comparison schema change; `application_comparison_schema_version` stays `"0.1"`. diff --git a/docs/application-comparison.md b/docs/application-comparison.md index 599474cc..707bdc63 100644 --- a/docs/application-comparison.md +++ b/docs/application-comparison.md @@ -100,16 +100,27 @@ rather than the history. ### Links and submodules A symlink anywhere in the tree is recreated as the link it is, never refused -and never followed by the comparison. Python discovery does not walk through a -link, here or in a checkout, so a link that is not a Python input changes -nothing: `CLAUDE.md -> AGENTS.md`, a linked `.claude/skills/…` directory, or a -`VERSION` link that leaves the repository reads as it would without the link. -A Python file that is a link is never read through. When its target is a -Python input the scope already reads, that file is compared at its own path; -a link whose target leaves the scope is outside discovery, as in a checkout; -any other target is a coverage gap over the link's own path -(`Linked Python input: tools.py`). A selected scope that is itself a link is -refused (exit 2). +and never read through. Every link under the scope is censused, including one +that resolves to nothing, and what it can hide decides what it is: + +- A link Python discovery would not read changes nothing, as in a checkout: + `CLAUDE.md -> AGENTS.md`, or a linked `.claude/skills/…` directory whose + target the scope already reads at its own path. +- A `*.py` link whose target is a Python input the scope already reads is + compared at that target's path. Any other `*.py` link — dangling, leaving + the scope or the repository, or landing on something that is not a Python + input — is a coverage gap over the link's own path + (`Linked Python input: agent.py`), so replacing `agent.py` with a dangling + link is `not_established`, never a removal. +- A link to a directory outside the scope that holds Python is a gap over the + link's path (`Linked directory holds Python outside the scope: lib`). +- A link that resolves to nothing in the repository (dangling, absolute, or + leaving the tree) is a gap only where the other side reads source at or + beneath its path (`Linked input resolves outside the tree: tools`). An + unchanged `agent/VERSION -> ../../VERSION` beside the application changes + nothing. + +A selected scope that is itself a link is refused (exit 2). A submodule's content is in another repository. The comparison never fetches it and materializes its gitlink as the empty directory a checkout without @@ -119,7 +130,8 @@ same content, so it is named in both sides' `limits` — and nothing more. An added, removed or moved gitlink is a coverage gap over its path on each side that has it, so the comparison is `partial`, and a binding the other side holds at that path is `not_established` rather than added or -removed. +removed. A gitlink at the selected scope itself covers the whole scope +(`…: the selected scope`). Partial clones with unfetched objects exit 2 with `objects_missing`, name the affected side, diff --git a/docs/distribution-surfaces.md b/docs/distribution-surfaces.md index df12123e..b8a76d8a 100644 --- a/docs/distribution-surfaces.md +++ b/docs/distribution-surfaces.md @@ -75,7 +75,7 @@ and this document are checked against each other by | `human_review_decision` | `docs/human-review-decision.md` | `release_decision_vocabulary` | `test_surface_enumerations_match_the_engine_vocabulary` | Host-neutral read-only evaluator; no GitHub acquisition, persistence or operation authority. | | `github_action` | `action.yml`, `scripts/github_action_outputs.py` | `merge_verdict_vocabulary` | `test_action_input_enumerates_engine_merge_verdicts`, `test_action_output_script_shares_the_engine_merge_verdicts` | The paired `shipgate_wheel`/`shipgate_wheel_sha256` inputs install a caller-supplied local wheel instead of a published version, so that route names no channel and claims no `executable_pin`; it is refused unless both halves are given, and it installs `--no-deps`. `tests/test_action_engine_install.py` proves the refusals. Every `python` the Action starts in the workspace runs with `-P` or as a script path, so a pull request's `pip/` or `agents_shipgate/` package cannot stand in for pip or the engine; the same file executes the install and merge-verdict steps against such a checkout. The `v1.0.0` tag predates that fix; the published `v1.1.0` carries it. | | `capability_diff` | `src/agents_shipgate/cli/diff.py`, `src/agents_shipgate/core/capability_diff_rows.py`, `src/agents_shipgate/core/host_comparison.py`, `src/agents_shipgate/report/host_comparison.py`, `src/agents_shipgate/core/unread_inputs.py`, `src/agents_shipgate/cli/verify/changed_inputs.py` | — | — | Answers no question the engine answers: it emits no verdict, no release decision and no pin. Every field is read from the drift payload the engine already produces — `risk` is the engine's severity and `expansion_signals` is the engine's word on widening — so there is no second implementation to drift. A `permission_mode` or `sandbox` row names the setting and its value as the file spells it (`enableAllProjectMcpServers: true`, `defaultMode: dontAsk`), recovered from the grant's published value and digest, and a Claude Code setting's `why` is the basis the engine's one setting table (`core/host_settings.py`) records for the value; that table also rates the grant and `check`'s violation, so a row's severity and the violation's risk give one answer (#827, `tests/test_prompt_disabling_settings.py`). `verify`/PR and `check` reuse the host comparator (#684, `tests/test_manifest_free_pr_rows.py`), and the source name each named reusable-workflow secret refers to, also non-widening, with a redacting name or target refused rather than compared, and an unreadable value neither compared nor named on this surface — only the host inventory and `audit --host` name its `job/destination`, as on `1.0.0` (#693, `tests/test_reusable_workflow_secret_mappings.py`); check retains argument redaction and its existing local-policy control. Missing comparison evidence never supplies empty comparable rows. Default host mode only (`--application` is registered separately below); workflow rows compare effective writes and reusable secret recipients (#685, `tests/test_workflow_capability_diff.py`) and each job's remote step action references, as a non-widening change (#771, `tests/test_workflow_step_action_references.py`); and each job's agent launches — a documented agent action's permission inputs, the permission flags of a `run:` that is one plain `claude -p` / `codex exec` command — and checkout refs, compared as text, as a change unless the job gains a documented widening rule, which the engine names in `expansion_signals` (`workflow_agent_widened_*`) — a rule read only from text the engine reads exactly (no shell is parsed; an argument input that is not a plain list of words is compared by a digest and read for no rule), and a gain the engine does not claim (a rule moved in from a job the launch left, one an unread step of the job rewritten as a read launch may already have met, or one the job's launch held before in an expression or an unread argument input) named in the `why` from the same engine function, never counted — with a note on a workflow row naming the untrusted-input trigger, write scopes, secrets and pull request checkout beside each agent step, read off the grant the engine published and moving no direction; an unread `run:` agent step (never compared, so never a row), an unreadable value or a setting published redacted is named only by the host inventory and `audit --host`, as for an unread secret value, an unread argument input or an unresolved launch is named there and in the `why` of a row reporting its launch, and a checkout ref holding credential-shaped text is refused as a redacting step reference is (#823, `tests/test_workflow_agent_launches.py`); every job id, step label, trigger and scope name those rows print is the label the engine published once where it built the grant, redacted, never re-derived here; `check`'s workflow evidence is derived from the raw declarations, which it still compares, and redacts job and scope names by the same rule; two distinct job ids or triggers in one workflow, or scope names in one `permissions` mapping, that publish alike are refused rather than compared, so while such a workflow exists `check` refuses on every run even when it is unchanged (#802, `tests/test_workflow_label_redaction.py`); artifact-only edits remain separate evidence. Tool-source subjects are #655. Where a partial or experimental surface is byte-identical on both sides, `diff` and `verify` compare the rest and name it in `unchanged_limits`; `check` keeps refusing, because its boundary result cannot carry a limit yet (#721). A surface the reader reaches through an in-tree link it reads through qualifies only when that link, a link with the same text at each link on the way, and the file it lands on, the same blob at the same path, are both unchanged, read from the base's Git tree entries against a commit's or, without following any link, the working tree's; any change to either is treated as before. The same proof decides which shared plugin-reference limits `check` leaves out, so behind such a link `check` compares, and publishes the rows it finds, exactly as for a limit at its own path, and a comparison `partial` only because of such a limit is `comparable` with it in `unchanged_limits` (#822, `tests/test_linked_unchanged_limits.py`). A hook row's `why` states the grant's loading basis, read from its published `source`, `access` and `risk` by the engine's `hook_loading_basis`; only a hook the host loads for this project earns an expansion signal — one a settings layer declares, or one a plugin selects that the repository's project settings enable from an in-repository marketplace — so a declared-only hook, or one a plugin selects without that enablement, is a row and never an expansion, and a removal names no basis (#714). `check` compares without a plugin-reference limit both sides share on an untouched source, which it cannot name and, untouched, does not route; a limit only one side carries makes its comparison incomparable. Those rows are not what routes a change: `check`, and the boundary check a manifest-backed `verify` runs, route a changed hook declaration of a plugin the project settings enable through the existing protected-surface rule, from the plugin hook reader's selection on both compared sides, and count a changed hook file such a plugin selects that the reader does not open as incomplete input; the rows beside either are unchanged (#809, `tests/test_enabled_plugin_hook_routing.py`). A partial clone that never fetched the base's objects is refused as `objects_missing`, exit `2`, never compared and never fetched; the refusal ends with the remediation sentence `verify` reports for the same reason, produced by the same function (#817, `tests/test_capability_diff_partial_clone.py`). The text of `diff`, `verify`, the PR comment and `check` reads the rows through one function, `review_changes`, and adds no row and changes no row value in any JSON projection (#795, `tests/test_host_diff_review_changes.py`): a permission rule is named with its disposition; an MCP server with the command name (never its path) or redacted URL, its package and argument digest (#819) and the env and header key names its grant already publishes, a URL printing only in the engine's sanitized scheme-and-host form and otherwise as `url not shown`, or, when none of those differ, a sentence naming what was compared and that the change is in a detail not shown, such as the command's path or another setting; a hook with each handler field that changed — its group's matcher, its command as its executable's name and digest, its timeout — before and after, a handler only one side declares, or the published handlers in a different order with a detail not shown that may also differ, and past the handler bound the same kind of sentence naming a handler past it, all read from the handlers its host-grants `0.7` grant publishes, which hold no command or argument text, and never re-derived here, and a declaration outside the documented hooks shape named as not shown rather than guessed (#819, `tests/test_hook_mcp_detail_fields.py`); those hook and MCP members display what `config_sha256` already binds, so grant equality and every inventory digest leave them out, a saved baseline holds none of them, and no row, row value, reason, digest or control answer moves; the PR comment gives the lines 1.1.0 printed their room first, the coverage block included, and prints an entry whole when the whole comment fits, otherwise cut to the widest length of at least 60 characters at which it does, or else in its shortest form (a difference cut after its name, an added or removed grant as its row), never longer than the entry 1.1.0 printed, with one line naming `verifier.json`, so no long entry hides a row, the coverage block, the change count, the review question, the reproduction or the advisory that 1.1.0 kept (#819 review, cycles 4 and 6); an allow rule the permission lattice decided another replaced (`widened` or `narrowed`), or the exact rule text that moved between dispositions in one host and source (`moved`), is one entry, never on the routes that redact rule arguments; and `diff` counts entries `from N rows` when one joins rows. Comparable results with entries end with one review question, naming the row count when an entry joins rows, and every result whose comparison names a base commit and a commit or working-tree head — a zero-row result and a refusal included (#812 follow-up, `tests/test_host_comparison_coverage.py`) — ends with the compared commits, the tool version and an `agents-shipgate diff --base ` reproduction, labelled `Inputs:` rather than `Compared:` where the comparison was refused, since that run compared nothing — and a refused comparison publishes no `review` object at all, so those two lines are the only place that run states its provenance, built from the `base_commit` it publishes beside the refusal; `check` and a provided diff print the question alone, and no result without a change asks a question. Every one of those facts is published beside the rows, so a machine consumer reads what a human reads (#795 slice 2, same test file): a row adds `disposition`, the `allow`/`ask`/`deny` list a permission rule is declared under and `null` for any other kind, on every route that publishes rows; and `review` in `diff --json` (capability diff `0.3`) and `host_comparison.review` in `verifier.json` (verifier `0.20`) — one object for one comparison — carry the presented changes, each naming the `row_indexes` it stands for, the `direction` the text uses (`widened`, `narrowed` and `moved` included, which no single row can carry), its cells, its `why` and one `expands`, plus a `summary` of `{rows, changes, widenings}` equal to `diff`'s summary line, the review question and the reproduction command. The block is refused unless its changes stand for every published row exactly once, its counters match and no joined change's two sides read alike, so the routes that redact rule arguments publish their rows alone and never a pair that reads `X → X`; `check`'s boundary result carries rows, with their dispositions, and no block. It is presentation, not a second opinion: it is the one `review_changes` projection the text prints, so the rows, their values, their count and every control answer are what they were. A comparison read back from JSON prints the changes it published, and one whose rows a caller sliced falls back to those rows. Each comparison also says what it established (#812, `tests/test_host_comparison_coverage.py`): `coverage` in `diff --json` (capability diff `0.3`) and `host_comparison.coverage` in `verifier.json` (verifier `0.20`) are the same object, printed as `What this run established` by `diff`, `verify` text and the PR comment. It is read off the grant changes, artifact changes, observed sources and blocking issues the comparator already computed: a file's rows, counting a source inside it (`#profiles.`, `#plugins.`); a file with no row and no artifact change called unchanged (`compared`, `0` rows) only when Git proves its blob identical, as the check `unchanged_limits` uses does, asked privately in one bounded batch and never published, because the artifact digest redacts `env` values and `apiKeyHelper`; a file that changed with no compared grant moving (`changed_without_grant_change`), whose artifact differs only in its digest or whose content Git shows differs while its artifact did not (never a difference a checkout line-ending conversion or a converting attribute explains, and no filter is run), never a plugin manifest or marketplace, a retargeted link or project settings while a hook's loading basis moved, worded as no compared grant changing and never as which fields changed; any other changed file with no row (`changed_without_rows`); a file Git neither proves identical nor shows differs — a provided diff, a link read, a redacted path, a working-tree file a checkout wrote with `CRLF` that Git reports unchanged — as `unchanged_not_proven`, never no change and never a change (#812 review cycle 3); the side that published a source, worded `published by` rather than `read in` for a plugin manifest or marketplace, which is published only while it declares hooks; and on a refused comparison each blocking source and its kind. Outside the bounded candidate rules below, a file no inventory observed is never an item and its absence is no claim, which the block states where it is read — one line under the heading and `read_sources_only` in the JSON — so a true list cannot be taken for the account of the change (#812 follow-up); a source already in `unchanged_limits` is not repeated; the list is capped at ten with `omitted_items`, ordered so what no row shows precedes a file's rows and, among blocking limits, by kind (`unreadable`, `parse_failed`, `unresolved_precedence`, then `unsupported`, `dynamic_source_excluded`, `remote_source_excluded`) — order, not severity, and a ranking of kinds rather than of items, since `unsupported` carries both a file this entry merely does not accept and one whose own text would not parse, so an item behind the count may still be one to repair; total down to every field an item is keyed by, the source name and then its side, limit and status — and counted in text as items not listed, ranked below those listed, and the PR comment lists only what fits in the room its entries, review question, reproduction, advisory, next action and evidence leave, at most 2000 characters, so the block never pushes out a line the comment prints without it (a row list that fills the comment by itself still truncates it, as on `1.0.0`); an instruction file's line carries no redacted-values note; sources are the inventory's redacted paths; `null` means not recorded, which is how a `0.19` verifier reads. It moves no row, reason, digest, baseline, control state or next action, and `check`'s boundary result and text carry none, so neither `check` nor a provided diff asks Git anything for it. The same list names the changed inputs this entry does not read (#821, `tests/test_unread_changed_inputs.py`): capability diff `0.4` and verifier `0.21` add a `changed_not_read` item, with the `candidate` rule that named it, for each path in the comparison's own changed-file set — the committed change, or the working tree's tracked and untracked changes — that a bounded, documented rule set recognises as plausibly agent configuration (`mcp.json` in a plugin directory, a plugin manifest's `mcpServers`, a Codex, Cursor or Copilot manifest's `hooks` and the hook files it names, a manifest or marketplace that does not parse, `.cursor/hooks.json`, host settings below the repository root, a marketplace entry's external `source`) and that no inventory published; a member is named whatever read its file, because no reader reads it. It is named from the path and, for a manifest or marketplace member, its text: nothing is fetched, run or read as a grant, so it is never a row, a widening, a `check` violation or a loading claim, and an external source is described redacted and never fetched. It ranks right after the blocking limits, inside the same cap; `read_sources_only` is `false` while one is named, and the first line says so instead; `unread_candidates` and `unread_candidates_not_examined` say whether the change set was examined and how many candidates were not — past the bound of 32, or because a file the rule needed was not read or did not parse, one count the text names both causes of. A manifest-free `verify` whose only host-relevant change is such an input, or a changed candidate it counts as not examined, publishes the comparison instead of the setup route, and `verify --preview` then names `audit --host` instead of `init --write`, in an agent-related workspace too; a `0.20` verifier reads with the search not recorded. A comparison refused only by plugin-reference limits, each bounded by its plugin directory, that no compared source depends on, is `partial` instead (#808, `tests/test_partial_host_comparison.py`); any other blocking limit it carries must be one both sides share on an unchanged source, named in `unchanged_limits` as on a comparable result. Capability diff `0.4` and verifier `0.21` publish `comparison_status: partial` with the refusal's `incomparable_reasons`, the rows, review and unchanged limits established outside those directories, and each directory (the outermost, where one holds another) as the reserved `coverage.items[].scope` on the `blocking_limit` items it bounds, and never call a changed project settings file without a row `changed_without_grant_change`, since the hooks whose loading basis it decides are not all compared; `diff`, `verify` text and the PR comment lead with `Partial comparison against …` or `Host capability comparison partial: …` and `Not compared: , …` before any entry, and a partial result with no entry is never printed as no change. Independence is read off the reader's reference graph, never off directory names: any other limit that is not unchanged, a reference leaving its plugin, a plugin at the root or holding project settings, a marketplace elsewhere declaring inline hooks for it, or a directory that does not publish as itself refuses as before. It answers no engine question and moves no control: a partial comparison is not comparable, `verify`'s control and route are the refusal's, the control envelope projects it as `incomparable` with no rows, and `check`, whose boundary result cannot name a directory, refuses its comparison and decides exactly as before. A `0.20` verifier claiming a partial comparison or a scope is refused. | -| `application_diff` | `src/agents_shipgate/cli/application_diff.py` | — | — | Advisory SDK/ADK source-wiring comparison through `diff --application`. Reuses framework observations and the binding graph; its comparator answers no release or merge verdict, activation verdict, executable pin, or declared authority question. Unlike host mode it computes per-agent source differences, not a projection of host drift. Scoped gaps and `not_established` candidates bound the answer; no deployment-root reachability is asserted. `tests/test_application_diff.py` and `tests/test_application_diff_review.py` prove the advisory boundary, isolation, uncertainty and evidence identity. Every scope, the root included, is materialized by the scoped verified materializer, so a link is recreated rather than refused, a link that is not a Python input changes nothing, and a Python file that is a link is never read through; a submodule is never read, named as a limit when its gitlink commit is unchanged and a coverage gap over its path otherwise; the host-configuration census adds no application gap (`tests/test_application_diff_reach.py`). | +| `application_diff` | `src/agents_shipgate/cli/application_diff.py` | — | — | Advisory SDK/ADK source-wiring comparison through `diff --application`. Reuses framework observations and the binding graph; its comparator answers no release or merge verdict, activation verdict, executable pin, or declared authority question. Unlike host mode it computes per-agent source differences, not a projection of host drift. Scoped gaps and `not_established` candidates bound the answer; no deployment-root reachability is asserted. `tests/test_application_diff.py` and `tests/test_application_diff_review.py` prove the advisory boundary, isolation, uncertainty and evidence identity. Every scope, the root included, is materialized by the scoped verified materializer, so a link is recreated rather than refused and never read through; every link under the scope, a dangling one included, is censused and gapped only where it can hide application source (a `*.py` link not aliasing an input the scope reads, a directory link holding Python outside the scope, or an unresolved link where the other side reads source); a submodule is never read, named as a limit when its gitlink commit is unchanged and a coverage gap over its path (the whole scope at the scope itself) otherwise; the host-configuration census adds no application gap (`tests/test_application_diff_reach.py`). | | `zero_install_detector` | `tools/shipgate-detect.py` | `agent_project_verdict` | `test_detector_verdict_matches_cli` | Emits no `diagnostics[]` and no `next_actions[]`; evidence strings and framework scores are simplified. See the script's own "Intentional simplifications". | | `emitted_ci_workflow` | `src/agents_shipgate/cli/discovery/ci_workflow.py` | `executable_pin` | `tests/test_adopter_pins_resolve.py::test_the_emitted_workflow_pins_the_release_and_not_the_source_tree`, `tests/test_release_source.py::test_candidate_workflow_uses_immutable_source_before_and_after_publication` | Ordinary/source/preview builds use the published fallback; a stamped candidate pins its verified Action SHA and package version. Before publication its smoke substitutes the exact local wheel inputs. Provenance asserts no qualification. | | `prompts` | `prompts/` | `contract_floor`, `executable_pin`, `placeholder_ownership`, `release_decision_vocabulary` | `test_executable_pin_resolves_in_a_published_channel`, `test_surface_enumerations_match_the_engine_vocabulary`, `test_surface_routes_human_owned_placeholders_to_a_human`, `tests/test_adopter_pins_resolve.py::test_every_pin_init_writes_into_an_adopter_repo_names_the_published_release`, `tests/test_adopter_pins_resolve.py::test_the_shipped_floor_is_decided_against_the_release_the_prompts_pin` | — | diff --git a/src/agents_shipgate/cli/application_diff.py b/src/agents_shipgate/cli/application_diff.py index 73864322..7339efbb 100644 --- a/src/agents_shipgate/cli/application_diff.py +++ b/src/agents_shipgate/cli/application_diff.py @@ -9,6 +9,7 @@ import ast import hashlib import json +import os import sys import tempfile from dataclasses import dataclass, field @@ -18,7 +19,7 @@ import typer from agents_shipgate.cli.discovery import detect_workspace -from agents_shipgate.cli.discovery.artifacts import _candidate_files +from agents_shipgate.cli.discovery.artifacts import _candidate_files, _skip_part from agents_shipgate.cli.scan.source_loading import _build_canonical_tools from agents_shipgate.cli.verify.git import ( PromisedObjectsMissingError, @@ -65,6 +66,8 @@ class Observations: coverage_gaps: list[dict[str, Any]] = field(default_factory=list) #: Unpopulated submodules under the scope, by scope-relative path. submodules: dict[str, str] = field(default_factory=dict) + #: Links under the scope that resolve to nothing in the tree. + unresolved_links: list[str] = field(default_factory=list) def gap( self, @@ -222,20 +225,14 @@ def observe( if result.limits: result.status = "partial" return result - # A Python input that is a link is never read through. When its target is a - # Python input this scope already reads, it is compared at its own path, so - # the link adds nothing; reading it again made one agent two ambiguous ones - # and hid that file's changes. Any other target is named as a gap below. - linked = {p.relative_to(root).as_posix(): p for p in python_files if p.is_symlink()} - read_directly = {p.resolve() for p in python_files if not p.is_symlink()} + linked = _observe_links(result, tree.resolve(), root, python_files) detected = detect_workspace(root, max_python_files=max_python_files) if detected.python_parse_truncated: result.gap(f"Python discovery truncated at {max_python_files} files.") # `detected.host_discovery_incomplete_paths` is deliberately not a gap. It # is the host-configuration census, which counts every link that could - # conceal a host path. Python discovery never walks through a link, here - # or in a checkout, so `CLAUDE.md -> AGENTS.md` or a linked skill directory - # hides no application source; a linked Python input is named below. + # conceal a host path, `CLAUDE.md -> AGENTS.md` included. The links that can + # conceal application source were censused above, by `_observe_links`. for item in detected.excluded_sources: result.gap(f"Excluded candidate: {item}", source=item.get("path")) # Discovery omits malformed Python; preserve that gap rather than an empty @@ -243,10 +240,7 @@ def observe( if len(python_files) > max_python_files: result.gap(f"Python input census exceeds {max_python_files} files.") for file in python_files[:max_python_files]: - relative = file.relative_to(root).as_posix() - if relative in linked: - if _resolved(file) not in read_directly: - result.gap(f"Linked Python input: {relative}", source=relative) + if file.relative_to(root).as_posix() in linked: continue try: ast.parse(file.read_bytes()) @@ -287,6 +281,60 @@ def _resolved(path: Path) -> Path | None: return None +def _observe_links( + result: Observations, tree: Path, root: Path, python_files: list[Path] +) -> set[str]: + """Census every link under the scope, and gap the ones that hide source. + + Discovery's inventory cannot be the census: it drops a path that does not + resolve, or resolves out of the scope, so a dangling `agent.py` link read as + a removed agent. Nothing here is read through a link. Returns the linked + `*.py` paths, which are never reader inputs. + + - A `*.py` link whose target is a Python input this scope already reads is + compared at that target's own path; reading the alias too made one agent + two ambiguous ones. Any other `*.py` link is a gap over its path. + - A link to a directory outside the scope that holds Python is a gap: the + scope's reader never walks it. + - A link that resolves to nothing in the tree is a gap only where the other + side reads source at or beneath it (`_reconcile_unresolved_links`), so + `agent/VERSION -> ../../VERSION` changes nothing. + - Any other link (`CLAUDE.md -> AGENTS.md`, a directory read at its own + path) is outside Python discovery, as in a checkout. + """ + + read_directly = {p.resolve() for p in python_files if not p.is_symlink()} + linked_python: set[str] = set() + for directory, dirnames, filenames in os.walk(root): + dirnames[:] = [name for name in dirnames if not _skip_part(name)] + for name in sorted(dirnames + filenames): + path = Path(directory) / name + if not path.is_symlink(): + continue + relative = path.relative_to(root).as_posix() + target = _resolved(path) + if name.endswith(".py"): + linked_python.add(relative) + if target not in read_directly: + result.gap(f"Linked Python input: {relative}", source=relative) + elif target is None or not target.is_relative_to(tree): + result.unresolved_links.append(relative) + elif target.is_dir() and not _read_by_scope(root, target) and any( + candidate.suffix == ".py" for candidate in target.rglob("*") + ): + result.gap( + f"Linked directory holds Python outside the scope: {relative}", + source=relative, + ) + return linked_python + + +def _read_by_scope(root: Path, target: Path) -> bool: + return target.is_relative_to(root) and not any( + _skip_part(part) for part in target.relative_to(root).parts + ) + + def _observe_source(result: Observations, root: Path, source: ToolSourceConfig) -> None: """Each reader's unlocated gaps cover its input file, not other sources. @@ -419,24 +467,42 @@ def _reconcile_submodules(base: Observations, head: Observations) -> None: ``--recurse-submodules`` leaves an empty directory, and so does the materializer. The same gitlink commit on both sides is the same content, so it cannot carry a change: it is named as a limit and nothing more. Any - other gitlink is a gap over its own path on each side that has it. + other gitlink is a gap over its own path on each side that has it, and over + the whole scope when it is the selected scope itself. """ for path in sorted(base.submodules.keys() | head.submodules.keys()): before, after = base.submodules.get(path), head.submodules.get(path) + where = "the selected scope" if path == "." else path if before == after: - message = f"Submodule content is not read (unchanged commit {before[:12]}): {path}" + message = f"Submodule content is not read (unchanged commit {before[:12]}): {where}" base.limits.append(message) head.limits.append(message) continue for side, commit in ((base, before), (head, after)): if commit is not None: side.gap( - f"Submodule content is not read (commit {commit[:12]}): {path}", - source=path, + f"Submodule content is not read (commit {commit[:12]}): {where}", + source=None if path == "." else path, ) +def _reconcile_unresolved_links(base: Observations, head: Observations) -> None: + """Gap a link that resolves to nothing where the other side reads source. + + Such a link hides nothing on its own: its content is not in the + repository on either side. It hides a change only when it replaced source + the other side reads, at its path or beneath it, which would otherwise be + reported as a definite removal or addition. + """ + + for side, other in ((base, head), (head, base)): + read = {path for path, _name in other.agents} | {s["path"] for s in other.sources} + for link in side.unresolved_links: + if any(path == link or path.startswith(link + "/") for path in read): + side.gap(f"Linked input resolves outside the tree: {link}", source=link) + + def _meaning(binding: dict[str, Any]) -> dict[str, Any]: return { k: v @@ -673,6 +739,7 @@ def in_scope(path: str, selected: str = selected_scope) -> bool: f"base={old_scope!r}, head={scope!r}. Check --scope/--base-scope." ) _reconcile_submodules(old, new) + _reconcile_unresolved_links(old, new) moves = _align_exact_moves(workspace, base_commit, head_commit, old, new) rows = compare(old, new, target_moves={m["base_source"]: m["head_source"] for m in moves}) for row in rows: diff --git a/tests/test_application_diff_reach.py b/tests/test_application_diff_reach.py index 06738fe8..405beb8c 100644 --- a/tests/test_application_diff_reach.py +++ b/tests/test_application_diff_reach.py @@ -69,34 +69,107 @@ def test_python_link_to_an_input_already_read_is_not_read_twice(repo): assert result["head"]["sources"] == [{"type": "openai_agents_sdk", "path": "real/helper.py"}] -@pytest.mark.parametrize("target", ["leaves_scope", "not_python"]) -def test_python_link_to_anything_else_is_never_read_through(repo, target): - # Discovery drops a link whose target leaves the scope, in a checkout as - # here. One that stays in scope but lands on something this scope does not - # read as Python is a gap over the link's own path, and only that path. +@pytest.mark.parametrize("target", ["leaves_scope", "not_python", "dangling"]) +def test_python_link_to_anything_else_is_a_gap_over_its_path(repo, target): + # Never read through, and never silence: the link's own path is unread, + # and only that path. The unrelated addition stays established. if target == "leaves_scope": commit(repo, {"shared/tools.py": SDK.replace("TOOLS", "[lookup]")}) link(repo, "app/tools.py", "../shared/tools.py") - else: + elif target == "not_python": commit(repo, {"app/tools_impl": SDK.replace("TOOLS", "[lookup]")}) link(repo, "app/tools.py", "tools_impl") + else: + link(repo, "app/tools.py", "missing.py") base = commit(repo, {"app/agent.py": SDK.replace("TOOLS", "[lookup]")}) head = commit(repo, {"app/agent.py": SDK.replace("TOOLS", "[lookup, execute]")}) result = run(repo, base, head, "--scope", "app") + assert result["comparison_status"] == "partial" assert [(r["agent_source"], r["tool"], r["change"]) for r in result["rows"]] == [ ("agent.py", "execute", "added") ] assert result["head"]["sources"] == [{"type": "openai_agents_sdk", "path": "agent.py"}] - if target == "leaves_scope": - assert result["comparison_status"] == "compared" - assert result["head"]["coverage_gaps"] == [] - else: - assert result["comparison_status"] == "partial" - assert [(g["source"], g["reason"]) for g in result["head"]["coverage_gaps"]] == [ + for side in ("base", "head"): + assert [(g["source"], g["reason"]) for g in result[side]["coverage_gaps"]] == [ ("tools.py", "Linked Python input: tools.py") ] +@pytest.mark.parametrize("direction", ["file_to_link", "link_to_file"]) +def test_python_file_replaced_by_a_dangling_link_is_not_a_removal(repo, direction): + # PR #877 review: discovery drops a dangling link before any census, so + # replacing `agent.py` with `agent.py -> missing.py` read as a definite + # removal, `compared`, with no gap. + source = {"app/agent.py": SDK.replace("TOOLS", "[lookup]")} + if direction == "file_to_link": + base = commit(repo, source) + (repo / "app/agent.py").unlink() + link(repo, "app/agent.py", "missing.py") + head = commit(repo, {}) + else: + link(repo, "app/agent.py", "missing.py") + base = commit(repo, {}) + (repo / "app/agent.py").unlink() + head = commit(repo, source) + result = run(repo, base, head, "--scope", "app") + assert result["comparison_status"] == "partial" + candidate = "removed" if direction == "file_to_link" else "added" + assert [(r["tool"], r["change"], r["candidate_change"]) for r in result["rows"]] == [ + ("lookup", "not_established", candidate) + ] + side = "head" if direction == "file_to_link" else "base" + assert result["rows"][0]["uncertainty"] == {side: ["Linked Python input: agent.py"]} + + +@pytest.mark.parametrize("direction", ["directory_to_link", "link_to_directory"]) +@pytest.mark.parametrize("target", ["/opt/tools", "../../../outside/tools", "missing"]) +def test_source_directory_replaced_by_an_unresolved_link_is_not_a_removal( + repo, direction, target +): + source = {"app/tools/agent.py": SDK.replace("TOOLS", "[lookup]"), "app/README.md": "x"} + if direction == "directory_to_link": + base = commit(repo, source) + git(repo, "rm", "-rq", "app/tools") + link(repo, "app/tools", target) + head = commit(repo, {}) + else: + link(repo, "app/tools", target) + base = commit(repo, {"app/README.md": "x"}) + git(repo, "rm", "-q", "app/tools") + head = commit(repo, source) + result = run(repo, base, head, "--scope", "app") + assert result["comparison_status"] == "partial" + assert [(r["tool"], r["change"]) for r in result["rows"]] == [("lookup", "not_established")] + side = "head" if direction == "directory_to_link" else "base" + assert result["rows"][0]["uncertainty"] == { + side: ["Linked input resolves outside the tree: tools"] + } + + +def test_unchanged_unresolved_link_beside_the_application_is_not_a_gap(repo): + # `agent/VERSION -> ../../VERSION` (CubeSandbox#1508) is the same on both + # sides and replaced nothing the other side reads. + link(repo, "app/VERSION", "../../VERSION") + base = commit(repo, {"app/agent.py": SDK.replace("TOOLS", "[lookup]")}) + head = commit(repo, {"app/agent.py": SDK.replace("TOOLS", "[lookup, execute]")}) + result = run(repo, base, head, "--scope", "app") + assert result["comparison_status"] == "compared" + assert result["base"]["limits"] == result["head"]["limits"] == [] + + +def test_directory_link_leaving_the_scope_with_python_is_a_gap(repo): + commit(repo, {"lib/helpers.py": SDK.replace("TOOLS", "[lookup]")}) + link(repo, "app/lib", "../lib") + base = commit(repo, {"app/agent.py": SDK.replace("TOOLS", "[lookup]")}) + head = commit(repo, {"app/agent.py": SDK.replace("TOOLS", "[lookup, execute]")}) + result = run(repo, base, head, "--scope", "app") + assert result["comparison_status"] == "partial" + assert [(r["tool"], r["change"]) for r in result["rows"]] == [("execute", "added")] + assert [(g["source"], g["reason"]) for g in result["head"]["coverage_gaps"]] == [ + ("lib", "Linked directory holds Python outside the scope: lib") + ] + + @pytest.mark.parametrize("scope", [".", "app"]) def test_unchanged_submodule_is_named_without_refusing(repo, scope): # temporalio/sdk-python#1868: temporalio/bridge/sdk-core is a gitlink the @@ -133,6 +206,32 @@ def test_submodule_behind_a_link_out_of_scope_is_not_the_scopes(repo): assert result["base"]["limits"] == result["head"]["limits"] == [] +@pytest.mark.parametrize("direction", ["submodule_to_directory", "directory_to_submodule"]) +def test_submodule_at_the_selected_scope_covers_the_whole_scope(repo, direction): + # PR #877 review: a gitlink at the scope itself named source ".", which + # covered no relative binding path, so its unread content let `agent.py` + # read as a definite addition or removal. + vendored = commit(repo, {"README.md": "vendored"}) + source = {"app/agent.py": SDK.replace("TOOLS", "[lookup]")} + if direction == "submodule_to_directory": + gitlink(repo, "app", vendored) + base = commit(repo, {}) + git(repo, "rm", "-q", "--cached", "app") + head = commit(repo, source) + else: + base = commit(repo, source) + git(repo, "rm", "-rq", "app") + gitlink(repo, "app", vendored) + head = commit(repo, {}) + result = run(repo, base, head, "--scope", "app") + assert result["comparison_status"] == "partial" + assert [(r["tool"], r["change"]) for r in result["rows"]] == [("lookup", "not_established")] + side = "base" if direction == "submodule_to_directory" else "head" + reason = f"Submodule content is not read (commit {vendored[:12]}): the selected scope" + assert result["rows"][0]["uncertainty"] == {side: [reason]} + assert [(g["source"], g["reason"]) for g in result[side]["coverage_gaps"]] == [(None, reason)] + + @pytest.mark.parametrize("move", ["added", "bumped", "removed"]) def test_moved_submodule_is_an_attributed_gap_not_a_refusal(repo, move): first = commit(repo, {"README.md": "first"})