diff --git a/src/backend/docs/bot-config-manifest/design.zh-CN.md b/src/backend/docs/bot-config-manifest/design.zh-CN.md index bb0ab44ff..34475b74c 100644 --- a/src/backend/docs/bot-config-manifest/design.zh-CN.md +++ b/src/backend/docs/bot-config-manifest/design.zh-CN.md @@ -346,7 +346,11 @@ apply 在平台侧执行，天然产出结构化记录（#935 的 `last-start` "apply_id": "…", "bot_id": "…", "trigger": "create|republish|restart|explicit", "started_at": "…", "finished_at": "…", "result": "SUCCEEDED|PARTIAL|FAILED", "sources": [ - {"name": "content", "ref": "v1.2.0", "resolved_sha": "9c1f4ae…"} + {"name": "content", "url": "https://code.example.com/team/content.git", + "ref": "v1.2.0", "mode": "strict", "resolved_sha": "9c1f4ae…"}, + {"name": "https://code.example.com/team/tools.git@main", + "url": "https://code.example.com/team/tools.git", + "ref": "main", "mode": "non_strict", "resolved_sha": "7e3b91c…"} ], "entries": [ {"category": "skills", "name": "reviewer", @@ -356,9 +360,11 @@ apply 在平台侧执行，天然产出结构化记录（#935 的 `last-start` } ``` -命名源的解析结果记在顶层 `sources`（声明的 `ref` + 解析出的 -`resolved_sha`）——「这批 bot 线上跑的是哪一版内容」由此可查；条目层记 -`from`（来自哪个源）或 `source_digest`（URL 源）。 +git 源的解析结果记在顶层 `sources`（`url` + 声明的 `ref` + 解析出的 +`resolved_sha`）——「这批 bot 线上跑的是哪一版内容」由此可查；**每条声明一行**， +内联源没有名字，记成 `url@ref`。`strict` 的基线按 `(url, ref, mode)` 从这里读回去 +（schema §2.3），跟这一行叫什么名字无关。条目层记 `from`（来自哪个源）或 +`source_digest`（URL 源）。 经 `GET …/config-manifest/last-apply` 暴露。script 的输出维持现状：容器内 `/home/admin/logs/startup_script.log`。 diff --git a/src/backend/docs/bot-config-manifest/manifest-schema.zh-CN.md b/src/backend/docs/bot-config-manifest/manifest-schema.zh-CN.md index 7c67d7777..1f1aa2df8 100644 --- a/src/backend/docs/bot-config-manifest/manifest-schema.zh-CN.md +++ b/src/backend/docs/bot-config-manifest/manifest-schema.zh-CN.md @@ -472,13 +472,27 @@ manifest: | `mode` | 行为 | | --- | --- | | `non_strict`（**默认**） | 应用新内容，并在 apply report 里对该条目**告警**，写明前后两个 SHA | -| `strict` | 解析出的 SHA 与上次 apply 记录的不同时，该条目**失败**，bot 继续跑它现在跑的 | - +| `strict` | 同一个 `(url, ref, mode)` 这次解析出的 SHA 与上次 apply 记录的不同时，该条目**失败**，bot 继续跑它现在跑的 | + +- **基线按 `(url, ref, mode)` 记**，不按源名。两个分支问的都是同一件事：「**这个 + 仓库的这个 ref**，在我们上次解析它之后动过没有」。源叫什么是文档的事，跟这个 + 问题无关——改名不会丢基线，把 `url` 指向另一个仓库也不会继承前一个仓库的 SHA。 +- **`mode` 也在键里**，理由只有一个：**一个钉扎只能被同样模式下的那次 apply 推进**。 + 同一个 `(url, ref)` 在一份文档里被声明两次（一个 `strict`、一个 `non_strict`）是 + 合法的。ref 移动时，宽松的那条会正常下发并记下新 SHA，钉死的那条拒绝。若两者共用 + 基线，那个新 SHA 就会变成钉扎的基线，下一次（文档一个字没改的）apply 就会把它刚 + 拒绝掉的那个 commit 交给它——`strict` 退化成「每次移动只拒绝一次，然后照单全收」。 + 带上 `mode`，两条声明各记各的：宽松的往前走，钉死的继续拒绝，直到文档重新钉扎。 +- **改 `ref` 或改 `url` 就是一次重新钉扎**：文档写出来的新 `(url, ref)` 没有任何 + 一次 apply 对它有意见，于是既不拒绝、也不告警，正常解析并被这次 apply 记下。 + 这也是 `strict` 源升版的唯一正道——不必先切成 `non_strict` 应用一次再切回来。 + `strict` 拒绝的始终只有一种情况：**文档没动，而 ref 在脚下动了**。 +- **SHA 形式的 `ref` 两个分支都触发不了**——它解析出来永远是它自己， + `(url, )` 的基线只可能等于同一个 SHA。这是构造上的结果，不是一条特例。 + 写 `mode` 是「接受但无效」，不是报错。 - **写在源上**，不是按 bot、也不是按清单——要描述的性质是「这个 ref 允不允许 在我脚下移动」，它属于持有 `ref` 的那个东西。一份清单里同时有一个钉死的 外部依赖和一个快速变动的内部仓库是常态。 -- **SHA 形式的 `ref` 忽略这个模式**（它动不了，两个分支都触发不了）——是 - 「接受但无效」，不是报错。 - 未知取值 `PUT` 时拒绝：拼错的 `mode` 若静默落到默认值，等于什么都没钉住。 - 内联 git `source` 同样接受 `mode`（它也持有 `ref`）；`oss` 源写 `mode` 会被 `PUT` 拒绝——它没有 ref 可以移动。 diff --git a/src/backend/docs/bot-config-manifest/user-manual.zh-CN.md b/src/backend/docs/bot-config-manifest/user-manual.zh-CN.md index 4aa8b5f0c..1ae038b82 100644 --- a/src/backend/docs/bot-config-manifest/user-manual.zh-CN.md +++ b/src/backend/docs/bot-config-manifest/user-manual.zh-CN.md @@ -678,7 +678,11 @@ GET /openapi/v1/bots/{bot_id}/with-manifest/status "started_at": "…", "finished_at": "…", "result": "SUCCEEDED|PARTIAL|FAILED", "sources": [ - {"name": "content", "ref": "v1.2.0", "resolved_sha": "9c1f4ae…"} + {"name": "content", "url": "https://code.example.com/team/content.git", + "ref": "v1.2.0", "mode": "strict", "resolved_sha": "9c1f4ae…"}, + {"name": "https://code.example.com/team/tools.git@main", + "url": "https://code.example.com/team/tools.git", + "ref": "main", "mode": "non_strict", "resolved_sha": "7e3b91c…"} ], "entries": [ {"category": "identity", "name": "SOUL.md", "action": "updated", "from": "content"}, @@ -1020,12 +1024,25 @@ schema 已定稿（见 `manifest-schema.zh-CN.md` §3.4），但**第一期没 | `mode` | 行为 | | --- | --- | | `non_strict`（**默认**） | 应用新内容，并在 apply 报告里对该条目**告警**，写明前后两个 SHA | -| `strict` | 解析出的 SHA 与上次 apply 记录的不同时，该条目**失败**，bot 继续跑它现在跑的 | - +| `strict` | 同一个 `(url, ref, mode)` 这次解析出的 SHA 与上次 apply 记录的不同时，该条目**失败**，bot 继续跑它现在跑的 | + +- **基线按 `(url, ref, mode)` 记**，不按源名。两个分支问的都是同一件事：「这个仓库 + 的这个 ref，在我们上次解析它之后动过没有」——源叫什么是你文档里的事，跟这个问题 + 无关。改名不丢基线；把 `url` 指到另一个仓库也不会继承前一个仓库的 SHA。 +- **`mode` 在键里**：同一个 `(url, ref)` 你可以声明两次，一条 `strict`、一条 + `non_strict`（「这几个条目可以跟着分支走，那个不行」）。ref 动了以后，宽松的那条 + 正常下发并记下新 SHA，钉死的那条拒绝——两者**各记各的基线**，所以宽松的那条不会 + 把钉死的那条的基线推上去。否则下一次 apply（你一个字都没改）就会把刚被拒绝的那个 + commit 交给钉死的条目。 +- **改 `ref`（或改 `url`）就是一次重新钉扎**：新的 `(url, ref)` 没有任何一次 + apply 对它有意见，所以既不拒绝也不告警，照常解析并被这次 apply 记下。 + **这就是 `strict` 源的升版方式**——不用先切 `non_strict` 应用一次再切回来。 + `strict` 拒绝的只有一种情况：**你没改文档，而 ref 在脚下动了**。 +- **SHA 形式的 ref 两个分支都触发不了**——它只会解析成它自己。是「接受但无效」， + 不是报错。 - **写在源上**，不是按 bot、也不是按清单——要描述的性质是「这个 ref 允不允许在我 脚下移动」，它属于持有 ref 的那个东西。一份清单里同时有一个钉死的外部依赖和一个 快速变动的内部仓库是常态。 -- **SHA 形式的 ref 忽略这个模式**（它动不了）——是「接受但无效」，不是报错。 - 拼错的取值会被拒绝，不会静默落到默认值。 ### 6.3 `digest`：哪里强制、哪里非法 @@ -1233,7 +1250,8 @@ bot 也是队列上的一个任务。所以部署里必须满足两个前提： - 用的是 **tag 且没动**？那就是没变——改 `ref`（§4.8）。 - 用的是 **branch 且 `mode: strict`**？SHA 变了会让该条目**失败**，这是你要的钉扎 - 语义。看报告里的前后 SHA。 + 语义。看报告里的前后 SHA。要让它跟上，就在文档里把 `ref` 改成你真正想要的那个 + tag 或 commit——换了 `ref` 就是一次重新钉扎，不会被拒绝（§6.2）。 - 取源失败并落到了 **`keep_last`**？报告里那一条会写明。 --- @@ -1738,7 +1756,7 @@ B.2.2 / B.2.3 / B.2.4 与 `GET …/with-manifest/status` 的 `apply` 字段都 | `result` | enum \| `""` | `RUNNING` / `SUCCEEDED` / `PARTIAL` / `FAILED`，见 B.7。终态是从逐条结果**推导出来的摘要，给人看的**。**空报告时是空串** | | `started_at` | datetime \| null | 开始时间；bot 从没 apply 过时 `null` | | `finished_at` | datetime \| null | 结束时间。**`null` 有两个原因，别拿它判「在跑」**：`result` 是 `RUNNING`（真的在跑），或者这是一份**空报告**（`result` 为空串）。要判在飞的活，读 `result == "RUNNING"`，不要读 `finished_at == null` | -| `sources` | object[] | 命名源的溯源，每个源一行，见下。**「这批 bot 线上跑的到底是哪一版内容」看这里** | +| `sources` | object[] | git 源的溯源，**每条声明一行**，见下。**「这批 bot 线上跑的到底是哪一版内容」看这里** | | `categories` | object[] | 每个**被声明的**类目一行，见下。文档没提的类目不出现，因为它根本没被碰 | | `entries` | object[] | 每个**被声明的条目**一行，跨所有类目，见下 | | `notes` | string[] | 不属于任何条目的 apply 级说明。今天只有一处：teclaw 上「所有类目都写完了、最后整包 artifact 重投失败」记在这里，而不是让整次 apply 失败。ARCA 上恒为空 | @@ -1747,11 +1765,18 @@ B.2.2 / B.2.3 / B.2.4 与 `GET …/with-manifest/status` 的 `apply` 字段都 | 字段 | 类型 | 含义 | | --- | --- | --- | -| `name` | string | 源名（`sources.` 里的那个名字） | +| `name` | string | 源名（`sources.` 里的那个名字）；**内联 `source` 没有名字，记成 `url@ref`**（省略 `ref` 时是 `url@HEAD`） | +| `url` | string \| null | 仓库地址，`${BOT_*}` 已替换。与 `ref`、`mode` 合起来就是 `strict` 基线的键（§6.2） | | `ref` | string \| null | 声明的 ref：tag / branch / commit SHA | +| `mode` | string \| null | 这一次解析所用的 `strict` / `non_strict`。它是基线键的一部分，不是备注（§6.2） | | `resolved_sha` | string \| null | 这一次**实际解析到**的 commit。`ref: main` 这种会动的引用，下周就是另一个值 | | `auth` | string \| null | 用到的凭证**名**。**永远只有名字，没有值** | +**一条声明一行**：两个 `from` 名指向同一个 `(url, ref)` 就是两行（各自带着作者 +写下的那个名字），`resolved_sha` 相同；同一个仓库内联声明两个 `ref` 也是两行。 +同一个 `(url, ref)` 内联声明两次、只有 `mode` 不同，同样是两行——内联源的 `name` +都是 `url@ref`，这两行靠 `mode` 区分。 + `categories[]`： | 字段 | 类型 | 含义 | diff --git a/src/backend/src/agentclaw/community/adapters/http/openapi_v1/bots/config_manifest_support.py b/src/backend/src/agentclaw/community/adapters/http/openapi_v1/bots/config_manifest_support.py index fded3858b..ee0cc84e2 100644 --- a/src/backend/src/agentclaw/community/adapters/http/openapi_v1/bots/config_manifest_support.py +++ b/src/backend/src/agentclaw/community/adapters/http/openapi_v1/bots/config_manifest_support.py @@ -35,6 +35,7 @@ ConfigManifestApply, ConfigManifestApplyCategory, ConfigManifestApplyEntry, + ConfigManifestApplySource, ) from .schemas import ( ConfigManifestApplyStarted, @@ -257,7 +258,9 @@ def apply_payload(report: ApplyReport) -> ConfigManifestApply: result=payload["result"], started_at=report.started_at, finished_at=report.finished_at, - sources=payload["sources"], + sources=[ + ConfigManifestApplySource(**source) for source in payload["sources"] + ], categories=[ ConfigManifestApplyCategory(**category) for category in payload["categories"] diff --git a/src/backend/src/agentclaw/community/adapters/http/openapi_v1/bots/schemas_config_manifest_apply.py b/src/backend/src/agentclaw/community/adapters/http/openapi_v1/bots/schemas_config_manifest_apply.py index c40a253c0..fea57885b 100644 --- a/src/backend/src/agentclaw/community/adapters/http/openapi_v1/bots/schemas_config_manifest_apply.py +++ b/src/backend/src/agentclaw/community/adapters/http/openapi_v1/bots/schemas_config_manifest_apply.py @@ -63,6 +63,49 @@ class ConfigManifestApplyEntry(BaseModel): ) +class ConfigManifestApplySource(BaseModel): + """One git source this apply resolved — one row per declaration. + + Named per declaration, keyed on the repository: two `from` names pointing at + the same `(url, ref, mode)` are two rows carrying the same `resolved_sha`, + and two inline declarations of one repository at two refs are two rows too. + """ + + name: str = Field( + description="The `from` name, or `@` for a source written " + "inline on an entry." + ) + url: str | None = Field( + default=None, + description="The repository URL, with any `${BOT_*}` placeholder " + "already substituted. Together with `ref` and `mode` it is what strict " + "mode compares the next apply against — so re-pointing url or ref is a " + "re-pin, not a moved ref.", + ) + ref: str | None = Field( + default=None, + description="The ref as declared: a tag, a branch, or a commit SHA. " + "`HEAD` when the source declared none.", + ) + mode: str | None = Field( + default=None, + description="`strict` or `non_strict`, as the source declared it. Part " + "of the baseline key, so a `non_strict` declaration of a repository " + "never advances the baseline a `strict` declaration of the same " + "repository is pinned against.", + ) + resolved_sha: str | None = Field( + default=None, + description="The commit that ref actually resolved to in this apply — " + "what tells you which version of the content this bot is running.", + ) + auth: str | None = Field( + default=None, + description="The credential's name, never its value. Null for an " + "anonymous fetch.", + ) + + class ConfigManifestApplyCategory(BaseModel): """One category's summary, including what overwriting it removed.""" @@ -131,14 +174,13 @@ class ConfigManifestApply(BaseModel): finished_at: datetime | None = Field( default=None, description="Null exactly while `result` is `RUNNING`." ) - sources: list[dict] = Field( + sources: list[ConfigManifestApplySource] = Field( default_factory=list, - description="Provenance for the manifest's named remote sources: what " - "each `source` name actually resolved to, including the exact " - "`resolved_sha`, since a moving `ref` like `main` means something " - "different next week. Always empty in this release — nothing is fetched " - "yet — and filled once remote sources are supported. A credential " - "appears by name only, never by value.", + description="Provenance for the manifest's git sources: what each " + "declaration actually resolved to, including the exact `resolved_sha`, " + "since a moving `ref` like `main` means something different next week. " + "One row per declaration; empty when the document names no git source. " + "A credential appears by name only, never by value.", ) categories: list[ConfigManifestApplyCategory] = Field( default_factory=list, diff --git a/src/backend/src/agentclaw/community/core/bot_config_manifest/apply/outcomes.py b/src/backend/src/agentclaw/community/core/bot_config_manifest/apply/outcomes.py index 179818dd9..3248aef97 100644 --- a/src/backend/src/agentclaw/community/core/bot_config_manifest/apply/outcomes.py +++ b/src/backend/src/agentclaw/community/core/bot_config_manifest/apply/outcomes.py @@ -342,25 +342,25 @@ class SourceResolution: Only the git road produces these: an object-store source resolves no ref, so it contributes no row. A named source:: - SourceResolution( - name="content", # the 'from' name - ref="v1.2.0", # the declared ref, verbatim - resolved_sha="4f2a9c1b8e7d6a5c4b3a2918f7e6d5c4b3a29187", - auth="git-prod", # the credential NAME - ) - - An inline git source has no ``from`` name, so it is recorded under its - repository URL instead:: - - SourceResolution( - name="https://code.example.com/team/content.git", - ref="HEAD", - resolved_sha="4f2a9c1b8e7d6a5c4b3a2918f7e6d5c4b3a29187", - auth=None, - ) - - ``name`` is the same key the strict-mode baselines are read back by, so the - report and ``SourceSession.baselines`` agree on a source's identity. + SourceResolution(name="content", + url="https://code.example.com/team/content.git", + ref="v1.2.0", mode="strict", + resolved_sha="7c1d…", auth="gh-readonly") + + An inline git source has no ``from`` name, so it is named by the repository + and the ref it declared, joined by ``@``:: + + SourceResolution(name="https://code.example.com/team/content.git@main", + url="https://code.example.com/team/content.git", + ref="main", mode="non_strict", + resolved_sha="9e8d…", auth=None) + + ``name`` is the **display**: one row per declaration, so two names pointing + at one repository are two rows and two inline declarations of one + repository at two refs are two rows. ``(url, ref, mode)`` is what the + strict-mode baselines are read back by — the display plays no part in that, + because "has this repository's ref moved since we last resolved it" is not + a question about what the document called the source. Created by: ``apply/source_session.SourceSession.adopt``, one per distinct ``display`` name, returned through ``resolution_records()``. @@ -371,20 +371,36 @@ class SourceResolution: support engineer reads, so this is a security property rather than tidiness. """ - #: The ``from`` name, or the repository URL for an inline source. + #: The ``from`` name, or ``@`` for an inline source. name: str + #: The substituted repository URL — no credentials, which this record is + #: structurally unable to carry anyway: it holds names, never values. Part + #: of the key the next apply reads its baseline by, and ``None`` only on a + #: row written before this field existed. + url: str | None = None #: The ref as declared: a tag, a branch, or a full SHA. ``"HEAD"`` when the #: source declared none. ref: str | None = None #: The 40-character commit id the ref actually resolved to. resolved_sha: str | None = None + #: The ``mode`` this resolution was made under, ``"strict"`` or + #: ``"non_strict"``. The last third of the baseline key, and load-bearing + #: rather than informational: a pin may only be advanced by an apply that + #: stood behind it under the *same* mode. Were it left out, a document + #: naming one ``(url, ref)`` twice — once ``strict``, once ``non_strict`` — + #: would let the lax declaration record a moved sha that the pinned one had + #: just refused, and the next apply would hand the pinned entry the very + #: commit it rejected: "refuse each move once, then deliver it". + mode: str | None = None #: The credential's name, never its value. ``None`` for an anonymous fetch. auth: str | None = None def as_dict(self) -> dict[str, Any]: return { "name": self.name, + "url": self.url, "ref": self.ref, + "mode": self.mode, "resolved_sha": self.resolved_sha, "auth": self.auth, } @@ -415,7 +431,9 @@ class ApplyReport: sources=( SourceResolution( name="content", + url="https://code.example.com/team/content.git", ref="v1.2.0", + mode="strict", resolved_sha="4f2a9c1b8e7d6a5c4b3a2918f7e6d5c4b3a29187", auth="git-prod", ), @@ -472,7 +490,9 @@ def as_payload(self) -> dict[str, Any]: "result": "PARTIAL", "started_at": "2026-03-01T09:00:00+00:00", "finished_at": "2026-03-01T09:00:04+00:00", - "sources": [{"name": "content", "ref": "v1.2.0", + "sources": [{"name": "content", + "url": "https://code.example.com/team/content.git", + "ref": "v1.2.0", "mode": "strict", "resolved_sha": "4f2a9c1b...", "auth": "git-prod"}], "categories": [{"category": "mcp", "aborted": False, "partially_written": False, "removed": []}], diff --git a/src/backend/src/agentclaw/community/core/bot_config_manifest/apply/source_fetchers.py b/src/backend/src/agentclaw/community/core/bot_config_manifest/apply/source_fetchers.py index b8f2ce75f..2eb97be07 100644 --- a/src/backend/src/agentclaw/community/core/bot_config_manifest/apply/source_fetchers.py +++ b/src/backend/src/agentclaw/community/core/bot_config_manifest/apply/source_fetchers.py @@ -263,9 +263,10 @@ class DeclaredFetch: #: without one, so :class:`GitSourceFetcher` asserts rather than branches. session: Optional[SourceSession] #: The declared ``from`` name, e.g. ``"content"``, or ``None`` for an - #: inline source. The git road falls back to the repository URL when this - #: is ``None``, and the result is the ``display`` that names the source in - #: the report and keys its baseline. + #: inline source. The git road falls back to ``@`` when this is + #: ``None``, and the result is the ``display`` that names the source in the + #: report — one row per declaration. The baseline is not read by it: that + #: is keyed on the substituted ``(url, ref, mode)``. name: Optional[str] @@ -531,10 +532,12 @@ class GitSourceFetcher(SourceFetcher): entry-level ``auth`` (declare it inside the source object). The ref resolves once through the apply's source session, ``mode`` is - enforced against the last apply's resolved SHA, and what comes back is a - tree for the entry to interpret. ``keep_last`` falls back under the same - ruling wire failures get: a *refusal* is configuration and must not be - masked, a *failure* is the transport and may be. + enforced against the last apply's resolved SHA for the same + ``(url, ref, mode)``, + and what comes back is a tree for the entry to interpret. ``keep_last`` + falls back under the same ruling wire failures get: a *refusal* is + configuration and must not be masked, a *failure* is the transport and may + be. """ def __init__( @@ -574,7 +577,14 @@ def fetch(self, request: DeclaredFetch) -> EntryDelivery: subpath=compose_subpath(decl.subpath, entry.get("subpath")), mode=decl.mode, ) - display = request.name if request.name is not None else spec.url + # One report row per declaration. An inline source has no name to + # report under, and the URL alone is not one: two entries reading one + # repository at two refs would collapse into a single row naming + # neither ref. ``spec.ref`` is already normalised ("HEAD" when the + # declaration omitted it), so the display is stable across applies. + display = ( + request.name if request.name is not None else f"{spec.url}@{spec.ref}" + ) auth = decl.auth try: @@ -583,9 +593,7 @@ def fetch(self, request: DeclaredFetch) -> EntryDelivery: binding = self._credentials.binding(name=auth) binding.reauthorize(httpx.URL(spec.url)) headers = dict(binding.headers_for(httpx.URL(spec.url))) - checkout, fresh = session.checkout( - spec, headers=headers, display=display - ) + checkout, fresh = session.checkout(spec, headers=headers) except CredentialError as exc: raise EntryFetchError(str(exc)) from exc except PrefixAuthorizationError as exc: @@ -597,7 +605,6 @@ def fetch(self, request: DeclaredFetch) -> EntryDelivery: ctx, session=session, spec=spec, - display=display, keep_last=request.keep_last, ) if fallback is not None: @@ -616,7 +623,7 @@ def fetch(self, request: DeclaredFetch) -> EntryDelivery: if expired is not None: raise EntryFetchError(expired) - baseline = session.baseline(display) + baseline = session.baseline(spec.url, spec.ref, spec.mode) if ( spec.mode == "strict" and baseline is not None @@ -630,7 +637,14 @@ def fetch(self, request: DeclaredFetch) -> EntryDelivery: # Adopted AFTER the strict gate: a refused move must not write the # moved SHA into this apply's report, because the next apply reads # its baseline from there — adopting here would turn strict mode - # into "refuse each move exactly once, then deliver it". + # into "refuse each move exactly once, then deliver it". The baseline + # is the one recorded for this (url, ref, mode), so editing url or ref + # asks about a pair nothing has an opinion on yet: a deliberate re-pin + # passes, and only a pair that resolved differently under its own name + # is a move. ``mode`` is in that key so the same degradation cannot + # come in sideways either: a non_strict declaration of this repository + # records under its own key and cannot hand a strict one the commit it + # just refused. session.adopt( display=display, spec=spec, checkout=checkout, auth_name=auth ) @@ -666,24 +680,24 @@ def _keep_last( *, session: SourceSession, spec: GitSourceSpec, - display: str, keep_last: bool, ) -> Optional[FetchedEntry]: """``keep_last`` for the git road: the receipt of the *last-resolved* SHA, when there was one. - Looks the baseline up by ``display`` and reads the receipt filed under - ``git+@:``. Answers ``None`` — meaning + Looks the baseline up by ``(url, ref, mode)`` and reads the receipt filed + under ``git+@:``. Answers ``None`` — meaning "no fallback, let the failure stand" — in three cases: ``keep_last`` is - off, the source has no baseline (a first-time source has no stored copy - entitled to answer for it), or no receipt exists at that address. + off, the pair has no baseline (a first-time pair has no stored copy + entitled to answer for it, and a freshly re-pinned ``ref`` is a + first-time pair), or no receipt exists at that address. On a hit the :class:`FetchedEntry` carries ``from_store=True`` and a ``fallback_reason``, which is what the report's note comes from. """ if not keep_last: return None - baseline = session.baseline(display) + baseline = session.baseline(spec.url, spec.ref, spec.mode) if baseline is None: return None target = git_receipt_url(spec.url, baseline, spec.subpath) diff --git a/src/backend/src/agentclaw/community/core/bot_config_manifest/apply/source_session.py b/src/backend/src/agentclaw/community/core/bot_config_manifest/apply/source_session.py index 5110e752a..293de9487 100644 --- a/src/backend/src/agentclaw/community/core/bot_config_manifest/apply/source_session.py +++ b/src/backend/src/agentclaw/community/core/bot_config_manifest/apply/source_session.py @@ -1,13 +1,35 @@ """One apply's named-source state. The four things a single apply needs and nothing more: the document's -``sources`` declarations, the strict-mode baselines read back from the last -apply that resolved each source, a checkout cache keyed on the substituted -``(url, ref)``, and the :class:`SourceResolution` records the report will -carry. It hangs on ``ApplyContext`` beside ``budget``, mutable by design inside -a frozen context, because the fetcher is a DI singleton (state there would leak -across applies) and a re-resolution per entry would break "the same -``(url, ref)`` is pulled once per apply". +``sources`` declarations, the strict-mode baselines read back — keyed on the +substituted ``(url, ref, mode)`` — from the last apply that resolved that +triple, a checkout cache keyed on ``(url, ref)``, and the +:class:`SourceResolution` records the report will carry. It hangs on ``ApplyContext`` beside ``budget``, mutable +by design inside a frozen context, because the fetcher is a DI singleton (state +there would leak across applies) and a re-resolution per entry would break "the +same ``(url, ref)`` is pulled once per apply". + +A baseline is a fact about a **repository at a ref**, not about what the +document called it: strict mode asks "has this ``(url, ref)`` resolved +differently since we last stood behind it", and renaming a source, or pointing +one at another repository, does not make that a different question about the +same pair. Keying on the pair is also what makes the document the way out of a +strict refusal — editing ``ref`` asks about a pair no apply has an opinion on, +so a deliberate re-pin passes where a ref that moved underneath the same pair +still does not. + +``mode`` is the third of the key for one reason, and it is not symmetry: **a +pin may only be advanced by an apply that stood behind it under the same +mode.** One document may legally declare the same ``(url, ref)`` twice, once +``strict`` and once ``non_strict`` — two sources, two entries, one repository. +When the ref then moves, the lax declaration delivers and records the new sha +while the pinned one refuses it. Were the two to share a baseline, that +recorded sha would become the pin's baseline, and the next apply would hand the +pinned entry the very commit it had just rejected — strict mode degraded to +"refuse each move exactly once, then deliver it", which is the failure the +adoption order elsewhere in this feature exists to prevent. Keyed with the +mode, the two declarations keep their own histories: the lax one advances, the +pinned one goes on refusing until the document re-pins it. A checkout and its resolution are **deliberately two events**. Fetching the tree answers "what does the ref name right now"; adopting it answers "and this @@ -77,20 +99,31 @@ class SourceSession: #: Empty when the document declares no ``sources``; an entry naming one #: then fails with "is not declared under 'sources'". sources: Mapping[str, Mapping[str, Any]] - #: Display name → the 40-character SHA the last apply that resolved that - #: source recorded. Read back out of ``ApplyReport.sources`` through the - #: report history, so the keys are ``SourceResolution.name`` values: the - #: declared ``from`` name for a named source, the repository URL for an - #: inline one:: + #: ``(substituted repository url, ref, mode)`` → the 40-character SHA the + #: last apply that resolved that triple recorded. Read back out of + #: ``ApplyReport.sources`` through the report history, off each row's + #: ``url``, ``ref`` and ``mode``:: #: #: { - #: "content": "4f2a9c1b8e7d6a5c4b3a2918f7e6d5c4b3a29187", - #: "https://code.example.com/solo.git": "9b1c...", + #: ("https://code.example.com/team/content.git", "v1.2.0", + #: "strict"): "4f2a9c1b8e7d6a5c4b3a2918f7e6d5c4b3a29187", + #: ("https://code.example.com/solo.git", "HEAD", "non_strict"): + #: "9b1c...", #: } #: - #: A source absent here has no strict opinion yet, so strict mode admits - #: its first resolution and ``keep_last`` has no baseline receipt to reuse. - baselines: Mapping[str, str] + #: **Not** keyed on the report's display name. The name is how a document + #: refers to a source; the baseline answers "did this repository's ref move + #: under us", and a rename, a re-pointed ``url`` or an edited ``ref`` all + #: change the name's answer without changing the pair's. ``mode`` is in the + #: key so that a ``non_strict`` declaration of a repository cannot advance + #: the baseline a ``strict`` declaration of the same repository is pinned + #: against — see the module docstring. + #: + #: A triple absent here has no strict opinion yet, so strict mode admits + #: its first resolution and ``keep_last`` has no baseline receipt to reuse — + #: which is exactly what an edited ``ref`` produces, and why re-pinning the + #: document is how a strict source is advanced. + baselines: Mapping[tuple[str, str, str], str] #: The git transport; injected so tests script it and production gets the #: subprocess client via the DI provider. git: GitSourceClient @@ -118,18 +151,44 @@ class SourceSession: _checkouts: dict[tuple[str, str], GitCheckout] = field(default_factory=dict) #: The report's ``sources`` rows, in the order they were adopted. _resolutions: list[SourceResolution] = field(default_factory=list) - #: The display names already in ``_resolutions``. Makes :meth:`adopt` - #: idempotent per source, so ten entries naming one source produce one row:: + #: The ``(display, url, ref, mode)`` tuples already in ``_resolutions``. + #: Makes :meth:`adopt` idempotent per declaration, so ten entries naming + #: one source produce one row — and two declarations of one repository + #: produce two, which is what "one row per declaration" means:: + #: + #: {("content", "https://code.example.com/team/content.git", + #: "v1.2.0", "strict"), + #: ("https://code.example.com/solo.git@main", + #: "https://code.example.com/solo.git", "main", "non_strict")} #: - #: {"content", "https://code.example.com/solo.git"} - _recorded: set[str] = field(default_factory=set) + #: The rule this shape exists to satisfy: **the recording identity must be + #: at least as fine as the key the recording feeds.** What it feeds is the + #: next apply's baseline, keyed on ``(url, ref, mode)``; collapse two rows + #: the baseline would have told apart and the loser records nothing, so — + #: if the loser is the ``strict`` one — a pin never establishes a baseline + #: and therefore never refuses anything. + #: + #: The identity simply **contains** that key, rather than something + #: believed to imply it. Two weaker shapes were tried and both had holes, + #: which is the argument for not trying a third: + #: + #: * ``display`` alone. An inline display is ``url@ref``, so it cannot see + #: ``mode`` at all: one repository at one ref declared both ``strict`` + #: and ``non_strict`` collapsed onto one row. + #: * ``(display, mode)``. The ``@`` join is not injective — ``@`` is legal + #: in a URL path and in a refname — so ``url="…/a@b", ref="c"`` and + #: ``url="…/a", ref="b@c"`` still share a display and still collapsed. + #: + #: ``display`` stays in the tuple because it is not implied by the key + #: either: two ``from`` names over one ``(url, ref, mode)`` are two + #: declarations and must stay two rows. + _recorded: set[tuple[str, str, str, str]] = field(default_factory=set) def checkout( self, spec: GitSourceSpec, *, headers: Mapping[str, str], - display: str, ) -> "tuple[GitCheckout, bool]": """The checkout for one ``(url, ref)``, fetching only the first time. @@ -150,11 +209,9 @@ def checkout( cache hit answers ``False``, because those bytes were charged when they actually moved. - ``display`` is the report's name for the source — the declared ``from`` - name, or the repository URL for an inline one — and is the same key - ``baselines`` is read by, so strict mode and the report agree on - identity. It is not part of the cache key and nothing is recorded here: - see :meth:`adopt`. + The report's name for the source plays no part here and nothing is + recorded: a checkout answers "what does this ref name right now", and + standing behind that answer is a separate event — see :meth:`adopt`. ``headers`` carries the credential's injected headers, or is empty for an anonymous fetch. @@ -186,7 +243,9 @@ def adopt( auth_name="git-prod") # _resolutions now ends with - SourceResolution(name="content", ref="v1.2.0", + SourceResolution(name="content", + url="https://code.example.com/team/content.git", + ref="v1.2.0", mode="strict", resolved_sha="4f2a9c1b8e7d6a5c4b3a2918f7e6d5c4b3a29187", auth="git-prod") @@ -196,16 +255,22 @@ def adopt( for the entry — a refused move is not adopted, so the report of a refusing (failed) apply carries no poisoned baseline, and the last apply's record keeps refusing the moved ref until the document is - re-pinned. Idempotent per display: every entry that names the source - stands behind the same resolution. + re-pinned. Idempotent per ``(display, url, ref, mode)``, so every entry + that names one source stands behind one resolution and two names over + one repository record one row each. The identity contains the whole + baseline key rather than a display believed to imply it — see + ``_recorded`` for the two narrower shapes that turned out not to. """ - if display in self._recorded: + key = (display, spec.url, spec.ref, spec.mode) + if key in self._recorded: return - self._recorded.add(display) + self._recorded.add(key) self._resolutions.append( SourceResolution( name=display, + url=spec.url, ref=spec.ref, + mode=spec.mode, resolved_sha=checkout.sha, auth=auth_name, ) @@ -217,13 +282,18 @@ def resolution_records(self) -> tuple[SourceResolution, ...]: git source, which includes every object-store-only document.""" return tuple(self._resolutions) - def baseline(self, display: str) -> Optional[str]: - """The SHA the last apply that resolved this source found, or ``None``. + def baseline(self, url: str, ref: str, mode: str) -> Optional[str]: + """The SHA the last apply that resolved this ``(url, ref, mode)`` + found, or ``None``. - ``baseline("content")`` → ``"4f2a9c1b8e7d6a5c4b3a2918f7e6d5c4b3a29187"`` - for a source resolved before, ``None`` for one seen the first time. + ``baseline("https://code.example.com/team/content.git", "v1.2.0", + "strict")`` → ``"4f2a9c1b8e7d6a5c4b3a2918f7e6d5c4b3a29187"`` for a + triple resolved before, ``None`` for one seen the first time — which + includes a pair the document has just re-pinned onto, and a repository + this document reads at one mode and the last apply recorded at the + other. """ - return self.baselines.get(display) + return self.baselines.get((url, ref, mode)) def close(self) -> None: """Remove every checkout's temporary tree from disk and empty the diff --git a/src/backend/src/agentclaw/community/core/bot_config_manifest/services/apply_report_codec.py b/src/backend/src/agentclaw/community/core/bot_config_manifest/services/apply_report_codec.py index 64019dbc9..2a4c2ba04 100644 --- a/src/backend/src/agentclaw/community/core/bot_config_manifest/services/apply_report_codec.py +++ b/src/backend/src/agentclaw/community/core/bot_config_manifest/services/apply_report_codec.py @@ -84,7 +84,9 @@ def report_from_payload( sources=tuple( SourceResolution( name=source.get("name", ""), + url=source.get("url"), ref=source.get("ref"), + mode=source.get("mode"), resolved_sha=source.get("resolved_sha"), auth=source.get("auth"), ) diff --git a/src/backend/src/agentclaw/community/core/bot_config_manifest/services/config_manifest_apply_service.py b/src/backend/src/agentclaw/community/core/bot_config_manifest/services/config_manifest_apply_service.py index ea93289ac..cfcb1be34 100644 --- a/src/backend/src/agentclaw/community/core/bot_config_manifest/services/config_manifest_apply_service.py +++ b/src/backend/src/agentclaw/community/core/bot_config_manifest/services/config_manifest_apply_service.py @@ -757,8 +757,9 @@ def last_apply( def _last_resolutions( self, *, entity_id: str, bot_id: str - ) -> dict[str, str]: - """Each source's SHA as the last apply that RESOLVED it (W7 strict). + ) -> dict[tuple[str, str, str], str]: + """Each ``(url, ref, mode)``'s SHA as the last apply that RESOLVED it, + never the row's ``name`` (W7; ``apply/source_session`` says why). The reports are where "what did we resolve" already lives (``ApplyReport.sources``), so strict mode reads them back rather than @@ -767,9 +768,10 @@ def _last_resolutions( have failed to fetch a source (its report carries no resolution for it — a failed fetch or a strict refusal adopts nothing), and reading only that row would wipe the baseline, silently disarming strict mode - and the ``keep_last`` receipt after one outage. Per source, the - newest report that carries it wins; a report with no resolutions — - or no reports — yields no opinions. + and the ``keep_last`` receipt after one outage. Per key, the newest + report that carries it wins; rows sharing a key (the report holds one + per declaration) carry the same sha, so any serves. A row missing + ``url`` or ``mode`` yields no opinion, nor do empty or absent reports. """ records = self._applies.recent( env=get_current_env(), @@ -777,16 +779,18 @@ def _last_resolutions( bot_id=bot_id, limit=_BASELINE_HISTORY_APPLIES, ) - baselines: dict[str, str] = {} + baselines: dict[tuple[str, str, str], str] = {} for record in records: report = self._to_report(record, entity_id=entity_id, bot_id=bot_id) if report is None: continue for source in report.sources: - if source.resolved_sha is None: + if (source.url is None or source.mode is None + or source.resolved_sha is None): continue - # Newest wins: an earlier walk-back entry is not overwritten. - baselines.setdefault(source.name, source.resolved_sha) + # Newest wins; "HEAD" normalised the way the spec does it. + key = (source.url, source.ref or "HEAD", source.mode) + baselines.setdefault(key, source.resolved_sha) return baselines # ── internals ─────────────────────────────────────────────────────────── diff --git a/src/backend/tests/community/core/bot_config_manifest/apply/test_apply_engine.py b/src/backend/tests/community/core/bot_config_manifest/apply/test_apply_engine.py index bdad65917..5cd5c22b0 100644 --- a/src/backend/tests/community/core/bot_config_manifest/apply/test_apply_engine.py +++ b/src/backend/tests/community/core/bot_config_manifest/apply/test_apply_engine.py @@ -876,7 +876,12 @@ async def test_the_sessions_resolutions_ride_into_the_report(): """ engine = _engine() resolution = SourceResolution( - name="charts", ref="main", resolved_sha="f" * 40, auth="ci-token" + name="charts", + url="https://git.corp/charts.git", + ref="main", + mode="strict", + resolved_sha="f" * 40, + auth="ci-token", ) # The test-visible seam for a checkout that a materialiser's resolve # would have recorded: the session's own record list, appended directly. @@ -892,7 +897,9 @@ async def test_the_sessions_resolutions_ride_into_the_report(): assert report.as_payload()["sources"] == [ { "name": "charts", + "url": "https://git.corp/charts.git", "ref": "main", + "mode": "strict", "resolved_sha": "f" * 40, "auth": "ci-token", } diff --git a/src/backend/tests/community/core/bot_config_manifest/apply/test_apply_service_lifecycle.py b/src/backend/tests/community/core/bot_config_manifest/apply/test_apply_service_lifecycle.py index 662bd6f22..a0c1a75b6 100644 --- a/src/backend/tests/community/core/bot_config_manifest/apply/test_apply_service_lifecycle.py +++ b/src/backend/tests/community/core/bot_config_manifest/apply/test_apply_service_lifecycle.py @@ -671,8 +671,9 @@ def test_a_strict_baseline_is_read_back_from_report_history(world, monkeypatch): Strict mode reads "what did we resolve last time" out of ``ApplyReport.sources`` rather than a second table, so the two cannot drift. No report — and a report with no resolutions — yield no - opinions; a recorded resolution yields its SHA by name; and the walk is - bounded by the history window rather than one row (see the next test). + opinions; a recorded resolution yields its SHA under its ``(url, ref)``; + and the walk is bounded by the history window rather than one row (see + the next test). """ service, _applies, _locks, _scripts, _manifests = world @@ -687,7 +688,12 @@ def test_a_strict_baseline_is_read_back_from_report_history(world, monkeypatch): assert service._last_resolutions(entity_id=_ENTITY, bot_id=_BOT) == {} charts = SourceResolution( - name="charts", ref="main", resolved_sha="f" * 40, auth="ci-token" + name="charts", + url="https://git.corp/charts.git", + ref="main", + mode="strict", + resolved_sha="f" * 40, + auth="ci-token", ) monkeypatch.setattr( service._applies, @@ -698,7 +704,50 @@ def test_a_strict_baseline_is_read_back_from_report_history(world, monkeypatch): ) assert service._last_resolutions( entity_id=_ENTITY, bot_id=_BOT - ) == {"charts": "f" * 40} + ) == {("https://git.corp/charts.git", "main", "strict"): "f" * 40} + + +def test_baselines_are_read_by_url_ref_and_mode_and_skip_incomplete_rows( + world, monkeypatch +): + """The key is the repository, the ref and the mode, with three consequences. + + Several rows in one report may share a key — the report carries one row + per *declaration*, so two ``from`` names over one repository at one mode + are two rows — and they always carry the same sha, because one apply + resolves a ``(url, ref)`` once. A second ref, or the same pair at the + other ``mode``, is its own key with its own answer. And a row missing + ``url`` or ``mode`` is skipped outright: that is a report written before + those were recorded, and there is no honest way to guess what it meant. + """ + service, _applies, _locks, _scripts, _manifests = world + url = "https://git.corp/charts.git" + rows = [ + SourceResolution(name="charts", url=url, ref="main", + mode="non_strict", resolved_sha="f" * 40), + # A second declaration of the same repository, ref and mode. + SourceResolution(name="dashboards", url=url, ref="main", + mode="non_strict", resolved_sha="f" * 40), + # Same repository, another ref: its own key, its own answer. + SourceResolution(name="pinned", url=url, ref="v1", + mode="strict", resolved_sha="c" * 40), + # Same repository AND ref, the other mode: also its own key. This is + # the row that must not become the strict declaration's baseline. + SourceResolution(name="pinned-main", url=url, ref="main", + mode="strict", resolved_sha="e" * 40), + # Pre-existing history: no url and no mode, so no baseline. + SourceResolution(name="legacy", ref="main", resolved_sha="d" * 40), + ] + monkeypatch.setattr( + service._applies, + "recent", + lambda *, env, entity_id, bot_id, limit: [_row(_report_with_sources(rows))], + ) + assert service._last_resolutions(entity_id=_ENTITY, bot_id=_BOT) == { + (url, "main", "non_strict"): "f" * 40, + (url, "v1", "strict"): "c" * 40, + (url, "main", "strict"): "e" * 40, + } def test_a_failed_apply_does_not_wipe_a_strict_baseline(world, monkeypatch): @@ -708,7 +757,13 @@ def test_a_failed_apply_does_not_wipe_a_strict_baseline(world, monkeypatch): mode for the apply after it — the record one row back still holds the baseline, and the newest row that carries a source wins per source.""" service, _applies, _locks, _scripts, _manifests = world - charts = SourceResolution(name="charts", ref="main", resolved_sha="e" * 40) + charts = SourceResolution( + name="charts", + url="https://git.corp/charts.git", + ref="main", + mode="strict", + resolved_sha="e" * 40, + ) empty_failed = _report_with_sources( [], status=ApplyStatus.FAILED, apply_id="failed-1" ) @@ -722,11 +777,17 @@ def test_a_failed_apply_does_not_wipe_a_strict_baseline(world, monkeypatch): ) assert service._last_resolutions( entity_id=_ENTITY, bot_id=_BOT - ) == {"charts": "e" * 40} + ) == {("https://git.corp/charts.git", "main", "strict"): "e" * 40} # Newest wins per source: a newer report that re-resolved the source is # the baseline, not an older one. - moved = SourceResolution(name="charts", ref="main", resolved_sha="b" * 40) + moved = SourceResolution( + name="charts", + url="https://git.corp/charts.git", + ref="main", + mode="strict", + resolved_sha="b" * 40, + ) monkeypatch.setattr( service._applies, "recent", @@ -736,7 +797,7 @@ def test_a_failed_apply_does_not_wipe_a_strict_baseline(world, monkeypatch): ) assert service._last_resolutions( entity_id=_ENTITY, bot_id=_BOT - ) == {"charts": "b" * 40} + ) == {("https://git.corp/charts.git", "main", "strict"): "b" * 40} diff --git a/src/backend/tests/community/core/bot_config_manifest/apply/test_identity_materialiser.py b/src/backend/tests/community/core/bot_config_manifest/apply/test_identity_materialiser.py index 55ccf5cf9..81da7e118 100644 --- a/src/backend/tests/community/core/bot_config_manifest/apply/test_identity_materialiser.py +++ b/src/backend/tests/community/core/bot_config_manifest/apply/test_identity_materialiser.py @@ -521,7 +521,7 @@ def test_a_moved_ref_on_the_git_road_lands_in_the_note(): ctx = _git_ctx( git, sources={"id": IDENTITY_GIT_SOURCE}, - baselines={"id": "b" * 40}, + baselines={("https://git.corp/id.git", "main", "non_strict"): "b" * 40}, ) resolved = _run(materialiser.resolve(ctx, [{"type": "RULES.md", "from": "id"}])) assert resolved.ok diff --git a/src/backend/tests/community/core/bot_config_manifest/apply/test_skills_materialiser.py b/src/backend/tests/community/core/bot_config_manifest/apply/test_skills_materialiser.py index 6e11c00c0..47b3dc552 100644 --- a/src/backend/tests/community/core/bot_config_manifest/apply/test_skills_materialiser.py +++ b/src/backend/tests/community/core/bot_config_manifest/apply/test_skills_materialiser.py @@ -715,7 +715,9 @@ def test_a_moved_ref_note_survives_into_the_package(): ctx = _git_ctx( git, sources={"src": SKILL_GIT_SOURCE}, - baselines={"src": "b" * 40}, + baselines={ + ("https://git.corp/skills.git", "main", "non_strict"): "b" * 40 + }, ) result, _, written = _run(_apply(materialiser, ctx, [{"name": "demo", "from": "src"}])) assert result.ok @@ -739,7 +741,9 @@ def test_git_keep_last_serves_the_stored_zip_through_the_zip_road(): ctx = _git_ctx( git, sources={"src": SKILL_GIT_SOURCE}, - baselines={"src": "b" * 40}, + baselines={ + ("https://git.corp/skills.git", "main", "non_strict"): "b" * 40 + }, ) result, _, written = _run( _apply( diff --git a/src/backend/tests/community/core/bot_config_manifest/apply/test_source_resolver.py b/src/backend/tests/community/core/bot_config_manifest/apply/test_source_resolver.py index e43f7448e..76510b6ac 100644 --- a/src/backend/tests/community/core/bot_config_manifest/apply/test_source_resolver.py +++ b/src/backend/tests/community/core/bot_config_manifest/apply/test_source_resolver.py @@ -626,10 +626,13 @@ def test_resolve_missing_session_is_loud(rig): def test_strict_refuses_when_the_ref_moved(rig): _, _, pipeline = rig git = _ScriptedGit() - # An inline source's report identity is its repository URL, so that is - # the key its baseline is read back by. + # The baseline is keyed on the repository and the ref, not on what the + # document called the source — so the same pair, resolving to a different + # commit, is the one thing strict mode refuses. ctx = make_context( - source_session=_session(git, baselines={GIT_URL: "b" * 40}) + source_session=_session( + git, baselines={(GIT_URL, "main", "strict"): "b" * 40} + ) ) with pytest.raises(EntryFetchError, match="moved"): pipeline.resolve( @@ -648,7 +651,9 @@ def test_non_strict_records_the_move_in_the_note(rig): _, _, pipeline = rig git = _ScriptedGit() ctx = make_context( - source_session=_session(git, baselines={GIT_URL: "b" * 40}) + source_session=_session( + git, baselines={(GIT_URL, "main", "non_strict"): "b" * 40} + ) ) decl = pipeline.resolve( ctx, @@ -673,6 +678,340 @@ def test_strict_on_the_first_apply_has_no_opinion(rig): assert decl.note() is None +def test_an_inline_source_is_named_url_at_ref(rig): + """An inline declaration has no name to report under, so it is named by + the repository and the ref it asked for. The URL alone would not do: two + entries reading one repository at two refs would collapse into one row + naming neither of them, and a reader could not tell which ref the sha + belonged to.""" + _, _, pipeline = rig + git = _ScriptedGit() + ctx = make_context(source_session=_session(git)) + for ref in ("a", "b"): + pipeline.resolve( + ctx, + entry={"source": {"protocol": "git", "url": GIT_URL, "ref": ref}}, + category="skills", + ) + records = ctx.source_session.resolution_records() + assert [r.name for r in records] == [f"{GIT_URL}@a", f"{GIT_URL}@b"] + # The url rides its own field, so a reader never has to split the name. + assert [(r.url, r.ref) for r in records] == [(GIT_URL, "a"), (GIT_URL, "b")] + + +def test_an_inline_strict_refusal_names_the_ref_it_refused(rig): + """The refusal message uses the display, which for an inline source now + says which ref moved — the difference between "this repository moved" and + a sentence a reader can act on when the document names it twice.""" + _, _, pipeline = rig + git = _ScriptedGit() + ctx = make_context( + source_session=_session( + git, baselines={(GIT_URL, "main", "strict"): "b" * 40} + ) + ) + with pytest.raises(EntryFetchError, match=f"{GIT_URL}@main"): + pipeline.resolve( + ctx, + entry={"source": {"protocol": "git", "url": GIT_URL, "ref": "main", + "mode": "strict"}}, + category="skills", + ) + + +def test_two_names_over_one_repository_are_two_rows_and_one_baseline(rig): + """Report rows are per declaration; baselines are per ``(url, ref, mode)``. + A document that names one repository twice at one mode gets both names back + in the report — each author finds the name they wrote — and one checkout, + one sha, and one baseline key behind them.""" + _, _, pipeline = rig + git = _ScriptedGit() + session = _session(git, sources={ + "content": {"protocol": "git", "url": GIT_URL, "ref": "main"}, + "docs": {"protocol": "git", "url": GIT_URL, "ref": "main"}, + }) + ctx = make_context(source_session=session) + for name in ("content", "docs"): + pipeline.resolve(ctx, entry={"from": name}, category="skills") + records = session.resolution_records() + assert [r.name for r in records] == ["content", "docs"] + assert {r.resolved_sha for r in records} == {_FAKE_SHA} + # One key, so the next apply reads one baseline for both rows — the apply + # service's own test pins that the collapse survives the report round trip. + assert {(r.url, r.ref, r.mode) for r in records} == { + (GIT_URL, "main", "non_strict") + } + assert len(git.specs) == 1, "one checkout per (url, ref) per apply" + + +def test_an_at_sign_in_a_url_or_ref_does_not_collide_two_declarations(rig): + """The ``@`` join that names an inline source is not injective. + + ``@`` is legal in a URL path and legal in a refname, so + ``url="…/a@b", ref="c"`` and ``url="…/a", ref="b@c"`` produce the same + display, ``…/a@b@c`` — two genuinely different repositories-at-refs with + one name. De-duplicating the report on anything derived from that join + dropped the second declaration, and a dropped ``strict`` declaration never + establishes a baseline, so it accepts every move thereafter. + + Exotic inputs, ordinary rule: the recording identity contains the baseline + key outright rather than a display believed to imply it. + """ + _, _, pipeline = rig + first = {"protocol": "git", "url": "https://git.corp/a@b", "ref": "c", + "mode": "strict"} + second = {"protocol": "git", "url": "https://git.corp/a", "ref": "b@c", + "mode": "strict"} + + git = _ScriptedGit() + session = _session(git) + ctx = make_context(source_session=session) + pipeline.resolve(ctx, entry={"source": first}, category="skills") + pipeline.resolve(ctx, entry={"source": second}, category="skills") + + rows = session.resolution_records() + # Both recorded. The display really does collide — that is the point — so + # the rows are told apart by the url and ref they carry in their own right. + assert [r.name for r in rows] == [ + "https://git.corp/a@b@c", "https://git.corp/a@b@c" + ] + assert [(r.url, r.ref) for r in rows] == [ + ("https://git.corp/a@b", "c"), ("https://git.corp/a", "b@c") + ] + + # So the second declaration has a baseline of its own, and its pin holds + # when its ref moves. + rebuilt = {(r.url, r.ref, r.mode): r.resolved_sha for r in rows} + assert rebuilt == { + ("https://git.corp/a@b", "c", "strict"): _FAKE_SHA, + ("https://git.corp/a", "b@c", "strict"): _FAKE_SHA, + } + moved = _ScriptedGit(sha="c" * 40) + with pytest.raises(EntryFetchError, match="moved"): + pipeline.resolve( + make_context(source_session=_session(moved, baselines=rebuilt)), + entry={"source": second}, + category="skills", + ) + + +def test_two_inline_declarations_at_two_modes_are_two_rows(rig): + """The same alias hazard, on the road where the display is not unique. + + A named source's display is its ``from`` name, which maps to exactly one + ``(url, ref, mode)``. An inline source's display is ``url@ref`` — mode-blind + — so two inline declarations of one repository at one ref but two modes + share it. De-duplicated on the display alone, whichever resolved first took + the slot and the other recorded nothing; when the loser was the ``strict`` + one, it never established a baseline, and a pin with no baseline never + refuses anything. The rule that closes it: **the recording identity must be + at least as fine as the key the recording feeds.** + """ + _, _, pipeline = rig + loose = {"protocol": "git", "url": GIT_URL, "ref": "main", + "mode": "non_strict"} + pinned = {"protocol": "git", "url": GIT_URL, "ref": "main", "mode": "strict"} + + # Apply N, the bot's first: the lax declaration resolves first and would + # have taken the shared display slot. + git = _ScriptedGit() + session = _session(git) + ctx = make_context(source_session=session) + pipeline.resolve(ctx, entry={"source": loose}, category="skills") + pipeline.resolve(ctx, entry={"source": pinned}, category="skills") + + rows = session.resolution_records() + # Two rows. They share a name — ``url@ref`` is what an inline source is + # called — and are told apart by the mode each was resolved under. + assert [(r.name, r.mode, r.resolved_sha) for r in rows] == [ + (f"{GIT_URL}@main", "non_strict", _FAKE_SHA), + (f"{GIT_URL}@main", "strict", _FAKE_SHA), + ] + + # Apply N+1: the ref has moved. The pin has a baseline to refuse against. + rebuilt = {(r.url, r.ref, r.mode): r.resolved_sha for r in rows} + assert rebuilt == { + (GIT_URL, "main", "non_strict"): _FAKE_SHA, + (GIT_URL, "main", "strict"): _FAKE_SHA, + } + moved = _ScriptedGit(sha="c" * 40) + with pytest.raises(EntryFetchError, match="moved"): + pipeline.resolve( + make_context(source_session=_session(moved, baselines=rebuilt)), + entry={"source": pinned}, + category="skills", + ) + + +def test_a_non_strict_alias_cannot_advance_a_strict_pin(rig): + """One repository, one ref, declared twice at two modes — and the pin holds. + + This configuration is legal and not even exotic: an author says "these + entries may follow the branch, that one may not", and both read the same + repository. When the ref moves, the lax declaration delivers the new commit + and records it; the pinned one refuses. + + The danger is what the *next* apply then reads. If both declarations shared + a baseline, the sha the lax one recorded would become the pin's baseline, + and the next apply — with nothing in the document changed — would hand the + pinned entry the very commit it had just rejected. That is strict mode + degraded to "refuse each move exactly once, then deliver it", which is the + failure the adopt-after-the-gate ordering exists to prevent; it would just + have come in sideways, through a different declaration, one apply later. + + ``mode`` is in the baseline key so the two keep separate histories. + """ + _, _, pipeline = rig + old = "b" * 40 + sources = { + "pinned": {"protocol": "git", "url": GIT_URL, "ref": "main", + "mode": "strict"}, + "loose": {"protocol": "git", "url": GIT_URL, "ref": "main", + "mode": "non_strict"}, + } + + # Apply N: the ref has moved off `old`, and both declarations see it. + git = _ScriptedGit() + session = _session( + git, sources=sources, + baselines={ + (GIT_URL, "main", "strict"): old, + (GIT_URL, "main", "non_strict"): old, + }, + ) + ctx = make_context(source_session=session) + + loose = pipeline.resolve(ctx, entry={"from": "loose"}, category="skills") + assert isinstance(loose, GitDelivery) + assert loose.note() and old in loose.note() # delivered, and the move noted + with pytest.raises(EntryFetchError, match="moved"): + pipeline.resolve(ctx, entry={"from": "pinned"}, category="skills") + + # The report carries the lax delivery — provenance is not sacrificed — but + # it is stamped with the mode it was resolved under, and the refused + # declaration adopted nothing. + rows = session.resolution_records() + assert [(r.name, r.mode, r.resolved_sha) for r in rows] == [ + ("loose", "non_strict", _FAKE_SHA) + ] + + # Apply N+1, document unchanged: the baselines the service rebuilds from + # that report leave the strict key untouched, so the pin still refuses. + rebuilt = {(r.url, r.ref, r.mode): r.resolved_sha for r in rows} + # Stated as an equality rather than "the strict key is absent": absent is + # also what an empty map gives, and a map that silently stopped carrying + # the mode would satisfy the weaker form for the wrong reason. + assert rebuilt == {(GIT_URL, "main", "non_strict"): _FAKE_SHA} + next_session = _session( + git, sources=sources, + baselines={**{(GIT_URL, "main", "strict"): old}, **rebuilt}, + ) + with pytest.raises(EntryFetchError, match="moved"): + pipeline.resolve( + make_context(source_session=next_session), + entry={"from": "pinned"}, + category="skills", + ) + + +def test_strict_passes_when_the_document_re_pins_the_ref(rig): + """Editing ``ref`` is how a strict source is advanced, and it has to be: + a baseline is a fact about a ``(url, ref)`` pair, and the pair the + document now names is one no apply has resolved yet. Keyed on the source's + name instead, this could never pass — the name did not change — and the + only way out of a strict pin would be to flip the source to + ``non_strict``, apply once, and flip it back.""" + _, _, pipeline = rig + git = _ScriptedGit() + ctx = make_context( + source_session=_session( + git, baselines={(GIT_URL, "v1", "strict"): "b" * 40} + ) + ) + decl = pipeline.resolve( + ctx, + entry={"source": {"protocol": "git", "url": GIT_URL, "ref": "v2", + "mode": "strict"}}, + category="skills", + ) + assert isinstance(decl, GitDelivery) + # Not a move: nothing is noted, and the new pair is what this apply now + # stands behind, so the next apply pins against v2. + assert decl.note() is None + recorded = ctx.source_session.resolution_records() + assert [(r.url, r.ref, r.resolved_sha) for r in recorded] == [ + (GIT_URL, "v2", _FAKE_SHA) + ] + + +def test_strict_passes_when_the_document_re_points_the_url(rig): + """The other half of the same rule. A baseline from one repository has no + standing over another — keyed on the name, a re-pointed ``url`` would + silently inherit the old repository's SHA and refuse a commit that never + could have matched it.""" + _, _, pipeline = rig + git = _ScriptedGit() + other = "https://git.corp/other.git" + ctx = make_context( + source_session=_session( + git, baselines={(GIT_URL, "main", "strict"): "b" * 40} + ) + ) + decl = pipeline.resolve( + ctx, + entry={"source": {"protocol": "git", "url": other, "ref": "main", + "mode": "strict"}}, + category="skills", + ) + assert isinstance(decl, GitDelivery) + assert decl.note() is None + recorded = ctx.source_session.resolution_records() + assert [(r.url, r.ref) for r in recorded] == [(other, "main")] + + +def test_a_sha_shaped_ref_trips_neither_branch(rig): + """What the schema doc has always promised: ``mode`` is "accepted but + inert" on a ``ref`` that is already a commit. It holds by construction + rather than by a special case — a SHA resolves to itself, so the pair + ``(url, )`` always answers with the sha its baseline holds.""" + _, _, pipeline = rig + git = _ScriptedGit() + ctx = make_context( + source_session=_session( + git, baselines={(GIT_URL, _FAKE_SHA, "strict"): _FAKE_SHA} + ) + ) + decl = pipeline.resolve( + ctx, + entry={"source": {"protocol": "git", "url": GIT_URL, "ref": _FAKE_SHA, + "mode": "strict"}}, + category="skills", + ) + assert isinstance(decl, GitDelivery) + assert decl.note() is None + + +def test_non_strict_does_not_call_a_re_pin_a_move(rig): + """The note answers "the ref moved under you". A document that edited its + own ``ref`` moved it deliberately and does not need telling — and a note + naming the previous ref's commit as what this one drifted from would be + describing a drift that never happened.""" + _, _, pipeline = rig + git = _ScriptedGit() + ctx = make_context( + source_session=_session( + git, baselines={(GIT_URL, "v1", "non_strict"): "b" * 40} + ) + ) + decl = pipeline.resolve( + ctx, + entry={"source": {"protocol": "git", "url": GIT_URL, "ref": "v2"}}, + category="skills", + ) + assert isinstance(decl, GitDelivery) + assert decl.note() is None + + def test_digest_on_a_git_source_is_refused(rig): _, _, pipeline = rig ctx = make_context(source_session=_session(_ScriptedGit())) @@ -698,7 +1037,7 @@ def test_git_keep_last_falls_back_to_the_baseline_receipt(rig): ctx = make_context(source_session=_session( git, sources={"app": {"protocol": "git", "url": GIT_URL, "ref": "main", "subpath": "pkg"}}, - baselines={"app": old_sha}, + baselines={(GIT_URL, "main", "non_strict"): old_sha}, )) result = pipeline.resolve( ctx, @@ -712,6 +1051,37 @@ def test_git_keep_last_falls_back_to_the_baseline_receipt(rig): assert result.note() and "keep_last" in result.note() +def test_git_keep_last_has_no_receipt_to_reuse_after_a_re_pin(rig): + """``keep_last`` reuses the stored copy of *what this pair last + resolved to*. After a re-pin there is no such copy — the baseline under + the old ``(url, ref)`` belongs to the ref the document just stopped + naming — so the fetch failure stands rather than delivering the previous + pin's bytes under the new one's name.""" + content, _, pipeline = rig + old_sha = "b" * 40 + baseline_url = git_receipt_url(GIT_URL, old_sha, "pkg") + content.store( + fetched_object(b"stored-tree-zip", url=baseline_url, + content_type="application/zip"), + scope=None, source_url=baseline_url, + ) + git = _ScriptedGit(error=FetchFailedError("git fetch failed")) + ctx = make_context(source_session=_session( + git, + sources={"app": {"protocol": "git", "url": GIT_URL, "ref": "v2", + "subpath": "pkg"}}, + # Recorded against the ref the document used to name. + baselines={(GIT_URL, "v1", "non_strict"): old_sha}, + )) + with pytest.raises(EntryFetchError, match="git fetch failed"): + pipeline.resolve( + ctx, + entry={"from": "app", "on_fetch_failure": "keep_last"}, + category="skills", + entry_identity="s1", + ) + + def test_git_credentials_reach_the_transport_as_headers(rig): _, credentials, pipeline = rig git = _ScriptedGit() @@ -851,10 +1221,10 @@ def test_a_source_with_no_subpath_takes_the_entrys_whole(rig): def test_two_entries_off_one_git_source_share_a_checkout_and_a_sha(rig): """One source, two paths, one fetch, one ``resolved_sha``. - The composition must not cost a second checkout: the cache and the report - identity key on ``(url, ref)`` and on the source's *name*, neither of which - an entry's subpath changes. If it did, the report would carry two rows for - one declared source and the strict-mode baseline would have two answers. + The composition must not cost a second checkout: the cache keys on + ``(url, ref)`` and the report row on the source's *display*, neither of + which an entry's subpath changes. If it did, the report would carry two + rows for one declared source. """ _, _, pipeline = rig git = _ScriptedGit() diff --git a/src/backend/tests/community/core/bot_config_manifest/apply/test_source_session.py b/src/backend/tests/community/core/bot_config_manifest/apply/test_source_session.py index 83d380cef..70d4ae0d1 100644 --- a/src/backend/tests/community/core/bot_config_manifest/apply/test_source_session.py +++ b/src/backend/tests/community/core/bot_config_manifest/apply/test_source_session.py @@ -57,8 +57,8 @@ def _spec(url: str = "https://git.corp/r.git", ref: str = "main") -> GitSourceSp def test_one_url_ref_pair_is_fetched_once_and_freshness_is_reported(): git = FakeGitClient(result=CHECKOUT) session = SourceSession(sources={}, baselines={}, git=git) - first, fresh = session.checkout(_spec(), headers={}, display="src") - second, again = session.checkout(_spec(), headers={}, display="src") + first, fresh = session.checkout(_spec(), headers={}) + second, again = session.checkout(_spec(), headers={}) # Same checkout object back, one underlying fetch for the pair — and only # the first caller is told it moved the bytes, so the ledger charges once. assert first is second @@ -73,13 +73,18 @@ def test_one_url_ref_pair_is_fetched_once_and_freshness_is_reported(): def test_adoption_records_the_resolution_once_per_display(): git = FakeGitClient(result=CHECKOUT) session = SourceSession(sources={}, baselines={}, git=git) - checkout, _ = session.checkout(_spec(), headers={}, display="src") + checkout, _ = session.checkout(_spec(), headers={}) session.adopt(display="src", spec=_spec(), checkout=checkout, auth_name="ci") for _ in range(2): session.adopt(display="src", spec=_spec(), checkout=checkout, auth_name="ci") assert session.resolution_records() == ( SourceResolution( - name="src", ref="main", resolved_sha="a" * 40, auth="ci" + name="src", + url="https://git.corp/r.git", + ref="main", + mode="non_strict", + resolved_sha="a" * 40, + auth="ci", ), ) @@ -87,11 +92,9 @@ def test_adoption_records_the_resolution_once_per_display(): def test_distinct_refs_or_urls_fetch_distinctly(): git = FakeGitClient(result=CHECKOUT) session = SourceSession(sources={}, baselines={}, git=git) - session.checkout(_spec(), headers={}, display="src") - session.checkout(_spec(ref="dev"), headers={}, display="src") - session.checkout( - _spec(url="https://git.corp/other.git"), headers={}, display="src2" - ) + session.checkout(_spec(), headers={}) + session.checkout(_spec(ref="dev"), headers={}) + session.checkout(_spec(url="https://git.corp/other.git"), headers={}) assert len(git.requests) == 3 @@ -99,7 +102,7 @@ def test_a_fetch_failure_is_raised_and_caches_nothing(): git = FakeGitClient(error=FetchFailedError("git fetch failed")) session = SourceSession(sources={}, baselines={}, git=git) try: - session.checkout(_spec(), headers={}, display="src") + session.checkout(_spec(), headers={}) raise AssertionError("expected FetchFailedError") except FetchFailedError: pass @@ -119,15 +122,29 @@ def test_close_is_idempotent_and_deregisters(monkeypatch): ) git = FakeGitClient(result=CHECKOUT) session = SourceSession(sources={}, baselines={}, git=git) - session.checkout(_spec(), headers={}, display="src") + session.checkout(_spec(), headers={}) session.close() session.close() assert removed == [Path("/tmp/x")] -def test_baseline_reads_the_map_not_a_repository(): +def test_baseline_reads_the_map_by_url_ref_and_mode(): + """The key is the repository, the ref and the mode — never the document's + name for them: strict mode asks "did this pair resolve differently under + this mode", and a rename is not that question.""" session = SourceSession( - sources={}, baselines={"src": "b" * 40}, git=FakeGitClient() + sources={}, + baselines={("https://git.corp/r.git", "main", "strict"): "b" * 40}, + git=FakeGitClient(), + ) + assert ( + session.baseline("https://git.corp/r.git", "main", "strict") == "b" * 40 ) - assert session.baseline("src") == "b" * 40 - assert session.baseline("unknown") is None + # Same repository, another ref — a re-pin, so no opinion. + assert session.baseline("https://git.corp/r.git", "v2", "strict") is None + # Same ref, another repository — likewise. + assert session.baseline("https://git.corp/other.git", "main", "strict") is None + # Same pair, the OTHER mode: a separate history, so no opinion either. This + # is what stops a non_strict declaration of a repository from handing a + # strict declaration of it the commit the strict one just refused. + assert session.baseline("https://git.corp/r.git", "main", "non_strict") is None