diff --git a/openspec/changes/rescope-go-code-standards/.openspec.yaml b/openspec/changes/rescope-go-code-standards/.openspec.yaml new file mode 100644 index 0000000..f161d5c --- /dev/null +++ b/openspec/changes/rescope-go-code-standards/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-08-16 diff --git a/openspec/changes/rescope-go-code-standards/design.md b/openspec/changes/rescope-go-code-standards/design.md new file mode 100644 index 0000000..f4a6a98 --- /dev/null +++ b/openspec/changes/rescope-go-code-standards/design.md @@ -0,0 +1,145 @@ +## Context + +`go-code-standards` holds seven requirements. Two are policy no tool can check: +mocks are generated, and a test does not re-cover behavior through an exported +alias. Five state source formatting that `gofumpt`, `golines`, `goimports`, +`wrapcheck`, and `revive` already produce or reject. + +The five repositories carrying Go code sit in three states. `osapi` and `gohai` +point at the capability without restating it. `nats-client` and `nats-server` +restate it under `Code style`. `osapi-orchestrator` has a removal open and held. +The conversion that produced this spread is half-finished, which is why the +question is worth settling now rather than after two more repositories move. + +`osapi-justfiles` already distributes shared recipes: a consumer runs +`just fetch`, files land in `.just/remote/`, and `.gitignore` keeps them +untracked. The `justfiles` capability records this under "Recipes are +distributed as fetched files" and "Fetched files are not linted". A second kind +of shared content needs no new mechanism. + +## Goals / Non-Goals + +**Goals:** + +- Keep in the corpus only what the corpus is for — decisions binding several + repositories that no tool reports on. +- Leave every rule in force. Nothing here relaxes a convention; each moves to + whichever of three homes can actually hold it. +- Let a repository be read on its own, by a person offline or an agent with one + checkout. + +**Non-Goals:** + +- Changing any convention. Signatures still span lines, imports stay grouped, + suites keep their names. +- Rewriting `.golangci.yml` to unify the per-repository exclusions. Those + differences are legitimate and this change does not touch them. +- Deciding what `osapi-justfiles` names the fragment or which recipe fetches it. + That is applying, not deciding. + +## Decisions + +### Three homes, chosen by what can hold a rule + +A rule goes to the corpus if it is a decision with consequences and no tool +reports on it; to tool configuration if a tool can enforce it; to the shared +`CONTRIBUTING.md` fragment otherwise. + +*Alternative: keep all seven in the corpus and add worked examples.* Rejected +after starting it. Adding Go snippets to a requirement makes it unambiguously +about implementation, which `config.yaml` says a requirement is not. It treats +the symptom — the capability is hard to apply without an example — while making +the category error harder to see. + +*Alternative: move all seven out and retire the capability.* Rejected. "Mocks +are generated" is exactly what a cross-repository corpus is for: it is a +decision, it has a stated failure mode, no linter checks it, and three +repositories currently violate it. Retiring the capability would delete the two +requirements worth having. + +### Distribute the fragment rather than point at the corpus + +Each repository holds the shared conventions on disk, fetched from one source, +rather than naming a capability in another repository. + +The failure that motivated this is recorded: `gohai` merged its removal while +`go-code-standards` existed only inside an unarchived change. For that period +the rules were stated once and that place was unreachable — a reader following +the pointer to `openspec/specs/` found nothing. A pointer is only as good as +what it resolves to, and it resolves across a repository boundary, over a +network, for every reader. + +*Alternative: keep the pointer and deep-link the file.* Rejected as +insufficient. It fixes findability for a person with a browser and leaves the +offline reader and the single-checkout agent with a link they cannot follow. + +*Alternative: git submodule for the corpus.* Rejected. It puts the whole spec +repository into every consumer to deliver one document, and submodules pin a +revision that then needs its own bumping. + +### Configuration is the statement for anything a tool checks + +Where `.golangci.yml` or a formatter decides a rule, documentation names where +the configuration lives instead of reproducing it. + +The failure is already on disk. Every Go repository's `CONTRIBUTING.md` lists +"errcheck, errname, goimports, govet, prealloc, predeclared, revive, +staticcheck". Each `.golangci.yml` enables `unused`, which no prose mentions, +and puts `goimports` under `formatters` rather than among the linters. One stale +approximation, copied five times, of a file sitting beside it. + +*Alternative: keep the prose list and add a check comparing it to the config.* +Rejected. Building a checker to keep a document honest about a file it +duplicates is more machinery than deleting the duplicate. + +### The capability keeps its name and path + +`go-code-standards` narrows rather than being renamed or split. + +*Alternative: rename to `go-testing-policy` to match what survives.* Rejected. +Five repositories reference the name today, a rename invalidates every one of +them, and the capability may legitimately regain non-lintable Go policy later. + +### Mocks gains a scenario rather than an exception + +The `Mocks are generated` requirement is modified to say that a double carrying +a real implementation is not a mock. + +Applying task 3.4 turned up three hand-written doubles. `gohai`'s +`fakeCollector` and `osapi-orchestrator`'s `mockRenderer` are scripted stand-ins +and do violate the rule. `osapi`'s `mockPKISigner` signs with a genuinely +generated ed25519 key pair — replacing it with a generated mock would swap +working cryptography for a canned return. The requirement did not distinguish +these, so applying it produced a finding against a test that is correct. + +*Alternative: leave the requirement and record `mockPKISigner` as an accepted +exception.* Rejected. An exception recorded outside the requirement is invisible +at the point anyone reads the rule, and the next reviewer raises it again. + +## Risks / Trade-offs + +- **The fragment is fetched, so a repository read on GitHub shows a pointer + rather than the conventions.** → The fetch mechanism already carries this + trade-off for recipes and it is accepted there. What a browser reader loses is + smaller than what an offline reader and a single-checkout agent gain, and the + fragment is one fetch away rather than one repository away. + +- **Two homes for Go conventions means a contributor must know which holds + what.** → The split follows a line a contributor can apply without being told: + if a tool rejects it, it is configuration; if a reviewer rejects it, it is + written down. + +- **Removing five requirements shrinks the corpus while the behavior + capabilities queued elsewhere have not arrived, making it look emptier.** → + The corpus is measured by what it governs, not its length. Five requirements + restating a formatter make it longer without making it say more. + +- **`gohai` is on `main` today with its conventions removed.** → Syncing the + capability into the corpus already restored a reachable answer. This change + determines what returns to the repository, and until it lands `gohai` is + covered by the synced capability rather than by nothing. + +- **`osapi-justfiles` becomes a dependency for reading a repository's + conventions.** → It already is, for building and testing one. A repository + that cannot fetch cannot run its checks either, so this adds no new failure + mode. diff --git a/openspec/changes/rescope-go-code-standards/proposal.md b/openspec/changes/rescope-go-code-standards/proposal.md new file mode 100644 index 0000000..d4b55b2 --- /dev/null +++ b/openspec/changes/rescope-go-code-standards/proposal.md @@ -0,0 +1,73 @@ +## Why + +`go-code-standards` states Go formatting conventions — multi-line signatures, +import grouping, error wrapping, early return — as corpus requirements. These +are not externally observable behavior, which is the level `config.yaml` says a +requirement sits at, and the strain shows in the scenarios written to satisfy +the format: "a parameter is added → the diff shows one added line" is a +rationale for a formatting preference, not a behavior anything can exhibit. + +The duplication this was meant to end has not ended, and the written rule is +already wrong. Every Go repository's `CONTRIBUTING.md` lists the linter set as +"errcheck, errname, goimports, govet, prealloc, predeclared, revive, +staticcheck". Each `.golangci.yml` enables `unused`, which no repository's prose +mentions, and configures `goimports` under `formatters` rather than as a linter. +Five hand-maintained copies of one stale approximation of a file that already +declares the truth — the drift the capability was written to prevent, occurring +inside the capability's own subject. + +Applying the capability exposed the third problem. `gohai` dropped its local +copy while `go-code-standards` existed only inside an unarchived change, so for +a period the rules were stated in exactly one place and that place was not +reachable. Removing a repository's copy is only safe when something reachable +answers in its place. + +## What Changes + +- **BREAKING** `go-code-standards` is reduced to the rules that are genuine + cross-repository policy — decisions with consequences that no tool can check. + Mocks being generated rather than hand-written, and a test not re-covering + behavior through an exported alias, stay. Function signature layout, file + naming, suite naming, table-driven structure, and the style baseline leave the + corpus. +- Formatting rules a tool already enforces are enforced rather than written. + `golangci-lint` and the formatters it runs are the statement of record; a rule + a linter checks is not restated in prose anywhere. +- Conventions that remain prose — worked examples, the package names a + repository uses for its external tests — return to `CONTRIBUTING.md`, where a + contributor and an agent already read them without a second repository. +- A shared `CONTRIBUTING.md` fragment is distributed the way shared recipes + already are, so one source produces the copy each repository holds on disk. + +## Capabilities + +### New Capabilities + +- `shared-contributor-documentation`: how contributor guidance common to several + repositories is distributed, so each repository holds a complete file on disk + while one source governs its content + +### Modified Capabilities + +- `go-code-standards`: removes the requirements that state formatting a tool + enforces, and narrows the capability's purpose to cross-repository policy + +## Impact + +Every repository carrying Go code, and the shared tooling repository that would +distribute the fragment: + +- `gohai` — has already dropped its copy (osapi-io/gohai#163); this change + determines what returns to it +- `osapi-orchestrator` — the equivalent removal is open and held + (osapi-io/osapi-orchestrator#76) +- `nats-client`, `nats-server` — still carry their copies, untouched +- `osapi` — its root `CONTRIBUTING.md` points without restating + (osapi-io/osapi#450) +- `osapi-justfiles` — would carry the shared fragment and the recipe that + fetches it + +`specify-go-code-standards` is in flight and its tasks 2.2, 2.3, 2.4, 3.4, and +3.6 are open. Those tasks convert the remaining repositories to a pointer-only +form; this change decides whether that conversion is the right destination +before two more repositories follow `gohai` into it. diff --git a/openspec/changes/rescope-go-code-standards/specs/go-code-standards/spec.md b/openspec/changes/rescope-go-code-standards/specs/go-code-standards/spec.md new file mode 100644 index 0000000..30ac0e0 --- /dev/null +++ b/openspec/changes/rescope-go-code-standards/specs/go-code-standards/spec.md @@ -0,0 +1,87 @@ +## MODIFIED Requirements + +### Requirement: Mocks are generated + +Where a test replaces an interface with a mock, the mock SHALL be generated by +`mockgen` from `go.uber.org/mock` and committed. A mock SHALL NOT be written by +hand. + +A hand-written mock drifts from its interface silently: adding a method breaks +the generated mock at compile time and leaves the hand-written one satisfying an +interface it no longer matches. + +This requirement binds a repository that mocks an interface. A repository that +tests against a real implementation — a filesystem in memory, a local HTTP +server — is not required to introduce mocking. + +A test double that carries a real implementation of the behavior under test, +rather than a scripted response, is not a mock and is not bound by this +requirement. + +#### Scenario: An interface gains a method + +- **WHEN** a method is added to a mocked interface +- **THEN** regenerating the mock is the only step needed, and skipping it fails + the build + +#### Scenario: A test needs a substitute implementation + +- **WHEN** a test needs to stand in for a dependency +- **THEN** it uses a generated mock or a real implementation, rather than a + struct written to satisfy the interface + +#### Scenario: A double does the real work + +- **WHEN** a test substitute performs the genuine operation, such as signing + with a real generated key pair +- **THEN** it is a real implementation rather than a mock, and generating it + would replace working behavior with a scripted one + +## REMOVED Requirements + +### Requirement: A function signature with parameters spans lines + +**Reason**: States source formatting rather than externally observable behavior. +`golines`, run by every repository's format recipe, already produces this layout +and produces it identically everywhere, so the written rule can only restate or +contradict what the formatter does. + +**Migration**: The formatter is the statement of record. A worked example +belongs in `CONTRIBUTING.md`, where a contributor reads it beside the command +that applies it. + +### Requirement: A file is named for what it holds + +**Reason**: A naming convention is a judgment a reader applies, not behavior a +system exhibits. The rule remains worth stating; the corpus is not where a +contributor looks for it. + +**Migration**: Moves to the shared `CONTRIBUTING.md` fragment, which every +repository holds on disk. + +### Requirement: Tests are table-driven suites + +**Reason**: Describes how tests are written rather than what the software does. +No requirement here constrains the behavior of any osapi-io component. + +**Migration**: Moves to the shared `CONTRIBUTING.md` fragment. + +### Requirement: A suite is named for the surface it tests + +**Reason**: A naming convention for test types, with no observable consequence +outside the test files themselves. + +**Migration**: Moves to the shared `CONTRIBUTING.md` fragment, together with the +per-repository package names it depends on. + +### Requirement: Style baseline + +**Reason**: Every clause is enforced by a tool the repositories already run — +`gofumpt` and `goimports` for formatting and import grouping, `wrapcheck` and +`revive` for error wrapping and early return. A rule a linter checks is +configuration, and stating it in prose as well produced a linter list that names +`goimports` as a linter, omits `unused`, and is copied into five repositories. + +**Migration**: `.golangci.yml` and the formatter configuration become the single +statement. Where a repository needs an exception it is expressed there, where +the tool reads it. diff --git a/openspec/changes/rescope-go-code-standards/specs/shared-contributor-documentation/spec.md b/openspec/changes/rescope-go-code-standards/specs/shared-contributor-documentation/spec.md new file mode 100644 index 0000000..5e50426 --- /dev/null +++ b/openspec/changes/rescope-go-code-standards/specs/shared-contributor-documentation/spec.md @@ -0,0 +1,91 @@ +## Purpose + +Defines how contributor guidance common to several repositories is distributed, +so that each repository holds a complete guide on disk while one source governs +the shared part of its content. + +## ADDED Requirements + +### Requirement: A shared convention has one source + +A convention that binds more than one repository SHALL be written in one place +and distributed from there. It SHALL NOT be maintained as an independent copy in +each repository that follows it. + +Independent copies diverge without anything reporting it, and each reader +believes the copy in front of them is current. + +#### Scenario: A convention changes + +- **WHEN** a convention that binds several repositories is amended +- **THEN** it is amended once and redistributed, rather than edited separately + in each repository + +#### Scenario: Copies have already diverged + +- **WHEN** two repositories state the same convention differently +- **THEN** neither is authoritative, and the divergence is resolved by + establishing the single source rather than by choosing the copy that looks + more current + +### Requirement: A repository holds the guidance it is bound by + +A repository SHALL contain the full text of the conventions its contributors are +held to, readable without fetching another repository. + +A pointer to guidance stored elsewhere SHALL NOT be the only statement of a +convention. A reader working offline, an agent with no second checkout, and a +reviewer reading a pull request in a browser each see only this repository. + +#### Scenario: A contributor reads the guide + +- **WHEN** a contributor opens a repository's contributing guide +- **THEN** the conventions they must follow are present in it, rather than named + and left to be retrieved + +#### Scenario: An agent works in a single checkout + +- **WHEN** an agent works in one repository with no access to another +- **THEN** the conventions binding that repository are readable from within it + +### Requirement: Distributed content is fetched, not committed + +Shared contributor documentation SHALL reach a repository by the same mechanism +as its other shared assets, and the fetched copy SHALL NOT be committed. + +A committed copy is indistinguishable from a local edit, so the next fetch +either overwrites deliberate changes or is not run at all. + +#### Scenario: A repository is set up + +- **WHEN** a contributor prepares a fresh checkout +- **THEN** the same command that retrieves the repository's other shared assets + retrieves its shared documentation + +#### Scenario: A fetched file is edited locally + +- **WHEN** a repository needs shared guidance to differ +- **THEN** the difference is expressed where the source can produce it, rather + than by editing the fetched copy + +### Requirement: A rule a tool enforces is not also written as prose + +Where a tool's configuration determines a convention, that configuration SHALL +be the statement of record, and the convention SHALL NOT be restated as prose +that can disagree with it. + +Prose describing a tool's settings is maintained by hand and checked by nobody, +so it drifts from the configuration while continuing to read as authoritative. + +#### Scenario: A linter set is documented + +- **WHEN** contributor documentation lists which linters run +- **THEN** it names where the configuration lives rather than reproducing the + list, because a reproduced list goes stale the first time the configuration + changes + +#### Scenario: Prose and configuration disagree + +- **WHEN** documentation and a tool's configuration state different rules +- **THEN** the configuration is what runs, and the prose is removed rather than + corrected diff --git a/openspec/changes/rescope-go-code-standards/tasks.md b/openspec/changes/rescope-go-code-standards/tasks.md new file mode 100644 index 0000000..2f5d1a9 --- /dev/null +++ b/openspec/changes/rescope-go-code-standards/tasks.md @@ -0,0 +1,73 @@ +## 1. Record the rescope + +- [x] 1.1 Establish what each requirement is: policy no tool reports on, + something a tool already enforces, or a convention a reader applies +- [x] 1.2 Write the `go-code-standards` delta removing the five formatting + requirements and widening `Mocks are generated` to exclude a double that + carries a real implementation +- [x] 1.3 Write the `shared-contributor-documentation` capability +- [x] 1.4 Record the decisions and their rejected alternatives in design.md + +## 2. Establish the shared fragment + +Nothing is removed from a repository until the fragment that replaces it exists +and can be fetched. + +- [ ] 2.1 `osapi-justfiles` — write the shared Go conventions fragment: file + naming, `types.go` for types only, test file naming, table-driven suites, + suite naming, the `export_test.go` pattern, and worked signature examples. + Verified by the fragment containing every rule removed from + `go-code-standards` by this change +- [ ] 2.2 `osapi-justfiles` — add the recipe that fetches it, following the + pattern the `justfiles` capability records for recipes. Verified by a fresh + checkout of a consumer producing the file +- [ ] 2.3 Confirm the fetched path is ignored rather than committed in every + consumer, and that no formatter or linter runs against it + +## 3. Return the conventions to each repository + +One repository per pull request, each fetching the fragment and stating its own +conventions beside it. + +- [ ] 3.1 `gohai` — restore the shared conventions via the fragment. It is on + `main` with them removed (osapi-io/gohai#163), so it goes first +- [ ] 3.2 `osapi-orchestrator` — supersede the held removal + (osapi-io/osapi-orchestrator#76) with the fragment, and close it +- [ ] 3.3 `osapi` — replace the pointer in its root `CONTRIBUTING.md` + (osapi-io/osapi#450) with the fragment +- [ ] 3.4 `nats-client` — replace its restated copy with the fragment +- [ ] 3.5 `nats-server` — replace its restated copy with the fragment + +## 4. Let the configuration speak for what it enforces + +- [ ] 4.1 Remove the hand-maintained linter list from every repository's + contributor documentation, naming `.golangci.yml` instead. Verified by no + repository's prose enumerating linters +- [ ] 4.2 Confirm the removed lists were wrong in the same way everywhere — + `goimports` named as a linter, `unused` omitted — so the reason for removing + them is recorded rather than asserted + +## 5. Resolve the mocks finding + +- [ ] 5.1 `osapi` — record `mockPKISigner` as a real implementation under the + widened requirement, rather than converting it. It signs with a generated + ed25519 key pair, and a generated mock would replace that with a canned return +- [ ] 5.2 `gohai` — replace `fakeCollector` with a generated mock, or state why + the collector interface is better served by a real implementation +- [ ] 5.3 `osapi-orchestrator` — replace `mockRenderer` with a generated mock, + and correct `CONTRIBUTING.md`, which says the repository declares no mocking + library while hand-rolling one + +## 6. Verification + +- [ ] 6.1 Confirm every rule removed from `go-code-standards` is stated in the + fragment or enforced by a tool, and that none was dropped +- [ ] 6.2 Confirm each of the five repositories holds the conventions on disk, + readable without fetching another repository +- [ ] 6.3 Confirm no convention is stated both in the fragment and in a + repository's own section +- [ ] 6.4 Confirm `go-code-standards` retains only requirements no tool reports + on +- [ ] 6.5 Confirm `specify-go-code-standards` tasks 2.2, 2.3, 2.4, and 3.6 are + reconciled with this change rather than left describing the pointer-only + destination it replaces