From f470d6f9c1e084aeda1d26cfe252e8295b5f833a Mon Sep 17 00:00:00 2001 From: Polichinl Date: Tue, 4 Aug 2026 05:33:52 +0200 Subject: [PATCH] =?UTF-8?q?docs(adr):=20ADR-015=20=E2=80=94=20why=20this?= =?UTF-8?q?=20repo=20imports=20another=20project's=20Appwrite=20client?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The þing-02 ratification attached a condition to the #146 deferral: record it as an ADR in this repo. That was never done. The reasoning lived only in a GitHub issue and in a deliberation folder outside the repository, which is what the condition existed to prevent -- an issue is a work item, not a place someone checks before deciding whether a rule applies to them. What the ADR records - Why the import stays: a hand-written client here would be the platform's THIRD copy, and the defect that prompted the assembly was common to the two existing copies, not a divergence between them. Upstream declines to export a client surface, deliberately, because exporting the current one would bless a shape its own maintainers have recorded as needing decomposition. There is nothing to unwind to. - What bounds it: an importer allowlist pinned by test, the _ContractStorePort DIP port (four methods, so the wire never sees Appwrite types), and the framework-contract test added with the 3.0.0 bump. Containment, not removal -- C-40 stays open on the residual. - The trigger in plain words: demand (a second incident rooted in duplicated client code) and supply (upstream carving out the auth/config seam). Either means revisit; both mean do it. Explicitly not "this seat's convenience". Two things found while writing it, neither previously recorded here - The supply half has PARTIALLY moved. The verdict was written against a single 3,064-line module; 3.0.0 relocated provisioning and transport out of it and added an audit package, leaving 2,841 lines. Still no exported client, so the trigger has not fired -- but "no movement" would be the wrong thing to believe. - The sibling obligation from the same verdict is DISCHARGED. It required this repo's legacy delivery path to be guarded or retired before 2026-11-30. It was retired in #149; C-63 is resolved; the deadline is moot here. Nobody had recorded that it was met. Also: `C-221` in the verdict is pipeline-core's REGISTER entry, not issue #221 (which is unrelated and closed). Noted in the appendix because this repo namespaces cross-repo identifiers for exactly that reason and the verdict's shorthand does not. Scope note: the deferral was written when one manager held the import; crafd added a second. Two adapters with identical contents now. 372 passed / 40 xfailed / 0 failed. ruff clean. Co-Authored-By: Claude Opus 5 (1M context) --- .../015_the_pipeline_core_appwrite_import.md | 172 ++++++++++++++++++ docs/ADRs/README.md | 9 + reports/technical_risk_register.md | 2 + 3 files changed, 183 insertions(+) create mode 100644 docs/ADRs/015_the_pipeline_core_appwrite_import.md diff --git a/docs/ADRs/015_the_pipeline_core_appwrite_import.md b/docs/ADRs/015_the_pipeline_core_appwrite_import.md new file mode 100644 index 0000000..27728c5 --- /dev/null +++ b/docs/ADRs/015_the_pipeline_core_appwrite_import.md @@ -0,0 +1,172 @@ +# ADR-015: Why this repository imports another project's Appwrite client + +**Status:** Accepted +**Date:** 2026-08-04 +**Deciders:** Project maintainers (PRIO MD&D Team) +**Arises from:** issue [#146](https://github.com/views-platform/views-postprocessing/issues/146), +recorded at the request of the þing-02 ratification +**Related:** [ADR-002](002_topology_and_dependency_rules.md) (dependency direction), +[ADR-009](009_boundary_contracts_and_configuration_validation.md) (boundary contracts), +[ADR-014](014_claims_and_the_guards_that_carry_them.md) §4 (a deferral needs a trigger), +register **C-40** + +--- + +## Context + +Each partner manager in this repository begins with two imports it would rather not have: + +```python +from views_pipeline_core.modules.appwrite.file import AppwriteConfig +from views_pipeline_core.modules.datastore import DatastoreModule +``` + +They reach into another project's Appwrite client — its auth handling, its provisioning +behaviour, its error semantics — and run it under this repository's identity and this +repository's API key. When that client does something surprising, it surprises FAO's +delivery, and the fix is in a repository this team does not own. + +This is knowingly kept. It has been raised repeatedly: register **C-40** carries it as a +Tier-2 concern, a cross-repository assembly examined it in July 2026 and told the platform +not to export the surface, and issue #146 tracks the eventual removal. None of that has +changed the code, and it should not have — but the *reason* has been recorded only in a +GitHub issue and in a deliberation folder outside this repository. A closed issue is not +where architectural reasoning survives. That is what this ADR is for, and writing it was +the condition attached when the deferral was ratified. + +**The word "deferred" is doing real work here and is easy to misread.** It does not mean +*not done yet*. It means: examined, decided, and held — with a stated condition under which +the decision is revisited. The distinction matters because the two read identically in an +issue tracker and completely differently to someone deciding whether they may change it. + +--- + +## Decision + +### §1 The import stays, because there is nothing to unwind to + +The obvious alternative is to write our own thin Appwrite client and drop the dependency. +We do not, for a reason that is about the platform rather than about this repository: +**a third copy of that client is the disease, not the cure.** + +Two hand-written copies already exist across the platform, and the defect that prompted the +cross-repository examination was common to both — not a divergence between them, but the +same mistake made twice. Adding a third copy here would multiply the surface the platform +is trying to shrink, and would do it in the repository that talks to the partner. + +The upstream project declines to export a supported client surface, and that refusal is +deliberate: exporting the current one would bless a shape its own maintainers have recorded +as needing decomposition. So the honest position is that the seam to unwind *to* has not +been built yet, by anyone, and building it unilaterally here would be worse than waiting. + +### §2 What bounds the blast radius while it stays + +A dependency kept on purpose still has to be contained, and this one is: + +- **Only the partner managers import it.** `tests/test_doc_accuracy.py` pins the importer + set to an explicit allowlist, so a third importer fails CI. Every other module — + the whole of `contract/` and `delivery/` — is free of it, proven in a fresh interpreter + by `tests/test_clone_readiness.py`. +- **The store is behind a port.** `_ContractStorePort` wraps the client in four methods, so + the wire modules never see Appwrite types. That is the dependency-inversion half of + C-40's prescribed mitigation, and it is the half that landed. +- **The framework contract is asserted.** `tests/test_framework_contract.py` checks that the + hooks we override still match upstream, that the classes remain instantiable, and that + every inherited attribute resolves — so an upstream change is found here rather than on a + delivery run. + +None of that removes the coupling. It makes the coupling *one file per partner wide* and +*visible when it moves*, which is the most that can be true while §1 holds. + +### §3 The condition under which this is revisited + +Two independent things can move, and they were ratified as a pair: + +- **Demand** — a second incident whose root cause is auth or provisioning handling in + duplicated client code, whether from divergence between copies or from a defect common to + them. +- **Supply** — the upstream project carving the auth and configuration seam out of its + Appwrite module, so that something exists to depend on. + +**Either one alone means revisit. Both together mean do it.** The deferral is tied to that, +explicitly *not* to this team's convenience — which is the wording the assembly chose, and +it is worth keeping because "when we get round to it" is how a deferral becomes a decision +nobody made. + +### §4 Where supply actually stands, which is not where the record says + +The verdict was written when the upstream Appwrite module was a single 3,064-line file. As +of the version this repository now pins, that has **partially** moved: provisioning has been +relocated to its own module, transport (including the request timeout this repository +depended on) to another, and an audit package now exists alongside. The main file is 2,841 +lines. + +**So the supply half has started and has not fired.** What we import is still there, and +there is still no exported client to depend on instead. But "no movement" would be the wrong +thing to believe, and a reader checking this in six months should check the module list +rather than trust this paragraph. + +### §5 Scope: this is two files now, not one + +The deferral was written when one manager held the import. The CRAF'd partner package added +a second, and it is a near-verbatim copy of the first. So the adapter to unwind is two +adapters with identical contents, and any fix must be applied twice or neither. + +That is not an argument to unwind sooner — it is the same bounded surface duplicated, and +register **C-33** already carries the duplication with its own trigger. It is recorded here +so that whoever eventually does the work is not surprised by the second file. + +--- + +## Consequences + +**What this costs.** Upstream changes land in FAO's delivery without this repository owning +the fix; the managers cannot be instantiated without the framework; and the platform's most +central package remains a hard dependency of the thing that talks to the partner. + +**What it buys.** No third client copy. The whole of `contract/` and `delivery/` stays +framework-free and reusable, which is what let a second partner be added as a package rather +than a fork. + +**Where this will go wrong first.** Someone reading issue #146 will see "deferred", read it +as "not done yet", and either do it — writing the third copy §1 exists to prevent — or treat +the import as unexamined and add a third importer. The allowlist test catches the second. +Nothing catches the first except this document. + +--- + +## Alternatives considered + +**Write our own thin client now.** Rejected in §1: it is the third copy, and it puts the +duplication in the repository with the partner relationship. + +**Wait silently and leave the reasoning in the issue.** This is what was happening, and it is +what the ratification objected to. An issue is a work item; it is not read by someone +deciding whether a rule applies to them. + +**Bound it with a test and consider it closed.** The allowlist and the port genuinely reduce +the risk, and it is tempting to call that the end. Rejected because containment is not +removal — C-40 stays open on exactly the residual this ADR describes, and closing it would +mean the next reader finds a bounded coupling with no record of why it is tolerated. + +--- + +## Appendix — the record this replaces + +The reasoning above was previously recorded only in issue +[#146](https://github.com/views-platform/views-postprocessing/issues/146) and in the +platform's deliberation folder (`views_platform/þingit/`), which is outside this repository +and not on any contributor's path. + +Two related facts, verified 2026-08-04 and recorded here because they are otherwise only in +that folder: + +- **The sibling obligation from the same verdict is discharged.** The assembly also required + this repository's legacy delivery path to be guarded or retired before 2026-11-30. It was + retired in #149 — the manager carries no such path, and register C-63 is resolved. The + deadline is moot here, and nobody had recorded that it was met. +- **`C-221` is the upstream project's register entry, not an issue number.** Issue #221 in + that repository is unrelated and closed. The entry is Tier 4 and describes the + decomposition need in §3's supply component. Cross-repository identifiers in this + repository are namespaced for exactly this reason + (`tests/test_register_integrity.py` enforces it), and the verdict's own shorthand is not. diff --git a/docs/ADRs/README.md b/docs/ADRs/README.md index 9e8ae78..48d7ab1 100644 --- a/docs/ADRs/README.md +++ b/docs/ADRs/README.md @@ -79,6 +79,15 @@ These ADRs form the architectural constitution of the repository. record in the same change. Arises from epic #181, which found seven places where this repo said one thing and did another. +- **ADR-015** — Why This Repository Imports Another Project's Appwrite Client + The partner managers import `views_pipeline_core.modules.{appwrite,datastore}` and run + another project's client under this repo's identity. Kept knowingly: a hand-written + client here would be the platform's *third* copy, and the upstream seam to depend on + instead does not exist yet. Records what bounds it (an importer allowlist, the + `_ContractStorePort` DIP port, a framework-contract test), and the two-part condition — + demand and supply — under which it is revisited. Arises from #146 and the þing-02 + ratification, which asked that the reasoning live here rather than in an issue. + ADRs numbered 010 and above define: - Domain-specific decisions diff --git a/reports/technical_risk_register.md b/reports/technical_risk_register.md index 97c1ca6..4043806 100644 --- a/reports/technical_risk_register.md +++ b/reports/technical_risk_register.md @@ -379,6 +379,8 @@ See also C-24 (schema contract per store), C-77 (the fourth home for partner ide **What actually changed, and what did not.** The blast radius is no longer *one file wide* — it is **one file, twice**, which is a different and slightly worse property: an upstream change now has two identical landing sites and no mechanism guarantees they are patched together (C-33). What did **not** change is the more important half: the count is still **bounded and pinned**. `test_views_pipeline_core_is_confined_to_the_partner_managers` (renamed in #211 — it had asserted *two* under a name that said *one*) was widened to an explicit allowlist, not deleted, so a *third* importer still fails CI. Every other module in the repository remains pipeline-core-free, including the whole of `contract/` and `delivery/`, and `tests/test_clone_readiness.py` still proves the machinery imports in a subprocess without it. +**Recorded as ADR-015 (2026-08-04).** The reasoning for keeping this import — and the two-part condition under which it is revisited — now lives in `docs/ADRs/015_the_pipeline_core_appwrite_import.md` rather than only in issue #146 and the platform's deliberation folder. That was the condition attached when the deferral was ratified, and it had not been met. The ADR also records that the **supply** half of the trigger has *partially* moved: pipeline-core 3.0.0 relocated provisioning and transport out of the Appwrite module (3,064 → 2,841 lines), but exports no client surface, so there is still nothing to unwind to. + **On þing-02 S24(5).** `docs/CLONING.md` cited that verdict as forbidding these imports outright. Reading it directly (`þingit/02_credential_identity_key_ownership/sáttmál.md:240-242` — precondition (5) itself; the section opens at `:232` under the heading *"§5 — The clone (`un-crafdapi`)"* — and `orð_dómr.md:418-441`), it binds *"the clone"* — `un-crafdapi` and `views-productionapi`, repositories **git-cloned from views-faoapi** — and does not reach an in-repo partner package of the producer. CLONING.md over-claimed; PR #211 corrects the citation rather than weakening the rule. This entry's own scope is unaffected: the coupling is a design concern here regardless of what the verdict binds, and issue **#146**'s deferred unwind now covers two files instead of one. Tier held at 2. The residual scope — the double inheritance and the framework-bound instantiation — is unchanged, and is still gated on views-pipeline-core 3.0.0 (C-44/C-62).