diff --git a/AGENTS.md b/AGENTS.md index a830e2de1..1d0de7583 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -17,6 +17,7 @@ - **CLI error messages must name the resource.** When a CLI command fails for a named resource (task, spawner, workspace, etc.), include the resource name in the returned error so operators piping or batching invocations get an actionable signal. `fmt.Errorf("task %s failed", name)`, not `errors.New("task failed")`. - **Test the happy path, not only the early-return guards.** When a handler has both early-return guards and a primary action (post a message, create a resource, emit a metric), unit tests must include at least one positive case verifying the primary action runs with the right arguments — not just the no-op branches. If the production code lacks a seam, add one (interface, function field, fake client) so the happy path is covered. - **Avoid vacuous substring assertions in printer/formatter tests.** When asserting a `label: value` line is emitted, match against the full `"label: value"` string (or a regex), not the bare value — bare values often collide with the resource's `Name` or other surrounding context in the fixture and pass even when the line is missing. +- **Cover the whole matrix when writing matrix-shaped tests.** When a test iterates over a set of resource kinds, agent types, or subcommands (table-driven cases, `for _, r := range resources`, the `agentConfigs` parametrization), include every member of that set that supports the behavior under test. If a member is deliberately omitted, leave an inline comment naming the gate (e.g., `// task and taskspawner have no create subcommand`). Reviewers should not have to ask "why isn't X here?" — the comment should answer it. - **Keep CRD enum docstrings consistent with the `+kubebuilder:validation:Enum` marker.** If the godoc says "empty matches both", either include `""` in the enum list so `field: ""` is accepted, or rephrase to "Omit to match both" so no one writes the explicit empty form. A docstring that invites a value the API server then rejects is a worse contract than either alternative. - **Qualify cross-CRD field references with the owning kind in docs.** In a CRD reference section, write `Task.spec.podOverrides.env` rather than bare `podOverrides.env` when describing a field that lives on a sibling CRD. A reader of one CRD's reference page should be able to locate the cited field without already knowing the layout of the others. diff --git a/self-development/agentconfig.yaml b/self-development/agentconfig.yaml index c9c4566b8..c7d4b1a3c 100644 --- a/self-development/agentconfig.yaml +++ b/self-development/agentconfig.yaml @@ -41,6 +41,7 @@ spec: - CLI error messages must name the resource: when a CLI command fails for a named resource (task, spawner, workspace, etc.), include the name in the returned error so operators piping or batching invocations get an actionable signal — `fmt.Errorf("task %s failed", name)`, not `errors.New("task failed")`. - Test the happy path, not only early-return guards: when a handler has both guard branches and a primary action (post a message, create a resource, emit a metric), include at least one positive test verifying the primary action runs with the right arguments. If the production code lacks a seam, add one (interface, function field, fake client) so the happy path is testable. - Avoid vacuous substring assertions in printer/formatter tests: when asserting a `label: value` line is emitted, match the full `"label: value"` string (or a regex), not the bare value — bare values frequently collide with the fixture's `Name` or surrounding context and pass even when the line is missing. + - Cover the whole matrix when writing matrix-shaped tests: when a test iterates over resource kinds, agent types, or subcommands (table-driven cases, `for _, r := range resources`, the `agentConfigs` parametrization), include every member of that set that supports the behavior under test. If a member is deliberately omitted, leave an inline comment naming the gate (e.g., `// task and taskspawner have no create subcommand`). Reviewers should not have to ask "why isn't X here?". - Keep CRD enum docstrings consistent with the `+kubebuilder:validation:Enum` marker: if the godoc says "empty matches both", either include `""` in the enum list so `field: ""` is accepted, or rephrase to "Omit to match both" so no one writes the explicit empty form. A docstring that invites a value the API server then rejects is a worse contract than either alternative. - Qualify cross-CRD field references with the owning kind in docs: in a CRD reference section, write `Task.spec.podOverrides.env` rather than bare `podOverrides.env` when describing a field that lives on a sibling CRD. A reader of one CRD's reference page should be able to locate the cited field without already knowing the layout of the others. - PRs that only modify files under `self-development/` are internal agent improvements: use `/kind cleanup` and write "NONE" in the `release-note` block, even when the change fixes a bug or adds a feature in agent behavior. Classify by file location, not by problem nature. diff --git a/self-development/kelos-workers.yaml b/self-development/kelos-workers.yaml index 71fcf7795..732ba646e 100644 --- a/self-development/kelos-workers.yaml +++ b/self-development/kelos-workers.yaml @@ -40,6 +40,7 @@ spec: - CLI error messages must name the resource: when a CLI command fails for a named resource (task, spawner, workspace, etc.), include the name in the returned error so operators piping or batching invocations get an actionable signal — `fmt.Errorf("task %s failed", name)`, not `errors.New("task failed")`. - Test the happy path, not only early-return guards: when a handler has both guard branches and a primary action (post a message, create a resource, emit a metric), include at least one positive test verifying the primary action runs with the right arguments. If the production code lacks a seam, add one (interface, function field, fake client) so the happy path is testable. - Avoid vacuous substring assertions in printer/formatter tests: when asserting a `label: value` line is emitted, match the full `"label: value"` string (or a regex), not the bare value — bare values frequently collide with the fixture's `Name` or surrounding context and pass even when the line is missing. + - Cover the whole matrix when writing matrix-shaped tests: when a test iterates over resource kinds, agent types, or subcommands (table-driven cases, `for _, r := range resources`, the `agentConfigs` parametrization), include every member of that set that supports the behavior under test. If a member is deliberately omitted, leave an inline comment naming the gate (e.g., `// task and taskspawner have no create subcommand`). Reviewers should not have to ask "why isn't X here?". - Keep CRD enum docstrings consistent with the `+kubebuilder:validation:Enum` marker: if the godoc says "empty matches both", either include `""` in the enum list so `field: ""` is accepted, or rephrase to "Omit to match both" so no one writes the explicit empty form. A docstring that invites a value the API server then rejects is a worse contract than either alternative. - Qualify cross-CRD field references with the owning kind in docs: in a CRD reference section, write `Task.spec.podOverrides.env` rather than bare `podOverrides.env` when describing a field that lives on a sibling CRD. A reader of one CRD's reference page should be able to locate the cited field without already knowing the layout of the others. - PRs that only modify files under `self-development/` are internal agent improvements: use `/kind cleanup` and write "NONE" in the `release-note` block, even when the change fixes a bug or adds a feature in agent behavior. Classify by file location, not by problem nature.