Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions openspec/changes/rescope-go-code-standards/.openspec.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-08-16
145 changes: 145 additions & 0 deletions openspec/changes/rescope-go-code-standards/design.md
Original file line number Diff line number Diff line change
@@ -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.
73 changes: 73 additions & 0 deletions openspec/changes/rescope-go-code-standards/proposal.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
@@ -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.
Loading