From dd835cd1b64490b49fee87cca0781613582dcef3 Mon Sep 17 00:00:00 2001 From: totalfrank Date: Fri, 11 Sep 2026 14:30:08 +0000 Subject: [PATCH 1/8] feat(apply): record the repository url on every source resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A SourceResolution named the source and the ref it declared, but not the repository those belonged to. The report could therefore say "content resolved to 7c1d…" without saying what "content" was, and anything reading the report back had no way to ask "is this the same repository we resolved last time". Adds `url` — the substituted repository URL — to SourceResolution, its wire shape, and the decode that reads a stored report back. It is report-safe by construction: this record carries names, never values, and the URL is the document's own after `${BOT_*}` substitution. The report's `sources` array also stops being a passthrough of raw dicts on the HTTP surface: it gains a named response model, so every field a caller can read is spelled out there, the way `categories` and `entries` already are. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01U8YX8W4uDYiHjcX9iRBLje --- .../bots/config_manifest_support.py | 5 +- .../bots/schemas_config_manifest_apply.py | 49 ++++++++++++++++--- .../bot_config_manifest/apply/outcomes.py | 43 +++++++++------- .../apply/source_session.py | 5 +- .../services/apply_report_codec.py | 1 + .../apply/test_apply_engine.py | 7 ++- .../apply/test_source_session.py | 6 ++- 7 files changed, 87 insertions(+), 29 deletions(-) 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 fded3858bb..ee0cc84e2d 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 c40a253c0b..9adcf8433c 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,42 @@ 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)` 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` it is what strict mode " + "compares the next apply against — so re-pointing either 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.", + ) + 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 +167,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 179818dd93..a7ba65454c 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,23 @@ 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 - ) + SourceResolution(name="content", + url="https://code.example.com/team/content.git", + ref="v1.2.0", resolved_sha="7c1d…", auth="gh-readonly") - An inline git source has no ``from`` name, so it is recorded under its - repository URL instead:: + 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", - ref="HEAD", - resolved_sha="4f2a9c1b8e7d6a5c4b3a2918f7e6d5c4b3a29187", - auth=None, - ) + SourceResolution(name="https://code.example.com/team/content.git@main", + url="https://code.example.com/team/content.git", + ref="main", resolved_sha="9e8d…", 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. + ``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)`` 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,8 +369,13 @@ 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. It is + #: half of the key the next apply reads its baseline by, and is ``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 @@ -384,6 +387,7 @@ class SourceResolution: def as_dict(self) -> dict[str, Any]: return { "name": self.name, + "url": self.url, "ref": self.ref, "resolved_sha": self.resolved_sha, "auth": self.auth, @@ -415,6 +419,7 @@ class ApplyReport: sources=( SourceResolution( name="content", + url="https://code.example.com/team/content.git", ref="v1.2.0", resolved_sha="4f2a9c1b8e7d6a5c4b3a2918f7e6d5c4b3a29187", auth="git-prod", @@ -472,7 +477,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", "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_session.py b/src/backend/src/agentclaw/community/core/bot_config_manifest/apply/source_session.py index 5110e752ab..743e1608fb 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 @@ -186,7 +186,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", resolved_sha="4f2a9c1b8e7d6a5c4b3a2918f7e6d5c4b3a29187", auth="git-prod") @@ -205,6 +207,7 @@ def adopt( self._resolutions.append( SourceResolution( name=display, + url=spec.url, ref=spec.ref, resolved_sha=checkout.sha, auth=auth_name, 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 64019dbc9f..1c9ff141ce 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,6 +84,7 @@ def report_from_payload( sources=tuple( SourceResolution( name=source.get("name", ""), + url=source.get("url"), ref=source.get("ref"), resolved_sha=source.get("resolved_sha"), auth=source.get("auth"), 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 bdad659178..9434ce1ba0 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,11 @@ 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", + 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,6 +896,7 @@ 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", "resolved_sha": "f" * 40, "auth": "ci-token", 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 83d380cefb..54bf76b7d2 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 @@ -79,7 +79,11 @@ def test_adoption_records_the_resolution_once_per_display(): 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", + resolved_sha="a" * 40, + auth="ci", ), ) From 4ee9c7f1dd017d860470d11765f0d64f18d9b4a0 Mon Sep 17 00:00:00 2001 From: totalfrank Date: Fri, 11 Sep 2026 14:34:13 +0000 Subject: [PATCH 2/8] fix(apply): key strict-mode baselines on (url, ref), not the source name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Strict mode answers "has this repository's ref moved since this bot last resolved it". The name the document happens to give the source is not part of that question, and keying the baseline on it made the answer wrong in four ways: * A `mode: strict` source could never be advanced by editing the document. Changing `ref` to a new tag — or to a commit SHA — keeps the name, so the old baseline still applied and the new resolution was refused. The only way out was to flip the source to `non_strict`, apply once, and flip it back, which is to say: to disarm the pin in order to move it. * The schema doc's promise that a SHA-shaped `ref` trips neither branch was not kept by either branch: such a source was still refused against a stale baseline under strict, and still got a "ref moved" note under non_strict. * Renaming a source dropped its baseline; re-pointing its `url` kept a baseline belonging to a different repository. The baseline map is now `(url, ref) -> sha`, read back off each report row's `url` and `ref` and looked up at the gate by the spec's own pair. Everything after that lookup is unchanged: the refusal still happens before adoption, `keep_last` still reads the receipt filed under the baseline sha, and the walk back through report history still lets an outage pass without disarming the pin. A re-pinned `ref` now simply has no baseline, which is the same state a first-time source is in, so it passes and is adopted under its new pair. Rows carrying no `url` contribute no baseline: guessing which repository an old row's name meant would be inventing a pin nobody wrote. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01U8YX8W4uDYiHjcX9iRBLje --- .../apply/source_fetchers.py | 33 +++-- .../apply/source_session.py | 68 +++++---- .../services/config_manifest_apply_service.py | 33 +++-- .../apply/test_apply_service_lifecycle.py | 68 +++++++-- .../apply/test_identity_materialiser.py | 2 +- .../apply/test_skills_materialiser.py | 4 +- .../apply/test_source_resolver.py | 132 +++++++++++++++++- .../apply/test_source_session.py | 16 ++- 8 files changed, 290 insertions(+), 66 deletions(-) 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 b8f2ce75ff..dd2c51e749 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 @@ -265,7 +265,8 @@ class DeclaredFetch: #: 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. + #: the report. The baseline is not read by it: that is keyed on the + #: substituted ``(url, ref)``. name: Optional[str] @@ -531,10 +532,11 @@ 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)``, + 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__( @@ -597,7 +599,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 +617,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) if ( spec.mode == "strict" and baseline is not None @@ -630,7 +631,11 @@ 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), so editing either in the + # document 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. session.adopt( display=display, spec=spec, checkout=checkout, auth_name=auth ) @@ -666,24 +671,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)`` 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) 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 743e1608fb..23780e3cb9 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,22 @@ """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)`` — from the last apply that resolved that pair, a +checkout cache keyed the same way, 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. 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 +86,27 @@ 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)`` → the 40-character SHA the last + #: apply that resolved that pair recorded. Read back out of + #: ``ApplyReport.sources`` through the report history, off each row's + #: ``url`` and ``ref``:: #: #: { - #: "content": "4f2a9c1b8e7d6a5c4b3a2918f7e6d5c4b3a29187", - #: "https://code.example.com/solo.git": "9b1c...", + #: ("https://code.example.com/team/content.git", "v1.2.0"): + #: "4f2a9c1b8e7d6a5c4b3a2918f7e6d5c4b3a29187", + #: ("https://code.example.com/solo.git", "HEAD"): "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. + #: + #: A pair 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] #: The git transport; injected so tests script it and production gets the #: subprocess client via the DI provider. git: GitSourceClient @@ -198,8 +214,9 @@ 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; a display is a name or ``url@ref``, + so every entry that names one source stands behind one resolution, and + two names over one repository record one row each. """ if display in self._recorded: return @@ -220,13 +237,16 @@ 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) -> Optional[str]: + """The SHA the last apply that resolved this ``(url, ref)`` 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")`` → + ``"4f2a9c1b8e7d6a5c4b3a2918f7e6d5c4b3a29187"`` for a pair resolved + before, ``None`` for one seen the first time — which includes a pair + the document has just re-pinned onto. """ - return self.baselines.get(display) + return self.baselines.get((url, ref)) 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/config_manifest_apply_service.py b/src/backend/src/agentclaw/community/core/bot_config_manifest/services/config_manifest_apply_service.py index ea93289ac5..248862603b 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]: + """Each ``(url, ref)``'s SHA as the last apply that RESOLVED it (W7 + strict). The reports are where "what did we resolve" already lives (``ApplyReport.sources``), so strict mode reads them back rather than @@ -767,9 +768,21 @@ 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; a report with no resolutions — or no + reports — yields no opinions. + + The key is ``(url, ref)`` and not the row's ``name``, because that is + the question strict mode asks. One report may hold several rows under + one key — the report has a row per declaration, so two ``from`` names + over one repository are two rows — and they always carry the same sha, + because one apply resolves a ``(url, ref)`` once. Any of them + therefore serves, and the ``setdefault`` that keeps the newest report's + answer keeps the first of them too. + + A row carrying no ``url`` contributes nothing: that is a report written + before the field existed, and inventing a baseline out of a name would + be guessing which repository it meant. """ records = self._applies.recent( env=get_current_env(), @@ -777,16 +790,20 @@ def _last_resolutions( bot_id=bot_id, limit=_BASELINE_HISTORY_APPLIES, ) - baselines: dict[str, str] = {} + baselines: dict[tuple[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.resolved_sha is None: continue # Newest wins: an earlier walk-back entry is not overwritten. - baselines.setdefault(source.name, source.resolved_sha) + # ``ref`` is normalised the way the spec normalises it, so a + # source that declared none keys on "HEAD" in both directions. + baselines.setdefault( + (source.url, source.ref or "HEAD"), source.resolved_sha + ) return baselines # ── internals ─────────────────────────────────────────────────────────── 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 662bd6f221..bf7d0ef718 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,11 @@ 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", + resolved_sha="f" * 40, + auth="ci-token", ) monkeypatch.setattr( service._applies, @@ -698,7 +703,44 @@ 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"): "f" * 40} + + +def test_baselines_are_read_by_url_and_ref_and_skip_rows_without_a_url( + world, monkeypatch +): + """The key is the repository and the ref, which has two consequences here. + + Several rows in one report may share a key — the report carries one row + per *declaration*, so two ``from`` names over one repository are two rows + — and they always carry the same sha, because one apply resolves a + ``(url, ref)`` once. And a row with no ``url`` is skipped outright: that + is a report written before the url was recorded, and there is no honest + way to guess which repository its name meant. + """ + service, _applies, _locks, _scripts, _manifests = world + url = "https://git.corp/charts.git" + rows = [ + SourceResolution(name="charts", url=url, ref="main", + resolved_sha="f" * 40), + # A second declaration of the same repository at the same ref. + SourceResolution(name="dashboards", url=url, ref="main", + resolved_sha="f" * 40), + # Same repository, another ref: its own key, its own answer. + SourceResolution(name="pinned", url=url, ref="v1", + resolved_sha="c" * 40), + # Pre-existing history: no url, 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"): "f" * 40, + (url, "v1"): "c" * 40, + } def test_a_failed_apply_does_not_wipe_a_strict_baseline(world, monkeypatch): @@ -708,7 +750,12 @@ 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", + resolved_sha="e" * 40, + ) empty_failed = _report_with_sources( [], status=ApplyStatus.FAILED, apply_id="failed-1" ) @@ -722,11 +769,16 @@ 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"): "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", + resolved_sha="b" * 40, + ) monkeypatch.setattr( service._applies, "recent", @@ -736,7 +788,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"): "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 55ccf5cf93..6776f11848 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"): "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 6e11c00c0c..83f9965d97 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,7 @@ 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"): "b" * 40}, ) result, _, written = _run(_apply(materialiser, ctx, [{"name": "demo", "from": "src"}])) assert result.ok @@ -739,7 +739,7 @@ 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"): "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 e43f7448e7..5e4e3c5b14 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,11 @@ 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"): "b" * 40}) ) with pytest.raises(EntryFetchError, match="moved"): pipeline.resolve( @@ -648,7 +649,7 @@ 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"): "b" * 40}) ) decl = pipeline.resolve( ctx, @@ -673,6 +674,96 @@ def test_strict_on_the_first_apply_has_no_opinion(rig): assert decl.note() is None +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"): "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"): "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): _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"): "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 +789,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"): old_sha}, )) result = pipeline.resolve( ctx, @@ -712,6 +803,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"): 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() 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 54bf76b7d2..8816c6d9bf 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 @@ -129,9 +129,17 @@ def test_close_is_idempotent_and_deregisters(monkeypatch): assert removed == [Path("/tmp/x")] -def test_baseline_reads_the_map_not_a_repository(): +def test_baseline_reads_the_map_by_url_and_ref(): + """The key is the repository and the ref, never the document's name for + them: strict mode asks "did this pair resolve differently", and a rename + is not that question.""" session = SourceSession( - sources={}, baselines={"src": "b" * 40}, git=FakeGitClient() + sources={}, + baselines={("https://git.corp/r.git", "main"): "b" * 40}, + git=FakeGitClient(), ) - assert session.baseline("src") == "b" * 40 - assert session.baseline("unknown") is None + assert session.baseline("https://git.corp/r.git", "main") == "b" * 40 + # Same repository, another ref — a re-pin, so no opinion. + assert session.baseline("https://git.corp/r.git", "v2") is None + # Same ref, another repository — likewise. + assert session.baseline("https://git.corp/other.git", "main") is None From df56a5e7a69eeaa3df37a170549c04b02fc5aff6 Mon Sep 17 00:00:00 2001 From: totalfrank Date: Fri, 11 Sep 2026 14:36:15 +0000 Subject: [PATCH 3/8] refactor(apply): name an inline git source url@ref in the report MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The report carries one row per declaration. An inline source has no `from` name to report under, and the URL alone was not an identity: two entries reading one repository at two refs de-duplicated onto a single row that named neither ref, so the report said a sha without saying which ref produced it. An inline source is now named `@`, with the ref already normalised to `HEAD` when the declaration omitted one. Strict refusals read better for the same reason — the message names the ref it refused, not just the repository. `SourceSession.checkout` loses its `display` parameter. It never used it except to explain itself: the display was the baseline key, and it no longer is. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01U8YX8W4uDYiHjcX9iRBLje --- .../apply/source_fetchers.py | 21 +++--- .../apply/source_session.py | 15 ++-- .../apply/test_source_resolver.py | 70 +++++++++++++++++-- .../apply/test_source_session.py | 18 +++-- 4 files changed, 94 insertions(+), 30 deletions(-) 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 dd2c51e749..285c40082f 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,10 +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. The baseline is not read by it: that is keyed on the - #: substituted ``(url, ref)``. + #: 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)``. name: Optional[str] @@ -576,7 +576,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: @@ -585,9 +592,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: 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 23780e3cb9..68af3465d5 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 @@ -135,9 +135,11 @@ class SourceSession: #: 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:: + #: idempotent per display, 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/solo.git"} + #: {"content", "https://code.example.com/solo.git@main"} _recorded: set[str] = field(default_factory=set) def checkout( @@ -145,7 +147,6 @@ def checkout( spec: GitSourceSpec, *, headers: Mapping[str, str], - display: str, ) -> "tuple[GitCheckout, bool]": """The checkout for one ``(url, ref)``, fetching only the first time. @@ -166,11 +167,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. 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 5e4e3c5b14..ed3a2fd548 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 @@ -674,6 +674,68 @@ 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"): "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)``. A + document that names one repository twice 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) for r in records} == {(GIT_URL, "main")} + assert len(git.specs) == 1, "one checkout per (url, ref) per apply" + + 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 @@ -973,10 +1035,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 8816c6d9bf..5cb21d47ca 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,7 +73,7 @@ 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") @@ -91,11 +91,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 @@ -103,7 +101,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 @@ -123,7 +121,7 @@ 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")] From f22d94f5839fe9d14538adae62b9f46f25786f8b Mon Sep 17 00:00:00 2001 From: totalfrank Date: Fri, 11 Sep 2026 14:37:58 +0000 Subject: [PATCH 4/8] docs(manifest): strict mode is per (url, ref); a re-pin is not a move MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The schema doc promised that a SHA-shaped `ref` trips neither branch of `mode`, and did not say what the baseline was actually keyed on. Both now follow from one sentence: the baseline is per `(url, ref)`, so changing either is a re-pin — no baseline, no refusal, no note — and a SHA-shaped ref can never trip either branch because it only ever resolves to itself. Also documents how a `strict` source is advanced (edit the ref; there is no flip to `non_strict` and back), and what the report's `sources` rows now carry: a `url` field, one row per declaration, and `url@ref` as the name of a source written inline on an entry. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01U8YX8W4uDYiHjcX9iRBLje --- .../docs/bot-config-manifest/design.zh-CN.md | 14 ++++++--- .../manifest-schema.zh-CN.md | 16 +++++++--- .../bot-config-manifest/user-manual.zh-CN.md | 31 ++++++++++++++----- 3 files changed, 46 insertions(+), 15 deletions(-) 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 bb0ab44ffe..5c992a03a1 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", "resolved_sha": "9c1f4ae…"}, + {"name": "https://code.example.com/team/tools.git@main", + "url": "https://code.example.com/team/tools.git", + "ref": "main", "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)` 从这里读回去 +(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 7c67d77774..3b7d502733 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,21 @@ manifest: | `mode` | 行为 | | --- | --- | | `non_strict`(**默认**) | 应用新内容,并在 apply report 里对该条目**告警**,写明前后两个 SHA | -| `strict` | 解析出的 SHA 与上次 apply 记录的不同时,该条目**失败**,bot 继续跑它现在跑的 | - +| `strict` | 同一个 `(url, ref)` 这次解析出的 SHA 与上次 apply 记录的不同时,该条目**失败**,bot 继续跑它现在跑的 | + +- **基线按 `(url, ref)` 记**,不按源名。两个分支问的都是同一件事:「**这个仓库 + 的这个 ref**,在我们上次解析它之后动过没有」。源叫什么是文档的事,跟这个问题 + 无关——改名不会丢基线,把 `url` 指向另一个仓库也不会继承前一个仓库的 SHA。 +- **改 `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 4aa8b5f0c7..e8ea62bcfb 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", "resolved_sha": "9c1f4ae…"}, + {"name": "https://code.example.com/team/tools.git@main", + "url": "https://code.example.com/team/tools.git", + "ref": "main", "resolved_sha": "7e3b91c…"} ], "entries": [ {"category": "identity", "name": "SOUL.md", "action": "updated", "from": "content"}, @@ -1020,12 +1024,20 @@ schema 已定稿(见 `manifest-schema.zh-CN.md` §3.4),但**第一期没 | `mode` | 行为 | | --- | --- | | `non_strict`(**默认**) | 应用新内容,并在 apply 报告里对该条目**告警**,写明前后两个 SHA | -| `strict` | 解析出的 SHA 与上次 apply 记录的不同时,该条目**失败**,bot 继续跑它现在跑的 | - +| `strict` | 同一个 `(url, ref)` 这次解析出的 SHA 与上次 apply 记录的不同时,该条目**失败**,bot 继续跑它现在跑的 | + +- **基线按 `(url, ref)` 记**,不按源名。两个分支问的都是同一件事:「这个仓库的 + 这个 ref,在我们上次解析它之后动过没有」——源叫什么是你文档里的事,跟这个问题 + 无关。改名不丢基线;把 `url` 指到另一个仓库也不会继承前一个仓库的 SHA。 +- **改 `ref`(或改 `url`)就是一次重新钉扎**:新的 `(url, ref)` 没有任何一次 + apply 对它有意见,所以既不拒绝也不告警,照常解析并被这次 apply 记下。 + **这就是 `strict` 源的升版方式**——不用先切 `non_strict` 应用一次再切回来。 + `strict` 拒绝的只有一种情况:**你没改文档,而 ref 在脚下动了**。 +- **SHA 形式的 ref 两个分支都触发不了**——它只会解析成它自己。是「接受但无效」, + 不是报错。 - **写在源上**,不是按 bot、也不是按清单——要描述的性质是「这个 ref 允不允许在我 脚下移动」,它属于持有 ref 的那个东西。一份清单里同时有一个钉死的外部依赖和一个 快速变动的内部仓库是常态。 -- **SHA 形式的 ref 忽略这个模式**(它动不了)——是「接受但无效」,不是报错。 - 拼错的取值会被拒绝,不会静默落到默认值。 ### 6.3 `digest`:哪里强制、哪里非法 @@ -1233,7 +1245,8 @@ bot 也是队列上的一个任务。所以部署里必须满足两个前提: - 用的是 **tag 且没动**?那就是没变——改 `ref`(§4.8)。 - 用的是 **branch 且 `mode: strict`**?SHA 变了会让该条目**失败**,这是你要的钉扎 - 语义。看报告里的前后 SHA。 + 语义。看报告里的前后 SHA。要让它跟上,就在文档里把 `ref` 改成你真正想要的那个 + tag 或 commit——换了 `ref` 就是一次重新钉扎,不会被拒绝(§6.2)。 - 取源失败并落到了 **`keep_last`**?报告里那一条会写明。 --- @@ -1738,7 +1751,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 +1760,15 @@ 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` 合起来就是 `strict` 基线的键(§6.2) | | `ref` | string \| null | 声明的 ref:tag / branch / commit SHA | | `resolved_sha` | string \| null | 这一次**实际解析到**的 commit。`ref: main` 这种会动的引用,下周就是另一个值 | | `auth` | string \| null | 用到的凭证**名**。**永远只有名字,没有值** | +**一条声明一行**:两个 `from` 名指向同一个 `(url, ref)` 就是两行(各自带着作者 +写下的那个名字),`resolved_sha` 相同;同一个仓库内联声明两个 `ref` 也是两行。 + `categories[]`: | 字段 | 类型 | 含义 | From 4a576af14af4cbb36f4619b586bbf436068dac3a Mon Sep 17 00:00:00 2001 From: totalfrank Date: Fri, 11 Sep 2026 14:57:22 +0000 Subject: [PATCH 5/8] fix(apply): keep the apply service under the module line cap `_last_resolutions`' new docstring pushed `config_manifest_apply_service.py` from 996 to 1013 lines, over the 1000-line cap `tests/community/architecture/test_no_oversized_modules.py` enforces. Says the same things in fewer words; the longer reasoning about why a baseline is keyed on the pair already lives in `apply/source_session`, which is where it belongs. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01U8YX8W4uDYiHjcX9iRBLje --- .../services/config_manifest_apply_service.py | 31 ++++++------------- 1 file changed, 9 insertions(+), 22 deletions(-) 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 248862603b..d746c8d6d6 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 @@ -758,8 +758,8 @@ def last_apply( def _last_resolutions( self, *, entity_id: str, bot_id: str ) -> dict[tuple[str, str], str]: - """Each ``(url, ref)``'s SHA as the last apply that RESOLVED it (W7 - strict). + """Each ``(url, ref)``'s SHA as the last apply that RESOLVED it — the + pair, 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 @@ -769,20 +769,10 @@ def _last_resolutions( 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 key, the newest - report that carries it wins; a report with no resolutions — or no - reports — yields no opinions. - - The key is ``(url, ref)`` and not the row's ``name``, because that is - the question strict mode asks. One report may hold several rows under - one key — the report has a row per declaration, so two ``from`` names - over one repository are two rows — and they always carry the same sha, - because one apply resolves a ``(url, ref)`` once. Any of them - therefore serves, and the ``setdefault`` that keeps the newest report's - answer keeps the first of them too. - - A row carrying no ``url`` contributes nothing: that is a report written - before the field existed, and inventing a baseline out of a name would - be guessing which repository it meant. + report that carries it wins; rows sharing a key (the report holds one + per declaration) carry the same sha, so any serves. No reports, no + resolutions in them, or no ``url`` on a row — an older one, whose name + names no repository to pin — all yield no opinion. """ records = self._applies.recent( env=get_current_env(), @@ -798,12 +788,9 @@ def _last_resolutions( for source in report.sources: if source.url is None or source.resolved_sha is None: continue - # Newest wins: an earlier walk-back entry is not overwritten. - # ``ref`` is normalised the way the spec normalises it, so a - # source that declared none keys on "HEAD" in both directions. - baselines.setdefault( - (source.url, source.ref or "HEAD"), source.resolved_sha - ) + # Newest wins; "HEAD" normalised the way the spec does it. + key = (source.url, source.ref or "HEAD") + baselines.setdefault(key, source.resolved_sha) return baselines # ── internals ─────────────────────────────────────────────────────────── From 754329d753d32afb5005f0214b87dee3ffc58ddf Mon Sep 17 00:00:00 2001 From: totalfrank Date: Fri, 11 Sep 2026 15:33:46 +0000 Subject: [PATCH 6/8] fix(apply): a non_strict alias must not advance a strict pin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Keying baselines on `(url, ref)` alone let one declaration of a repository disarm another's pin, which a Codex review caught and a probe test confirmed. A document may legally declare the same `(url, ref)` twice — once `strict`, once `non_strict` — when some entries may follow a branch and one may not. When the ref then moves, the lax declaration delivers the new commit and records it while the pinned one refuses. Sharing a baseline, that recorded sha became the pin's baseline, so the very next apply — with nothing in the document changed — handed the pinned entry the commit it had just rejected. That is strict mode degraded to "refuse each move exactly once, then deliver it": the failure the adopt-after-the-gate ordering exists to prevent, coming in sideways through a second declaration one apply later. `mode` joins the key, because a pin may only be advanced by an apply that stood behind it under the same mode. The two declarations keep separate histories: the lax one advances and notes its moves, the pinned one goes on refusing until the document re-pins it. Provenance is not sacrificed to get there — the lax delivery still gets its report row, now stamped with the mode it was resolved under. Every property of the original change survives: a re-pinned ref or url still passes, a rename still keeps its baseline, a SHA-shaped ref still trips neither branch, and report rows are still one per declaration. The regression test asserts the rebuilt baseline map by equality rather than by the absence of the strict key — absent is also what an empty map gives, and a row that silently stopped carrying its mode would satisfy the weaker form for the wrong reason. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01U8YX8W4uDYiHjcX9iRBLje --- .../docs/bot-config-manifest/design.zh-CN.md | 6 +- .../manifest-schema.zh-CN.md | 16 ++- .../bot-config-manifest/user-manual.zh-CN.md | 18 ++- .../bots/schemas_config_manifest_apply.py | 17 ++- .../bot_config_manifest/apply/outcomes.py | 33 +++-- .../apply/source_fetchers.py | 22 ++-- .../apply/source_session.py | 64 ++++++---- .../services/apply_report_codec.py | 1 + .../services/config_manifest_apply_service.py | 18 +-- .../apply/test_apply_engine.py | 2 + .../apply/test_apply_service_lifecycle.py | 43 ++++--- .../apply/test_identity_materialiser.py | 2 +- .../apply/test_skills_materialiser.py | 8 +- .../apply/test_source_resolver.py | 115 +++++++++++++++--- .../apply/test_source_session.py | 23 ++-- 15 files changed, 277 insertions(+), 111 deletions(-) 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 5c992a03a1..34475b74c7 100644 --- a/src/backend/docs/bot-config-manifest/design.zh-CN.md +++ b/src/backend/docs/bot-config-manifest/design.zh-CN.md @@ -347,10 +347,10 @@ apply 在平台侧执行,天然产出结构化记录(#935 的 `last-start` "started_at": "…", "finished_at": "…", "result": "SUCCEEDED|PARTIAL|FAILED", "sources": [ {"name": "content", "url": "https://code.example.com/team/content.git", - "ref": "v1.2.0", "resolved_sha": "9c1f4ae…"}, + "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", "resolved_sha": "7e3b91c…"} + "ref": "main", "mode": "non_strict", "resolved_sha": "7e3b91c…"} ], "entries": [ {"category": "skills", "name": "reviewer", @@ -362,7 +362,7 @@ apply 在平台侧执行,天然产出结构化记录(#935 的 `last-start` git 源的解析结果记在顶层 `sources`(`url` + 声明的 `ref` + 解析出的 `resolved_sha`)——「这批 bot 线上跑的是哪一版内容」由此可查;**每条声明一行**, -内联源没有名字,记成 `url@ref`。`strict` 的基线按 `(url, ref)` 从这里读回去 +内联源没有名字,记成 `url@ref`。`strict` 的基线按 `(url, ref, mode)` 从这里读回去 (schema §2.3),跟这一行叫什么名字无关。条目层记 `from`(来自哪个源)或 `source_digest`(URL 源)。 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 3b7d502733..1f1aa2df88 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,11 +472,17 @@ manifest: | `mode` | 行为 | | --- | --- | | `non_strict`(**默认**) | 应用新内容,并在 apply report 里对该条目**告警**,写明前后两个 SHA | -| `strict` | 同一个 `(url, ref)` 这次解析出的 SHA 与上次 apply 记录的不同时,该条目**失败**,bot 继续跑它现在跑的 | - -- **基线按 `(url, ref)` 记**,不按源名。两个分支问的都是同一件事:「**这个仓库 - 的这个 ref**,在我们上次解析它之后动过没有」。源叫什么是文档的事,跟这个问题 - 无关——改名不会丢基线,把 `url` 指向另一个仓库也不会继承前一个仓库的 SHA。 +| `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` 应用一次再切回来。 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 e8ea62bcfb..c0fe7e1843 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 @@ -679,10 +679,10 @@ GET /openapi/v1/bots/{bot_id}/with-manifest/status "result": "SUCCEEDED|PARTIAL|FAILED", "sources": [ {"name": "content", "url": "https://code.example.com/team/content.git", - "ref": "v1.2.0", "resolved_sha": "9c1f4ae…"}, + "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", "resolved_sha": "7e3b91c…"} + "ref": "main", "mode": "non_strict", "resolved_sha": "7e3b91c…"} ], "entries": [ {"category": "identity", "name": "SOUL.md", "action": "updated", "from": "content"}, @@ -1024,11 +1024,16 @@ schema 已定稿(见 `manifest-schema.zh-CN.md` §3.4),但**第一期没 | `mode` | 行为 | | --- | --- | | `non_strict`(**默认**) | 应用新内容,并在 apply 报告里对该条目**告警**,写明前后两个 SHA | -| `strict` | 同一个 `(url, ref)` 这次解析出的 SHA 与上次 apply 记录的不同时,该条目**失败**,bot 继续跑它现在跑的 | +| `strict` | 同一个 `(url, ref, mode)` 这次解析出的 SHA 与上次 apply 记录的不同时,该条目**失败**,bot 继续跑它现在跑的 | -- **基线按 `(url, ref)` 记**,不按源名。两个分支问的都是同一件事:「这个仓库的 - 这个 ref,在我们上次解析它之后动过没有」——源叫什么是你文档里的事,跟这个问题 +- **基线按 `(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` 应用一次再切回来。 @@ -1761,8 +1766,9 @@ B.2.2 / B.2.3 / B.2.4 与 `GET …/with-manifest/status` 的 `apply` 字段都 | 字段 | 类型 | 含义 | | --- | --- | --- | | `name` | string | 源名(`sources.` 里的那个名字);**内联 `source` 没有名字,记成 `url@ref`**(省略 `ref` 时是 `url@HEAD`) | -| `url` | string \| null | 仓库地址,`${BOT_*}` 已替换。与 `ref` 合起来就是 `strict` 基线的键(§6.2) | +| `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 | 用到的凭证**名**。**永远只有名字,没有值** | 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 9adcf8433c..fea57885b3 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 @@ -67,8 +67,8 @@ 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)` are two rows carrying the same `resolved_sha`, and two - inline declarations of one repository at two refs are two rows too. + 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( @@ -78,15 +78,22 @@ class ConfigManifestApplySource(BaseModel): url: str | None = Field( default=None, description="The repository URL, with any `${BOT_*}` placeholder " - "already substituted. Together with `ref` it is what strict mode " - "compares the next apply against — so re-pointing either is a re-pin, " - "not a moved ref.", + "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 — " 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 a7ba65454c..3248aef97f 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 @@ -344,21 +344,23 @@ class SourceResolution: SourceResolution(name="content", url="https://code.example.com/team/content.git", - ref="v1.2.0", resolved_sha="7c1d…", auth="gh-readonly") + 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", resolved_sha="9e8d…", auth=None) + 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)`` 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. + 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()``. @@ -372,15 +374,24 @@ class SourceResolution: #: 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. It is - #: half of the key the next apply reads its baseline by, and is ``None`` - #: only on a row written before this field existed. + #: 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 @@ -389,6 +400,7 @@ def as_dict(self) -> dict[str, Any]: "name": self.name, "url": self.url, "ref": self.ref, + "mode": self.mode, "resolved_sha": self.resolved_sha, "auth": self.auth, } @@ -421,6 +433,7 @@ class ApplyReport: name="content", url="https://code.example.com/team/content.git", ref="v1.2.0", + mode="strict", resolved_sha="4f2a9c1b8e7d6a5c4b3a2918f7e6d5c4b3a29187", auth="git-prod", ), @@ -479,7 +492,7 @@ def as_payload(self) -> dict[str, Any]: "finished_at": "2026-03-01T09:00:04+00:00", "sources": [{"name": "content", "url": "https://code.example.com/team/content.git", - "ref": "v1.2.0", + "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 285c40082f..2eb97be07e 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 @@ -266,7 +266,7 @@ class DeclaredFetch: #: 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)``. + #: is keyed on the substituted ``(url, ref, mode)``. name: Optional[str] @@ -532,7 +532,8 @@ 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 for the same ``(url, ref)``, + 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 @@ -622,7 +623,7 @@ def fetch(self, request: DeclaredFetch) -> EntryDelivery: if expired is not None: raise EntryFetchError(expired) - baseline = session.baseline(spec.url, spec.ref) + baseline = session.baseline(spec.url, spec.ref, spec.mode) if ( spec.mode == "strict" and baseline is not None @@ -637,10 +638,13 @@ def fetch(self, request: DeclaredFetch) -> EntryDelivery: # 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". The baseline - # is the one recorded for this (url, ref), so editing either in the - # document 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. + # 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 ) @@ -681,7 +685,7 @@ def _keep_last( """``keep_last`` for the git road: the receipt of the *last-resolved* SHA, when there was one. - Looks the baseline up by ``(url, ref)`` and reads the receipt filed + 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 pair has no baseline (a first-time pair has no stored copy @@ -693,7 +697,7 @@ def _keep_last( """ if not keep_last: return None - baseline = session.baseline(spec.url, spec.ref) + 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 68af3465d5..2a834ea308 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 @@ -2,9 +2,9 @@ The four things a single apply needs and nothing more: the document's ``sources`` declarations, the strict-mode baselines read back — keyed on the -substituted ``(url, ref)`` — from the last apply that resolved that pair, a -checkout cache keyed the same way, and the :class:`SourceResolution` records -the report will carry. It hangs on ``ApplyContext`` beside ``budget``, mutable +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". @@ -18,6 +18,19 @@ 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 apply stands behind that answer" — the strict-mode refusal sits between the @@ -86,27 +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]] - #: ``(substituted repository url, ref)`` → the 40-character SHA the last - #: apply that resolved that pair recorded. Read back out of + #: ``(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`` and ``ref``:: + #: ``url``, ``ref`` and ``mode``:: #: #: { - #: ("https://code.example.com/team/content.git", "v1.2.0"): - #: "4f2a9c1b8e7d6a5c4b3a2918f7e6d5c4b3a29187", - #: ("https://code.example.com/solo.git", "HEAD"): "9b1c...", + #: ("https://code.example.com/team/content.git", "v1.2.0", + #: "strict"): "4f2a9c1b8e7d6a5c4b3a2918f7e6d5c4b3a29187", + #: ("https://code.example.com/solo.git", "HEAD", "non_strict"): + #: "9b1c...", #: } #: #: **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. + #: 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 pair absent here has no strict opinion yet, so strict mode admits its - #: first resolution and ``keep_last`` has no baseline receipt to reuse — + #: 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] + 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 @@ -203,7 +220,7 @@ def adopt( # _resolutions now ends with SourceResolution(name="content", url="https://code.example.com/team/content.git", - ref="v1.2.0", + ref="v1.2.0", mode="strict", resolved_sha="4f2a9c1b8e7d6a5c4b3a2918f7e6d5c4b3a29187", auth="git-prod") @@ -225,6 +242,7 @@ def adopt( name=display, url=spec.url, ref=spec.ref, + mode=spec.mode, resolved_sha=checkout.sha, auth=auth_name, ) @@ -236,16 +254,18 @@ def resolution_records(self) -> tuple[SourceResolution, ...]: git source, which includes every object-store-only document.""" return tuple(self._resolutions) - def baseline(self, url: str, ref: str) -> Optional[str]: - """The SHA the last apply that resolved this ``(url, ref)`` 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("https://code.example.com/team/content.git", "v1.2.0")`` → - ``"4f2a9c1b8e7d6a5c4b3a2918f7e6d5c4b3a29187"`` for a pair resolved - before, ``None`` for one seen the first time — which includes a pair - the document has just re-pinned onto. + ``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((url, ref)) + 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 1c9ff141ce..2a4c2ba041 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 @@ -86,6 +86,7 @@ def report_from_payload( 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 d746c8d6d6..cfcb1be348 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,9 +757,9 @@ def last_apply( def _last_resolutions( self, *, entity_id: str, bot_id: str - ) -> dict[tuple[str, str], str]: - """Each ``(url, ref)``'s SHA as the last apply that RESOLVED it — the - pair, never the row's ``name`` (W7; ``apply/source_session`` says why). + ) -> 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 @@ -770,9 +770,8 @@ def _last_resolutions( only that row would wipe the baseline, silently disarming strict mode 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. No reports, no - resolutions in them, or no ``url`` on a row — an older one, whose name - names no repository to pin — all yield no opinion. + 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(), @@ -780,16 +779,17 @@ def _last_resolutions( bot_id=bot_id, limit=_BASELINE_HISTORY_APPLIES, ) - baselines: dict[tuple[str, 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.url is None or source.resolved_sha is None: + if (source.url is None or source.mode is None + or source.resolved_sha is None): continue # Newest wins; "HEAD" normalised the way the spec does it. - key = (source.url, source.ref or "HEAD") + key = (source.url, source.ref or "HEAD", source.mode) baselines.setdefault(key, source.resolved_sha) return baselines 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 9434ce1ba0..5cd5c22b03 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 @@ -879,6 +879,7 @@ async def test_the_sessions_resolutions_ride_into_the_report(): name="charts", url="https://git.corp/charts.git", ref="main", + mode="strict", resolved_sha="f" * 40, auth="ci-token", ) @@ -898,6 +899,7 @@ async def test_the_sessions_resolutions_ride_into_the_report(): "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 bf7d0ef718..a0c1a75b64 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 @@ -691,6 +691,7 @@ def test_a_strict_baseline_is_read_back_from_report_history(world, monkeypatch): name="charts", url="https://git.corp/charts.git", ref="main", + mode="strict", resolved_sha="f" * 40, auth="ci-token", ) @@ -703,33 +704,38 @@ def test_a_strict_baseline_is_read_back_from_report_history(world, monkeypatch): ) assert service._last_resolutions( entity_id=_ENTITY, bot_id=_BOT - ) == {("https://git.corp/charts.git", "main"): "f" * 40} + ) == {("https://git.corp/charts.git", "main", "strict"): "f" * 40} -def test_baselines_are_read_by_url_and_ref_and_skip_rows_without_a_url( +def test_baselines_are_read_by_url_ref_and_mode_and_skip_incomplete_rows( world, monkeypatch ): - """The key is the repository and the ref, which has two consequences here. + """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 are two rows - — and they always carry the same sha, because one apply resolves a - ``(url, ref)`` once. And a row with no ``url`` is skipped outright: that - is a report written before the url was recorded, and there is no honest - way to guess which repository its name meant. + 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", - resolved_sha="f" * 40), - # A second declaration of the same repository at the same ref. + mode="non_strict", resolved_sha="f" * 40), + # A second declaration of the same repository, ref and mode. SourceResolution(name="dashboards", url=url, ref="main", - resolved_sha="f" * 40), + mode="non_strict", resolved_sha="f" * 40), # Same repository, another ref: its own key, its own answer. SourceResolution(name="pinned", url=url, ref="v1", - resolved_sha="c" * 40), - # Pre-existing history: no url, so no baseline. + 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( @@ -738,8 +744,9 @@ def test_baselines_are_read_by_url_and_ref_and_skip_rows_without_a_url( lambda *, env, entity_id, bot_id, limit: [_row(_report_with_sources(rows))], ) assert service._last_resolutions(entity_id=_ENTITY, bot_id=_BOT) == { - (url, "main"): "f" * 40, - (url, "v1"): "c" * 40, + (url, "main", "non_strict"): "f" * 40, + (url, "v1", "strict"): "c" * 40, + (url, "main", "strict"): "e" * 40, } @@ -754,6 +761,7 @@ def test_a_failed_apply_does_not_wipe_a_strict_baseline(world, monkeypatch): name="charts", url="https://git.corp/charts.git", ref="main", + mode="strict", resolved_sha="e" * 40, ) empty_failed = _report_with_sources( @@ -769,7 +777,7 @@ def test_a_failed_apply_does_not_wipe_a_strict_baseline(world, monkeypatch): ) assert service._last_resolutions( entity_id=_ENTITY, bot_id=_BOT - ) == {("https://git.corp/charts.git", "main"): "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. @@ -777,6 +785,7 @@ def test_a_failed_apply_does_not_wipe_a_strict_baseline(world, monkeypatch): name="charts", url="https://git.corp/charts.git", ref="main", + mode="strict", resolved_sha="b" * 40, ) monkeypatch.setattr( @@ -788,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 - ) == {("https://git.corp/charts.git", "main"): "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 6776f11848..81da7e1186 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={("https://git.corp/id.git", "main"): "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 83f9965d97..47b3dc5522 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={("https://git.corp/skills.git", "main"): "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={("https://git.corp/skills.git", "main"): "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 ed3a2fd548..c307b389bf 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 @@ -630,7 +630,9 @@ def test_strict_refuses_when_the_ref_moved(rig): # 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, "main"): "b" * 40}) + source_session=_session( + git, baselines={(GIT_URL, "main", "strict"): "b" * 40} + ) ) with pytest.raises(EntryFetchError, match="moved"): pipeline.resolve( @@ -649,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, "main"): "b" * 40}) + source_session=_session( + git, baselines={(GIT_URL, "main", "non_strict"): "b" * 40} + ) ) decl = pipeline.resolve( ctx, @@ -702,7 +706,9 @@ def test_an_inline_strict_refusal_names_the_ref_it_refused(rig): _, _, pipeline = rig git = _ScriptedGit() ctx = make_context( - source_session=_session(git, baselines={(GIT_URL, "main"): "b" * 40}) + source_session=_session( + git, baselines={(GIT_URL, "main", "strict"): "b" * 40} + ) ) with pytest.raises(EntryFetchError, match=f"{GIT_URL}@main"): pipeline.resolve( @@ -714,10 +720,10 @@ def test_an_inline_strict_refusal_names_the_ref_it_refused(rig): def test_two_names_over_one_repository_are_two_rows_and_one_baseline(rig): - """Report rows are per declaration; baselines are per ``(url, ref)``. A - document that names one repository twice 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.""" + """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={ @@ -732,10 +738,83 @@ def test_two_names_over_one_repository_are_two_rows_and_one_baseline(rig): 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) for r in records} == {(GIT_URL, "main")} + 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_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 @@ -746,7 +825,9 @@ def test_strict_passes_when_the_document_re_pins_the_ref(rig): _, _, pipeline = rig git = _ScriptedGit() ctx = make_context( - source_session=_session(git, baselines={(GIT_URL, "v1"): "b" * 40}) + source_session=_session( + git, baselines={(GIT_URL, "v1", "strict"): "b" * 40} + ) ) decl = pipeline.resolve( ctx, @@ -773,7 +854,9 @@ def test_strict_passes_when_the_document_re_points_the_url(rig): git = _ScriptedGit() other = "https://git.corp/other.git" ctx = make_context( - source_session=_session(git, baselines={(GIT_URL, "main"): "b" * 40}) + source_session=_session( + git, baselines={(GIT_URL, "main", "strict"): "b" * 40} + ) ) decl = pipeline.resolve( ctx, @@ -795,7 +878,9 @@ def test_a_sha_shaped_ref_trips_neither_branch(rig): _, _, pipeline = rig git = _ScriptedGit() ctx = make_context( - source_session=_session(git, baselines={(GIT_URL, _FAKE_SHA): _FAKE_SHA}) + source_session=_session( + git, baselines={(GIT_URL, _FAKE_SHA, "strict"): _FAKE_SHA} + ) ) decl = pipeline.resolve( ctx, @@ -815,7 +900,9 @@ def test_non_strict_does_not_call_a_re_pin_a_move(rig): _, _, pipeline = rig git = _ScriptedGit() ctx = make_context( - source_session=_session(git, baselines={(GIT_URL, "v1"): "b" * 40}) + source_session=_session( + git, baselines={(GIT_URL, "v1", "non_strict"): "b" * 40} + ) ) decl = pipeline.resolve( ctx, @@ -851,7 +938,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={(GIT_URL, "main"): old_sha}, + baselines={(GIT_URL, "main", "non_strict"): old_sha}, )) result = pipeline.resolve( ctx, @@ -885,7 +972,7 @@ def test_git_keep_last_has_no_receipt_to_reuse_after_a_re_pin(rig): sources={"app": {"protocol": "git", "url": GIT_URL, "ref": "v2", "subpath": "pkg"}}, # Recorded against the ref the document used to name. - baselines={(GIT_URL, "v1"): old_sha}, + baselines={(GIT_URL, "v1", "non_strict"): old_sha}, )) with pytest.raises(EntryFetchError, match="git fetch failed"): pipeline.resolve( 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 5cb21d47ca..70d4ae0d1c 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 @@ -82,6 +82,7 @@ def test_adoption_records_the_resolution_once_per_display(): name="src", url="https://git.corp/r.git", ref="main", + mode="non_strict", resolved_sha="a" * 40, auth="ci", ), @@ -127,17 +128,23 @@ def test_close_is_idempotent_and_deregisters(monkeypatch): assert removed == [Path("/tmp/x")] -def test_baseline_reads_the_map_by_url_and_ref(): - """The key is the repository and the ref, never the document's name for - them: strict mode asks "did this pair resolve differently", and a rename - is not that question.""" +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={("https://git.corp/r.git", "main"): "b" * 40}, + baselines={("https://git.corp/r.git", "main", "strict"): "b" * 40}, git=FakeGitClient(), ) - assert session.baseline("https://git.corp/r.git", "main") == "b" * 40 + assert ( + session.baseline("https://git.corp/r.git", "main", "strict") == "b" * 40 + ) # Same repository, another ref — a re-pin, so no opinion. - assert session.baseline("https://git.corp/r.git", "v2") is None + 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") is None + 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 From 54ff99bc26bd1adec27222ce26aab86bccae5270 Mon Sep 17 00:00:00 2001 From: totalfrank Date: Fri, 11 Sep 2026 15:43:43 +0000 Subject: [PATCH 7/8] fix(apply): de-duplicate source resolutions per (display, mode) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The same alias hazard as the last commit, surviving on the road where the display is not unique. Codex caught it on the new head; a probe confirmed it. `adopt` de-duplicated on the display alone. A named source's display is its `from` name, which maps to exactly one `(url, ref, mode)`, so it was fine. An inline source's display is `url@ref` — mode-blind — so two inline declarations of one repository at one ref but two modes shared it. 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. That is worse than the bug it followed: not "refuse once then deliver", but a pin disarmed from the first apply onward. The rule that closes it, and the one to check against next time a key or an identity moves: **the recording identity must be at least as fine as the key the recording feeds.** The baseline key is `(url, ref, mode)`, so the recording identity is now `(display, mode)`. Both declarations get a report row. They share a name, because `url@ref` is what an inline source is called, and are told apart by the mode each was resolved under. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01U8YX8W4uDYiHjcX9iRBLje --- .../bot-config-manifest/user-manual.zh-CN.md | 2 + .../apply/source_session.py | 38 ++++++++++----- .../apply/test_source_resolver.py | 48 +++++++++++++++++++ 3 files changed, 77 insertions(+), 11 deletions(-) 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 c0fe7e1843..1ae038b82c 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 @@ -1774,6 +1774,8 @@ B.2.2 / B.2.3 / B.2.4 与 `GET …/with-manifest/status` 的 `apply` 字段都 **一条声明一行**:两个 `from` 名指向同一个 `(url, ref)` 就是两行(各自带着作者 写下的那个名字),`resolved_sha` 相同;同一个仓库内联声明两个 `ref` 也是两行。 +同一个 `(url, ref)` 内联声明两次、只有 `mode` 不同,同样是两行——内联源的 `name` +都是 `url@ref`,这两行靠 `mode` 区分。 `categories[]`: 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 2a834ea308..f865b3eb64 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 @@ -151,13 +151,26 @@ 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 display, 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:: + #: The ``(display, mode)`` pairs 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/solo.git@main"} - _recorded: set[str] = field(default_factory=set) + #: {("content", "strict"), + #: ("https://code.example.com/solo.git@main", "non_strict")} + #: + #: ``mode`` is in here for the same reason it is in the baseline key, and + #: the rule is worth stating on its own: **the recording identity must be + #: at least as fine as the key the recording feeds.** A named source's + #: display is its ``from`` name, which maps to exactly one + #: ``(url, ref, mode)``, so it is already fine enough. An inline source's + #: display is ``url@ref``, which is not: two inline declarations of one + #: repository at one ref but two modes share it. De-duplicated on the + #: display alone, whichever resolved first would take the slot and the + #: other would record nothing — leaving, if the loser was the ``strict`` + #: one, a pin that never establishes a baseline and therefore never + #: refuses anything. + _recorded: set[tuple[str, str]] = field(default_factory=set) def checkout( self, @@ -230,13 +243,16 @@ 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; a display is a name or ``url@ref``, - so every entry that names one source stands behind one resolution, and - two names over one repository record one row each. + re-pinned. Idempotent per ``(display, mode)``; a display is a name or + ``url@ref``, so every entry that names one source stands behind one + resolution, and two names over one repository record one row each. The + mode is part of that identity rather than a detail of the row: see + ``_recorded``. """ - if display in self._recorded: + key = (display, spec.mode) + if key in self._recorded: return - self._recorded.add(display) + self._recorded.add(key) self._resolutions.append( SourceResolution( name=display, 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 c307b389bf..0d2a4a38fa 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 @@ -744,6 +744,54 @@ def test_two_names_over_one_repository_are_two_rows_and_one_baseline(rig): assert len(git.specs) == 1, "one checkout per (url, ref) per apply" +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. From 25bcb7dc60e5f4932aaeaeaa943db22c412659cf Mon Sep 17 00:00:00 2001 From: totalfrank Date: Fri, 11 Sep 2026 15:50:53 +0000 Subject: [PATCH 8/8] fix(apply): de-duplicate resolutions by the full source identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third finding of one root cause, and the last shape that can have this hole: the recording identity now contains the baseline key outright instead of something believed to imply it. `adopt` keyed on `(display, mode)`. An inline display is `url@ref`, and that join 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"` share the display `.../a@b@c` while having different baseline keys. The second declaration was dropped, and a dropped `strict` declaration never establishes a baseline, so it accepts every subsequent move. The key is now `(display, url, ref, mode)`. `display` stays because it is not implied by the rest either: two `from` names over one `(url, ref, mode)` are two declarations and must stay two rows. Both narrower shapes and why each failed are recorded on `_recorded`, so the next person to touch this has the counterexamples rather than the conclusion. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01U8YX8W4uDYiHjcX9iRBLje --- .../apply/source_session.py | 60 +++++++++++-------- .../apply/test_source_resolver.py | 51 ++++++++++++++++ 2 files changed, 87 insertions(+), 24 deletions(-) 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 f865b3eb64..293de9487c 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 @@ -151,26 +151,38 @@ 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, mode)`` pairs 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:: + #: 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", "strict"), - #: ("https://code.example.com/solo.git@main", "non_strict")} + #: {("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")} #: - #: ``mode`` is in here for the same reason it is in the baseline key, and - #: the rule is worth stating on its own: **the recording identity must be - #: at least as fine as the key the recording feeds.** A named source's - #: display is its ``from`` name, which maps to exactly one - #: ``(url, ref, mode)``, so it is already fine enough. An inline source's - #: display is ``url@ref``, which is not: two inline declarations of one - #: repository at one ref but two modes share it. De-duplicated on the - #: display alone, whichever resolved first would take the slot and the - #: other would record nothing — leaving, if the loser was the ``strict`` - #: one, a pin that never establishes a baseline and therefore never - #: refuses anything. - _recorded: set[tuple[str, 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, @@ -243,13 +255,13 @@ 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, mode)``; a display is a name or - ``url@ref``, so every entry that names one source stands behind one - resolution, and two names over one repository record one row each. The - mode is part of that identity rather than a detail of the row: see - ``_recorded``. + 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. """ - key = (display, spec.mode) + key = (display, spec.url, spec.ref, spec.mode) if key in self._recorded: return self._recorded.add(key) 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 0d2a4a38fa..76510b6ac9 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 @@ -744,6 +744,57 @@ def test_two_names_over_one_repository_are_two_rows_and_one_baseline(rig): 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.