From b5c73b85e1bae7aec4266d2df09e8034095b3c1b Mon Sep 17 00:00:00 2001 From: Joe Beda Date: Sun, 26 Jul 2026 13:05:24 -0700 Subject: [PATCH 1/3] docs(adr): cross-model references and the network boundary (ADR-0010, ADR-0011) Design output from a grilling session on #25. Nothing is implemented; both ADRs record intent so the format work has a settled shape to build against. ADR-0010 decides that a model declares a scope slug and references another model's items as slug.Name, resolving against a whole-file copy committed to this repo. An explicit imports: path list locates copies, each file's own scope: owns its slug, and resolution does not recurse. Provenance lives in a header comment rather than in the schema, because an origin: key would put a vendoring concept into the format that every single-model user would meet in the schema reference, and rather than a lock file, because models commonly sit as peer files in docs/ where a root lock would be several directories away. The surface digest covers structure and normative content but not documentation prose. Qualified entity references stay deferred; they already fail schema validation, and lint gains a friendly error ahead of it. ADR-0011 replaces the blanket no-network rule in go-style.md with the property it was standing in for: lint and render never perform network I/O, and the commands that do are named, opt-in, and delegate transport to git/gh rather than implementing it. ADR-0011 lands with TestADR_0011_OfflinePackages, which walks transitive imports of the offline packages and fails on net, net/http or os/exec. os/exec is banned there deliberately: without it the rule could be evaded by shelling out to curl. Verified to fail when it should, by temporarily banning a package that is genuinely in the graph. ADR-0010 has no test yet because none of it is built; the ADR names what will pin it. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Joe Beda --- internal/model/network_test.go | 54 +++++++ ...010-cross-model-references-by-vendoring.md | 135 ++++++++++++++++++ project-docs/adr/0011-network-boundary.md | 74 ++++++++++ 3 files changed, 263 insertions(+) create mode 100644 internal/model/network_test.go create mode 100644 project-docs/adr/0010-cross-model-references-by-vendoring.md create mode 100644 project-docs/adr/0011-network-boundary.md diff --git a/internal/model/network_test.go b/internal/model/network_test.go new file mode 100644 index 0000000..104a111 --- /dev/null +++ b/internal/model/network_test.go @@ -0,0 +1,54 @@ +package model + +import ( + "os/exec" + "slices" + "strings" + "testing" +) + +// TestADR_0011_OfflinePackages guards ADR-0011: lint, render, schema, and model +// never perform network I/O, no matter what flags are passed. It walks the +// transitive import graph rather than direct imports, because the guarantee is +// about what the code can reach, not what it names. +// +// os/exec is banned alongside the net packages on purpose: without it the rule +// could be evaded by shelling out to curl. Commands that do use the network +// live in cmd/modelith and delegate transport to git/gh. +// +// It lives here rather than in each package because the rule is one decision +// about a set of packages, and splitting it would let a new offline package be +// added without a guard. Adding one means adding it to offlinePackages. +func TestADR_0011_OfflinePackages(t *testing.T) { + t.Parallel() + + offlinePackages := []string{ + "github.com/stacklok/modelith/internal/lint", + "github.com/stacklok/modelith/internal/render/markdown", + "github.com/stacklok/modelith/internal/render/mermaid", + "github.com/stacklok/modelith/internal/schema", + "github.com/stacklok/modelith/internal/model", + } + + // net/url and net/netip are parsers that perform no I/O; the JSON Schema + // library pulls them in for format validation. Everything else under net + // dials something. + banned := []string{"net", "net/http", "net/rpc", "net/smtp", "os/exec"} + + for _, pkg := range offlinePackages { + // nolint:gosec // G204 flags a variable argument. pkg comes from the + // literal offlinePackages slice above and never from input, so the + // injection the rule guards against cannot occur. + out, err := exec.Command("go", "list", "-deps", pkg).Output() + if err != nil { + t.Fatalf("go list -deps %s: %v", pkg, err) + } + for _, dep := range strings.Split(strings.TrimSpace(string(out)), "\n") { + if slices.Contains(banned, dep) { + t.Errorf("%s transitively imports %q\n"+ + "ADR-0011: lint, render, schema and model never perform network I/O.\n"+ + "See project-docs/adr/0011-network-boundary.md.", pkg, dep) + } + } + } +} diff --git a/project-docs/adr/0010-cross-model-references-by-vendoring.md b/project-docs/adr/0010-cross-model-references-by-vendoring.md new file mode 100644 index 0000000..4c2d280 --- /dev/null +++ b/project-docs/adr/0010-cross-model-references-by-vendoring.md @@ -0,0 +1,135 @@ +# Cross-model references resolve against vendored copies + +A model may declare a `scope:` slug and reference another model's items as +`slug.Name`. References resolve against a *copy of the other model committed to +this repo*, located by an explicit `imports:` path list, with provenance +recorded in a header comment. Fetching is an explicit, opt-in operation +delegated to `git`/`gh`; lint and render never touch the network. + +## Context + +A system outgrows one model, and the parts end up in different repos with +different lifecycles and owners. Today nothing lets one model refer to an item +defined in another. The workaround — define the same item in both models and +rely on the names matching — leaves the claim "these are the same" in prose, +where nothing resolves it and nothing checks it. That is issue #25. + +The design pressure came from a real case: two models in separate repos sharing +a small enum, each defining it locally and each asserting the shared-ness in a +`description:`. The names agreed; the prose had already drifted. Nothing would +have caught a fourth value appearing on one side. + +Two shapes were considered for that case. An *equivalence* form, where both +models keep a local definition and assert they match, was rejected: it requires +the format to carry a second mechanism, and it makes drift the normal state +rather than an error. The *reference* form — one canonical definition, others +point at it — is the whole design. Which model hosts the canonical copy, and +whether shared vocabulary deserves a third model of its own, is a modelling +choice for the author. The tool takes no position; the authoring skill offers +guidance. + +## Decision + +**Identity.** A model may declare a top-level `scope:` slug (kebab-case). +Uniqueness is conventional and scoped to the set of models that reference each +other; there is no registry, and none is planned. The slug is short because it +is written inline at every reference site, which is also why there is no +prefix-alias layer. `scope:` is optional — a model that nobody imports never +needs one — but it is required to *be* imported. + +**Reference.** An item in another model is written `slug.Name`. In this change +only attribute `type` may be qualified; see "deferred" below. + +**Resolution.** A model lists `imports:` as paths relative to itself. Each +listed file declares its own `scope:`, which is the single source of truth for +the slug — nothing is repeated, so nothing can disagree. There is no directory +convention and no scanning. Resolution does not recurse: a vendored model's own +`imports:` are ignored, because those paths describe its repo, not this one. + +**Vendoring.** An imported model is a whole copy committed to this repo, +unmodified except for a provenance header. Whole-file rather than an extracted +subset, so that referencing something new from an already-imported model is a +local edit rather than another fetch. + +**Provenance.** Origin, imported-at, and a surface digest live in a +`# modelith:` header comment in the vendored file. Not in the schema: an +`origin:` key would put a vendoring concept into the domain-model format that +every single-model user would then meet in the schema reference. Not a lock +file either: models commonly sit as a couple of peer files in a `docs/` +directory, and a lock at the repo root would be several directories from +anything it describes. The format already carries machine-read data in a header +comment (`# yaml-language-server: $schema=`), so this is an established shape +rather than a new one. + +**Digest.** The surface digest covers structure and normative content — names, +ids, types, cardinalities, references, enum value names, and invariant +*statements*. It excludes documentation: `description`, `definition`, `note`, +`derivation`, scenario steps, `title`, comments, and key order. An invariant's +statement is the rule itself, so a reworded one is a real change to what a +consumer relies on; a `definition` is documentation of a name, and polishing it +should not page anybody. The digest answers "did anything change"; when an +explicit freshness check has both files in hand, it diffs them to report what. + +**Checks.** `lint` re-verifies vendored files against their own headers by +default: offline, free, and it catches the hand-edit that a `DO NOT EDIT` +comment cannot prevent. Checking whether *origin* has moved is opt-in behind a +flag, so a repo that wants aggressive skew detection enables it in CI and +everyone else never makes a network call. This mirrors `--completeness error`: +one command, posture chosen per repo. The digest is a drift detector, not a +security boundary — anyone editing a vendored file can recompute the header, +and that is fine. + +**Fetching.** modelith does not implement network transport; it shells out +(ADR-0011). `gh` already solves authentication for private and internal +repositories, which the motivating models require. Commands are executed as an +argv array, never through a shell, and never sourced from a model file: a +vendored artifact must not be able to influence what runs. + +**Render.** Rendered Markdown documents the dependency — an imports section +plus qualified type names, with relative deep links into the imported model's +rendered `.md` — but never expands imported definitions and never opens another +file. Output stays a pure function of the single model being rendered, so the +golden-fixture invariant holds and a vendored file changing cannot silently +rewrite an unrelated `.md`. Links to un-rendered models will dangle; that is +accepted. + +**Commands.** Everything that manages an imported model lives under +`modelith deps`: `import` adds a model and stamps its header, `update` +refreshes one from origin, and `check` reports whether origin has moved. Only +`update` and `check` use the network, which is why they sit in a group of their +own rather than as flags on `lint`. Origins are assumed to be GitHub; other +hosts are a bridge to cross when someone needs one, and the origin field is a +string, so widening it later costs nothing. + +The network boundary this rests on — that `lint` and `render` never reach the +network, and that commands which do delegate transport rather than implementing +it — is its own decision, recorded as ADR-0011. + +## Deferred + +Qualified references in `relationship.entity` and `subtypeOf` are not +supported. They already fail schema validation, since both fields carry +`pattern: ^[A-Z][A-Za-z0-9]+$`; lint gains a friendly error ahead of the raw +schema violation. The resolution mechanism is shared and would mostly work, but +two questions have no live instance driving them: whether the Mermaid ER draws +a foreign entity, and whether reciprocity and pairing checks apply across a +boundary. Relaxing the pattern later is backward compatible. Following +ADR-0007, the bar to lift this is a concrete modelling failure, not +expressiveness for its own sake. + +## Consequences + +- The format gains `scope:` and `imports:`; the schema, the structs, and the + schema reference change together, guarded by `TestSchemaStructSync`. +- Attribute `type` stops being an unconstrained string. A qualified type whose + scope is not imported becomes an error; today such a value is silently + treated as a primitive, because `lint.go`'s check only fires on PascalCase. +- `lint` and `render` gain the ability to read files other than their argument + — `lint` to resolve imports, `render` only to the extent of emitting links. +- New command surface for import, update, and freshness checking. +- A vendored model is a whole extra model in the repo. Completeness and + "unused" diagnostics must not fire on it, and it must not be mistaken for a + model the repo owns. +- Nothing pins this ADR by test yet, because none of it is built. When it + lands, `TestADR_0010_*` covers the digest's field selection and the + no-transitive-resolution rule; both are decisions a test can hold. diff --git a/project-docs/adr/0011-network-boundary.md b/project-docs/adr/0011-network-boundary.md new file mode 100644 index 0000000..1a6745c --- /dev/null +++ b/project-docs/adr/0011-network-boundary.md @@ -0,0 +1,74 @@ +# The network boundary is lint and render, not the whole binary + +`lint`, `render`, and the packages behind them never perform network I/O. +Commands that do are named, opt-in, and delegate transport to an external tool +rather than implementing it. This replaces the blanket "no network calls" rule +in `go-style.md`, which was a proxy for the property actually wanted. + +## Context + +`go-style.md` says: *"If a feature seems to need a network call, a daemon, or a +browser, that's a sign to redesign it, not to add the dependency."* Written +when modelith only ever read local files, that rule cost nothing and protected +something real. + +Cross-model references (ADR-0010) put pressure on it. A model vendored from +another repository has to get there somehow, and the honest options were to +abandon the feature, to make the user copy files by hand forever, or to admit +that a fetch is legitimate at a moment the user chooses. The blanket rule +could not express the third, so it would have been quietly violated or used to +block a feature it was never aimed at. + +The property worth protecting was never "this binary contains no socket." It +is that the commands people run constantly — in editors, in pre-commit hooks, +in CI on every pull request — are fast, deterministic, and work on a plane. + +## Decision + +**`lint` and `render` never touch the network.** Neither do `internal/lint`, +`internal/render/...`, `internal/schema`, or `internal/model`. This holds no +matter what flags are passed; there is no escape hatch that makes lint fetch +something. A feature that would require it is redesigned or refused, exactly +as the old rule said. + +**Commands that use the network say so in their name and are opt-in.** They +live under a distinct subcommand group, never run implicitly as a side effect +of another command, and never run at all unless asked. Where a check spans both +worlds — verifying a local file versus asking whether its origin moved — the +offline half is the default and the online half is a flag. + +**modelith does not implement network transport.** It shells out to `git` and +`gh`, which already handle authentication, private hosts, proxies, SSH agents, +and 2FA. The binary keeps no HTTP client, no TLS configuration, and no +credential handling. This is the choice `cmd/go` made for VCS fetches. + +## Why + +- **Determinism where it is load-bearing.** Rendering is byte-deterministic and + golden-tested. A renderer that can reach the network is a renderer whose + output depends on someone else's uptime. +- **The common path stays offline.** Air-gapped work, flights, and CI that + should not depend on a third party's availability all keep working, because + the commands those settings run are the ones on the offline side. +- **No credential surface.** The class of bug where a tool mishandles a token + cannot occur in a binary that has never seen one. +- **The old rule was a proxy.** "No network" was shorthand for "fast, + deterministic, no runtime dependencies." Naming the property directly makes + the rule usable in cases the shorthand mishandles. + +## Consequences + +- `go-style.md` is updated to state the boundary rather than the prohibition. + Where that file and this ADR disagree, this ADR is the decision and the file + is stale. +- A future proposal to make `lint` or `render` reach the network is refused by + this ADR, whatever the convenience argument. +- Adding an HTTP client to any package remains off the table; a network-using + command shells out instead. +- Pinned by `TestADR_0011_OfflinePackages`, which walks the transitive imports + of the offline packages and fails if any of them reaches `net`, `net/http`, + or `os/exec`. Banning `os/exec` there is deliberate: without it the rule + could be evaded by shelling out to `curl`. +- The offline packages import `net/url` and `net/netip` today, via the JSON + Schema library's format validation. Both are parsers that perform no I/O, so + they are permitted by name. From 08a738da21a8fdf116541bae9fcaf6c401036ae6 Mon Sep 17 00:00:00 2001 From: Joe Beda Date: Sun, 26 Jul 2026 13:23:58 -0700 Subject: [PATCH 2/3] docs(adr): address review findings on ADR-0010/0011 Review pass on PR #32 returned 2 blocking and 8 should-fix findings. Blocking. ADR-0010 described the freshness check two incompatible ways: as a flag on lint mirroring --completeness error, and as a command under modelith deps. The second is the decision; the flag wording was left behind when the command group was added, and ADR-0011 repeated it. lint never asks whether origin moved. Blocking. A vendored model is not the repo's work, but nothing said so operationally. The Action lints every globbed file with --completeness and optionally renders each one (action.yml:90-99), so a vendored model would turn a user's CI red for gaps in someone else's document. Ownership-scoped diagnostics now skip a file carrying a provenance header. Local imports. A model in the same repo, canonical where it sits, has no origin and no header. Provenance-header presence is the signal that separates a copy from a canonical file, and it does the same job three times: what to verify, what completeness applies to, and whether an unresolvable import is an error. Peers in one repo import each other with no ceremony. Also: go-style.md is actually updated rather than merely declared updated, and ADR-0011 no longer claims precedence over a rule file, which inverted CLAUDE.md; the TestSchemaStructSync claim is corrected, since it does not cover docs/06-schema-reference.md; duplicate and absent scopes among imports are decided; the narrowing of ADR-0002 that lossy imported Markdown represents is stated rather than left implicit; qualified names in prose are deferred explicitly; and ADR-0011 no longer implies its test proves the no-flag claim end to end, since cmd/modelith reaches net through pflag. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Joe Beda --- .claude/rules/go-style.md | 17 +++- ...010-cross-model-references-by-vendoring.md | 98 ++++++++++++++----- project-docs/adr/0011-network-boundary.md | 21 ++-- 3 files changed, 105 insertions(+), 31 deletions(-) diff --git a/.claude/rules/go-style.md b/.claude/rules/go-style.md index b8005fd..687e982 100644 --- a/.claude/rules/go-style.md +++ b/.claude/rules/go-style.md @@ -18,9 +18,20 @@ file wins. build flags (`LDFLAGS`) live; routing through it keeps them centralized. - The binary is static: `CGO_ENABLED=0`, no cgo dependencies. Verify a new dependency doesn't pull in cgo before adding it. -- One binary, no runtime dependencies. modelith reads and writes files and - prints diagnostics. If a feature seems to need a network call, a daemon, or - a browser, that's a sign to redesign it, not to add the dependency. +- One binary, no runtime dependencies for the common path. modelith reads and + writes files and prints diagnostics. If a feature seems to need a daemon or a + browser, that's a sign to redesign it, not to add the dependency. +- **`lint` and `render` never perform network I/O**, whatever flags are passed, + and neither do `internal/lint`, `internal/render/...`, `internal/schema`, or + `internal/model`. `TestADR_0011_OfflinePackages` enforces this by walking + their transitive imports for `net`, `net/http`, and `os/exec` — `os/exec` is + banned there so the rule can't be evaded by shelling out to `curl`. A + proposal to relax this for convenience is refused; see + [`0011-network-boundary.md`](../../project-docs/adr/0011-network-boundary.md). +- A command that does use the network says so in its name, runs only when + asked, and never as a side effect of another command. It delegates transport + to `git`/`gh` rather than adding an HTTP client, so the binary holds no TLS + configuration and no credentials. ## Dependencies diff --git a/project-docs/adr/0010-cross-model-references-by-vendoring.md b/project-docs/adr/0010-cross-model-references-by-vendoring.md index 4c2d280..29dc2e1 100644 --- a/project-docs/adr/0010-cross-model-references-by-vendoring.md +++ b/project-docs/adr/0010-cross-model-references-by-vendoring.md @@ -43,13 +43,31 @@ only attribute `type` may be qualified; see "deferred" below. **Resolution.** A model lists `imports:` as paths relative to itself. Each listed file declares its own `scope:`, which is the single source of truth for the slug — nothing is repeated, so nothing can disagree. There is no directory -convention and no scanning. Resolution does not recurse: a vendored model's own -`imports:` are ignored, because those paths describe its repo, not this one. - -**Vendoring.** An imported model is a whole copy committed to this repo, -unmodified except for a provenance header. Whole-file rather than an extracted -subset, so that referencing something new from an already-imported model is a -local edit rather than another fetch. +convention and no scanning. Resolution does not recurse: an imported model's +own `imports:` are ignored, so only items defined directly in a listed file are +reachable. Two listed files declaring the same `scope:` is an error, as is +listing a file that declares none — with no alias layer there is no local +remedy for a collision, so it has to be named rather than silently resolved. + +**Local and vendored imports.** An import is either *local* — a model that +lives in this repo and is canonical here — or *vendored* — a copy of a model +whose home is elsewhere. Both are written the same way, as a path in +`imports:`. The difference is not declared: **a provenance header is the +signal.** A file carrying one is a copy, so it is verified against that header +and can be refreshed from origin. A file without one is canonical here, so +there is nothing to verify, nothing to refresh, and its absence of a header is +not a finding. Two models that are peers in one repo therefore import each +other with no ceremony at all, which is the common case and must stay the +cheapest one. + +The cost is that a file copied in by hand, with no header, is indistinguishable +from one that is canonical. That is consistent with the posture below: this +mechanism detects drift, it does not police it. + +**Vendoring.** A vendored import is a whole copy committed to this repo, +unmodified except for its provenance header. Whole-file rather than an +extracted subset, so that referencing something new from an already-imported +model is a local edit rather than another fetch. **Provenance.** Origin, imported-at, and a surface digest live in a `# modelith:` header comment in the vendored file. Not in the schema: an @@ -72,12 +90,28 @@ explicit freshness check has both files in hand, it diffs them to report what. **Checks.** `lint` re-verifies vendored files against their own headers by default: offline, free, and it catches the hand-edit that a `DO NOT EDIT` -comment cannot prevent. Checking whether *origin* has moved is opt-in behind a -flag, so a repo that wants aggressive skew detection enables it in CI and -everyone else never makes a network call. This mirrors `--completeness error`: -one command, posture chosen per repo. The digest is a drift detector, not a -security boundary — anyone editing a vendored file can recompute the header, -and that is fine. +comment cannot prevent. Local imports have no header and so have nothing to +verify. Whether *origin* has moved is a separate question that `lint` never +asks; it belongs to `modelith deps check`, and a repo that wants aggressive +skew detection runs that in CI. The digest is a drift detector, not a security +boundary — anyone editing a vendored file can recompute the header, and that +is fine. + +**Whose model is it.** A vendored file is not this repo's work, and +ownership-scoped diagnostics must not fire on it: completeness gaps, unused +enums and glossary terms, and unexercised entities are all findings about a +model its authors control. The provenance header is the signal here too. This +matters beyond taste, because the Action this repo ships lints every globbed +file with `--completeness` and optionally renders each one +(`action.yml:90-99`), so without this rule a vendored model turns a user's CI +red for gaps in someone else's document. Excluding a vendored directory from +the glob is the user's other lever, but it must not be the only one. + +**A vendored file linted on its own.** Globs and editors will lint vendored +files directly, and such a file's own `imports:` point at paths in its home +repo that do not exist here. Unresolvable imports in a file that carries a +provenance header are skipped rather than reported; in a file without one they +are an error. This is the same signal doing the same job a third time. **Fetching.** modelith does not implement network transport; it shells out (ADR-0011). `gh` already solves authentication for private and internal @@ -91,7 +125,15 @@ rendered `.md` — but never expands imported definitions and never opens anothe file. Output stays a pure function of the single model being rendered, so the golden-fixture invariant holds and a vendored file changing cannot silently rewrite an unrelated `.md`. Links to un-rendered models will dangle; that is -accepted. +accepted, as is a link landing on the wrong heading when an entity and an enum +share a name, since both render as `### \`Name\`` into the same anchor. + +This is a deliberate narrowing of ADR-0002, which holds that the Markdown +carries the full truth while the Mermaid ER is the lossy view. For imported +items the Markdown is lossy too: it names the dependency and links to it rather +than restating it. Determinism wins over completeness here, because a renderer +that reads other files is a renderer whose golden output changes when a file it +does not own changes. **Commands.** Everything that manages an imported model lives under `modelith deps`: `import` adds a model and stamps its header, `update` @@ -117,19 +159,31 @@ boundary. Relaxing the pattern later is backward compatible. Following ADR-0007, the bar to lift this is a concrete modelling failure, not expressiveness for its own sake. +Qualified names in prose are deferred with them. `lint`'s backticked-term check +(`lint.go:864-877`) warns on an unknown `` `Name` `` but will not resolve +`` `slug.Name` ``, so a cross-model reference in a `definition:` stays +unchecked. That is the pre-existing behaviour for anything non-PascalCase, and +this change does not improve it. + ## Consequences -- The format gains `scope:` and `imports:`; the schema, the structs, and the - schema reference change together, guarded by `TestSchemaStructSync`. +- The format gains `scope:` and `imports:`. The schema and the structs are held + together by `TestSchemaStructSync`; `docs/06-schema-reference.md` is not + covered by it and has to be updated by hand in the same change. - Attribute `type` stops being an unconstrained string. A qualified type whose scope is not imported becomes an error; today such a value is silently treated as a primitive, because `lint.go`'s check only fires on PascalCase. - `lint` and `render` gain the ability to read files other than their argument — `lint` to resolve imports, `render` only to the extent of emitting links. -- New command surface for import, update, and freshness checking. -- A vendored model is a whole extra model in the repo. Completeness and - "unused" diagnostics must not fire on it, and it must not be mistaken for a - model the repo owns. +- New command surface under `modelith deps`. +- A vendored model is a whole extra model in the repo, and every consumer of a + model file has to learn the vendored-or-not distinction: `lint`, the + completeness rules, `render --check`, the shipped Action, and this repo's + own `TestADR_0009_NoSelfModel` allowlist, which fails on any `*.modelith.yaml` + it does not know about. - Nothing pins this ADR by test yet, because none of it is built. When it - lands, `TestADR_0010_*` covers the digest's field selection and the - no-transitive-resolution rule; both are decisions a test can hold. + lands, `TestADR_0010_*` covers the digest's field selection, the + no-transitive-resolution rule, and that ownership-scoped diagnostics skip a + file with a provenance header; all three are decisions a test can hold. The + deep-link anchor scheme needs a test too, since it depends on the renderer's + heading format rather than on anything declared. diff --git a/project-docs/adr/0011-network-boundary.md b/project-docs/adr/0011-network-boundary.md index 1a6745c..e02f3b8 100644 --- a/project-docs/adr/0011-network-boundary.md +++ b/project-docs/adr/0011-network-boundary.md @@ -33,9 +33,10 @@ as the old rule said. **Commands that use the network say so in their name and are opt-in.** They live under a distinct subcommand group, never run implicitly as a side effect -of another command, and never run at all unless asked. Where a check spans both -worlds — verifying a local file versus asking whether its origin moved — the -offline half is the default and the online half is a flag. +of another command, and never run at all unless asked. Where a question spans +both worlds — verifying a local file versus asking whether its origin moved — +the offline half belongs to the command people already run and the online half +is a separate command, not a flag on the first. **modelith does not implement network transport.** It shells out to `git` and `gh`, which already handle authentication, private hosts, proxies, SSH agents, @@ -58,9 +59,11 @@ credential handling. This is the choice `cmd/go` made for VCS fetches. ## Consequences -- `go-style.md` is updated to state the boundary rather than the prohibition. - Where that file and this ADR disagree, this ADR is the decision and the file - is stale. +- `go-style.md` is updated in this change to state the boundary rather than the + prohibition, so the two do not disagree. That file remains authoritative on + Go conventions per `CLAUDE.md`; an ADR records why a rule changed, it does not + outrank the rule file. Keeping them in step matters because `go-style.md` + loads on every Go edit and an ADR does not. - A future proposal to make `lint` or `render` reach the network is refused by this ADR, whatever the convenience argument. - Adding an HTTP client to any package remains off the table; a network-using @@ -69,6 +72,12 @@ credential handling. This is the choice `cmd/go` made for VCS fetches. of the offline packages and fails if any of them reaches `net`, `net/http`, or `os/exec`. Banning `os/exec` there is deliberate: without it the rule could be evaded by shelling out to `curl`. +- That test guards the library packages, not the commands. `cmd/modelith` + reaches `net` transitively through `spf13/pflag` today, and the network-using + commands will legitimately import `os/exec`, so the package-level ban cannot + extend there. "No flag makes `lint` fetch" is therefore a design commitment + enforced by the library boundary rather than a claim a test proves end to + end; a command-level guard would need to assert on wiring, not imports. - The offline packages import `net/url` and `net/netip` today, via the JSON Schema library's format validation. Both are parsers that perform no I/O, so they are permitted by name. From 8791d7334df6d6ab82d50e96c9cb526bca0d4a79 Mon Sep 17 00:00:00 2001 From: Joe Beda Date: Sun, 26 Jul 2026 13:28:17 -0700 Subject: [PATCH 3/3] docs(adr): close the two design gaps round 2 flagged render --check has the same shape as the completeness problem: it fails when a model's .md is absent or stale, and a vendored model owes neither -- its rendered form belongs to its home repo. Vendored files are skipped by render --check; naming one explicitly still renders it, which is how a deep link gets a target. The scope-collision text said there was no local remedy, which stopped being true when the same change introduced local imports: two local models collide and you rename one. Only vendored copies of two upstreams that chose the same slug are stuck, because there is no alias layer. Also names lint as where the error is raised. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Joe Beda --- .../0010-cross-model-references-by-vendoring.md | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/project-docs/adr/0010-cross-model-references-by-vendoring.md b/project-docs/adr/0010-cross-model-references-by-vendoring.md index 29dc2e1..68c3934 100644 --- a/project-docs/adr/0010-cross-model-references-by-vendoring.md +++ b/project-docs/adr/0010-cross-model-references-by-vendoring.md @@ -45,9 +45,12 @@ listed file declares its own `scope:`, which is the single source of truth for the slug — nothing is repeated, so nothing can disagree. There is no directory convention and no scanning. Resolution does not recurse: an imported model's own `imports:` are ignored, so only items defined directly in a listed file are -reachable. Two listed files declaring the same `scope:` is an error, as is -listing a file that declares none — with no alias layer there is no local -remedy for a collision, so it has to be named rather than silently resolved. +reachable. Two listed files declaring the same `scope:` is a lint error, as is +listing a file that declares none. Between two local models a collision is +fixed by renaming a `scope:`; between vendored copies of two upstreams that +chose the same slug there is no local remedy, because there is no alias layer +to bind one of them to a different name. Either way it is named rather than +silently resolved to whichever file was listed first. **Local and vendored imports.** An import is either *local* — a model that lives in this repo and is canonical here — or *vendored* — a copy of a model @@ -107,6 +110,14 @@ file with `--completeness` and optionally renders each one red for gaps in someone else's document. Excluding a vendored directory from the glob is the user's other lever, but it must not be the only one. +`render --check` is the same problem wearing different clothes: it fails when a +model's `.md` is absent or stale, and a vendored model arrives with neither +obligation — its rendered form belongs to its home repo. Vendored files are +therefore skipped by `render --check` rather than being required to carry a +committed `.md` that this repo would then have to regenerate whenever upstream +moved. Rendering one explicitly, by naming it, still works; that is how a deep +link into a vendored model's `.md` gets something to point at. + **A vendored file linted on its own.** Globs and editors will lint vendored files directly, and such a file's own `imports:` point at paths in its home repo that do not exist here. Unresolvable imports in a file that carries a