diff --git a/AGENTS.md b/AGENTS.md index a830e2de1..9d9b498b0 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. +- **Assert byte-equality, not `Contains`, in stream-forwarding tests.** When a function both forwards bytes from an input to an output and parses metadata along the way (a `StreamUsage`-style tee/parse loop), assert that the full captured output equals the normalized expected bytes (`if out.String() != want`), not `strings.Contains(out, line+"\n")` per input line. A per-line substring check still passes when the forwarder drops a partial line, reorders writes, or duplicates output — exactly the regressions a streaming refactor risks. If the same package already has a stricter byte-equality test (e.g., for unterminated input), reuse its shape across the table-driven cases instead of weakening to `Contains`. - **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..fa9fb8e8b 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. + - Assert byte-equality, not `Contains`, in stream-forwarding tests: when a function both forwards bytes from an input to an output and parses metadata along the way (a `StreamUsage`-style tee/parse loop), assert `out.String() == want` against the normalized expected bytes, not `strings.Contains(out, line+"\n")` per input line. A per-line substring check still passes when the forwarder drops a partial line, reorders writes, or duplicates output — exactly the regressions a streaming refactor risks. If the same package already has a stricter byte-equality test (e.g., for unterminated input), reuse its shape across the table-driven cases instead of weakening to `Contains`. - 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 ec743f5f6..8f955f00a 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. + - Assert byte-equality, not `Contains`, in stream-forwarding tests: when a function both forwards bytes from an input to an output and parses metadata along the way (a `StreamUsage`-style tee/parse loop), assert `out.String() == want` against the normalized expected bytes, not `strings.Contains(out, line+"\n")` per input line. A per-line substring check still passes when the forwarder drops a partial line, reorders writes, or duplicates output — exactly the regressions a streaming refactor risks. If the same package already has a stricter byte-equality test (e.g., for unterminated input), reuse its shape across the table-driven cases instead of weakening to `Contains`. - 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.