From f92fbebe048d1f0c3dee1cf5cbbda27e8a0e4373 Mon Sep 17 00:00:00 2001 From: Gunju Kim Date: Sat, 30 May 2026 18:06:20 +0000 Subject: [PATCH 01/21] self-development: broaden substring-assertion rule to forwarding tests The existing "vacuous substring assertions" convention was scoped only to printer/formatter tests, leaving stream/forwarding tests uncovered. In #1189 both the Kelos reviewer and cubic independently flagged a forwarder test that used strings.Contains per line, which passes even when lines are dropped, duplicated, or reordered. Extend the rule to require full byte-equality comparison for writer/pipe forwarding tests, mirroring the assertForwarded fix landed in that PR. Co-Authored-By: Claude Opus 4.8 --- AGENTS.md | 2 +- self-development/agentconfig.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 63a4cb11c..8c5f274a4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -19,7 +19,7 @@ - **Do not use Gomega's global `Expect()` inside `Eventually` polling blocks.** Gomega does not retry on `Expect` failures — a transient API error short-circuits the poller and fails the test on the first blip instead of retrying. In e2e `WaitFor*` helpers, either inline the call and return a zero-value on error, or use the `Eventually(func(g Gomega) { ... })` form so failed assertions are caught and retried. - **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. +- **Avoid vacuous substring assertions in printer/formatter and stream-forwarding 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. When asserting that a writer or pipe forwards its input, compare the full normalized output for byte equality, not `strings.Contains` per line — a per-line substring check still passes when lines are dropped, duplicated, or reordered. - **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 6f93c08c2..6aab66b6d 100644 --- a/self-development/agentconfig.yaml +++ b/self-development/agentconfig.yaml @@ -40,7 +40,7 @@ spec: - Do not use Gomega's global `Expect()` inside `Eventually` polling blocks: Gomega does not retry on `Expect` failures, so a transient API error short-circuits the poller. In e2e `WaitFor*` helpers, either inline the call and return a zero-value on error, or use the `Eventually(func(g Gomega) { ... })` form so failed assertions are caught and retried. - 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. + - Avoid vacuous substring assertions in printer/formatter and stream-forwarding 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. When asserting that a writer or pipe forwards its input, compare the full normalized output for byte equality, not `strings.Contains` per line — a substring check still passes when lines are dropped, duplicated, or reordered. - 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. From e144db37e96abc038d8d06807580af7dadd437c4 Mon Sep 17 00:00:00 2001 From: Gunju Kim Date: Sun, 31 May 2026 18:03:29 +0000 Subject: [PATCH 02/21] self-development: require author/subject filters on autonomous-run triggers PR #1241 (Add kanon-development TaskSpawners) shipped event-driven triggers without author scoping. The Kelos/cubic review flagged this twice and both fixes were applied in that PR: - P1 (confidence 9) on kanon-pr-responder.yaml: add an `author` filter to the `pull_request_review` trigger; without it, unapproved review comments can trigger autonomous task runs. - P2 on kanon-reviewer.yaml: the `issue_comment` trigger was not scoped to pull requests, so `/kelos review` on a regular issue could start the reviewer and make the PR-only command fail. Add a TaskSpawner convention so future spawners ship the `author` allowlist and `commentOn: PullRequest` scoping up front instead of being corrected in review. Existing conventions only said issue_comment fires for both subjects; they did not state the baseline that every autonomous-run trigger must be restricted to a trusted author and the intended subject. Co-Authored-By: Claude Opus 4.8 --- self-development/agentconfig.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/self-development/agentconfig.yaml b/self-development/agentconfig.yaml index 6aab66b6d..0a54640f2 100644 --- a/self-development/agentconfig.yaml +++ b/self-development/agentconfig.yaml @@ -48,4 +48,5 @@ spec: - Prefer webhook-based triggers (`githubWebhook`) over poll-based (`githubPullRequests`) for real-time event-driven tasks - The `{{.Branch}}` template variable is empty for issue-only events; use `{{with index . "Branch"}}{{.}}{{else}}main{{end}}` when it may be empty - The `issue_comment` webhook event fires for both issues and pull requests; design prompts to detect and handle both contexts + - Restrict every trigger that can launch an autonomous run to a trusted author and the intended subject: give `issue_comment` and `pull_request_review` filters an `author` allowlist (e.g. `author: gjkim42`) so an unapproved comment or review cannot start a task and spend tokens, and add `commentOn: PullRequest` to filters whose command is PR-only (e.g. `/kelos review`) so the same command on a plain issue does not start a PR-only task. An unfiltered event trigger is reachable by anyone who can comment - Do not include manual PR branch checkout instructions in prompts — Kelos already checks out the PR branch automatically From 9b1b5438bd550be0aa539473059175a26807122d Mon Sep 17 00:00:00 2001 From: Gunju Kim Date: Mon, 1 Jun 2026 18:04:08 +0000 Subject: [PATCH 03/21] self-development: distinguish "not found" from "lookup failed" in handlers PR #1238 (Add WebhookGateway CRD) shipped request handlers that conflated API lookup errors with empty results. The review flagged this repeatedly and the maintainer fixed every instance: - A TaskSpawner List error was treated as an empty match set, so a transient API failure silently dropped the webhook with a 200; the fix returns the error so the handler responds 5xx and the sender retries. - The gateway Get returned 404 unconditionally; the fix returns 404 only on IsNotFound and 5xx on RBAC/transient errors. Add a coding convention so handlers and reconcilers propagate lookup errors as 5xx (sender redelivers) instead of answering 200 with an empty result. This is distinct from the existing "fail fast on invalid configuration" rule, which covers startup config/secrets rather than request-time API reads. Co-Authored-By: Claude Opus 4.8 --- AGENTS.md | 1 + self-development/agentconfig.yaml | 1 + 2 files changed, 2 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 8c5f274a4..172c6e28f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -10,6 +10,7 @@ - **Kubernetes resource comparison.** Use semantic `.Equal()` or `.Cmp()` methods for `resource.Quantity` comparisons, not `reflect.DeepEqual` — structurally different Quantity values can be semantically identical (e.g., `1000m` vs `1` CPU). - **Never use `os.Getenv()` for secrets as Go `flag` defaults.** Go's `flag` package prints default values in usage/help output, which leaks secret values. Instead, use an empty default and read the env var after `flag.Parse()`. - **Fail fast on invalid configuration.** Do not silently fall back to degraded behavior (e.g., unauthenticated requests) when configuration or credentials are invalid or missing. Return an error or exit immediately instead of returning nil or empty values that mask the failure. +- **Distinguish "not found" from "lookup failed" in request handlers.** In a webhook/HTTP handler or reconciler, a `List`/`Get` that errors is not the same as an empty result. Do not treat a failed lookup as "no match" and answer `200` — that silently drops the event with no redelivery. Propagate the error so the handler returns `5xx` and the sender retries; return `404` only on `IsNotFound`, and `5xx` on RBAC or transient errors. - **Keep API surfaces minimal.** When adding new API fields, types, or CRD changes, include only what is immediately needed. Do not add speculative fields — API is hard to change once shipped. Start with the minimum viable API and extend in follow-up PRs. - **API changes must preserve backward compatibility for existing manifests.** Existing in-cluster resources must continue to apply after a CRD update. Do not change a field's kind (scalar ↔ array, string ↔ object) on an existing field; do not add `MinLength`, `Required`, or other tightening validation to a field that previously accepted absence/empty values; when replacing a field, mark the old one `+deprecated` and keep it functional rather than removing it. When the schema must change, sweep `examples/`, `self-development/`, and any in-tree YAMLs that use the old form and update them in the same PR. - **Maintain API changes only in the latest Kelos API version.** Add new CRD fields, validation, enum values, constants, and user-facing API behavior only to the latest served/storage API version. Older served versions are compatibility surfaces: keep them served, convertible, and backward-compatible, but do not add new capabilities there unless required to preserve existing manifests or implement conversion/migration compatibility. diff --git a/self-development/agentconfig.yaml b/self-development/agentconfig.yaml index 0a54640f2..a71884624 100644 --- a/self-development/agentconfig.yaml +++ b/self-development/agentconfig.yaml @@ -34,6 +34,7 @@ spec: - Kubernetes resource comparison: use semantic `.Equal()` or `.Cmp()` methods for `resource.Quantity` comparisons, not `reflect.DeepEqual` - Never use `os.Getenv()` for secrets as Go `flag` defaults: Go's `flag` package prints default values in usage/help output, which leaks secret values; use an empty default and read the env var after `flag.Parse()` - Fail fast on invalid configuration: do not silently fall back to degraded behavior (e.g., unauthenticated requests) when configuration or credentials are invalid or missing; return an error or exit immediately + - Distinguish "not found" from "lookup failed" in request handlers: in a webhook/HTTP handler or reconciler, a `List`/`Get` that errors is not the same as an empty result; do not treat a failed lookup as "no match" and answer `200` (which silently drops the event with no redelivery), but propagate the error so the handler returns `5xx` and the sender retries — return `404` only on `IsNotFound`, and `5xx` on RBAC or transient errors - Keep API surfaces minimal: when adding new API fields, types, or CRD changes, include only what is immediately needed; do not add speculative fields — API is hard to change once shipped - API changes must preserve backward compatibility for existing manifests: existing in-cluster resources must continue to apply after a CRD update. Do not change a field's kind (scalar ↔ array) on an existing field; do not add `MinLength`/`Required` to a field that previously accepted absence/empty values; replace by deprecating (mark `+deprecated`, keep functional) rather than removing. When the schema must change, sweep `examples/` and `self-development/` for YAMLs using the old form and update them in the same PR. - Docs must match implementation, not aspiration: describe only what the code actually does; do not document unimplemented behavior, overstate guarantees, or describe security checks (e.g., HMAC validation) that aren't enforced. Verify the code enforces a contract before documenting it. From 496f6a77f00515603344522dc83f90d954e2d2b4 Mon Sep 17 00:00:00 2001 From: Gunju Kim Date: Wed, 15 Jul 2026 18:05:54 +0000 Subject: [PATCH 04/21] self-development: honor reduced-motion preferences --- AGENTS.md | 1 + self-development/agentconfig.yaml | 1 + 2 files changed, 2 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 172c6e28f..5645b713f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -17,6 +17,7 @@ - **Import versioned Kelos APIs with aliases.** Prefer aliasing the current versioned API import as `kelos` when a file uses only one API version. Use explicit aliases like `kelosv1alpha1` only when a file genuinely needs multiple versions. - **Docs must match implementation, not aspiration.** When writing or updating docs, READMEs, or comments, describe only what the code actually does. Do not document unimplemented behavior, overstate guarantees, or describe security checks (e.g., HMAC validation) that aren't enforced. Before describing a contract ("X is filtered", "Y is validated"), verify the code enforces it — partial enforcement should be documented as partial. - **Keep `docs/reference.md` user-facing.** Document API fields, configuration, validation, defaults, and observable operational behavior. Do not describe controller or runtime implementation details unless users need them to configure, operate, or troubleshoot Kelos. Prefer outcomes over mechanisms, and remove nonessential internal detail instead of moving it to a separate architecture document. +- **Honor reduced-motion preferences in web UI.** Do not add continuous or non-essential animation without a `prefers-reduced-motion` fallback. Keep the static state distinguishable when animation is disabled; prefer gating animation under `@media (prefers-reduced-motion: no-preference)`. - **Do not use Gomega's global `Expect()` inside `Eventually` polling blocks.** Gomega does not retry on `Expect` failures — a transient API error short-circuits the poller and fails the test on the first blip instead of retrying. In e2e `WaitFor*` helpers, either inline the call and return a zero-value on error, or use the `Eventually(func(g Gomega) { ... })` form so failed assertions are caught and retried. - **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. diff --git a/self-development/agentconfig.yaml b/self-development/agentconfig.yaml index a71884624..eacc75437 100644 --- a/self-development/agentconfig.yaml +++ b/self-development/agentconfig.yaml @@ -38,6 +38,7 @@ spec: - Keep API surfaces minimal: when adding new API fields, types, or CRD changes, include only what is immediately needed; do not add speculative fields — API is hard to change once shipped - API changes must preserve backward compatibility for existing manifests: existing in-cluster resources must continue to apply after a CRD update. Do not change a field's kind (scalar ↔ array) on an existing field; do not add `MinLength`/`Required` to a field that previously accepted absence/empty values; replace by deprecating (mark `+deprecated`, keep functional) rather than removing. When the schema must change, sweep `examples/` and `self-development/` for YAMLs using the old form and update them in the same PR. - Docs must match implementation, not aspiration: describe only what the code actually does; do not document unimplemented behavior, overstate guarantees, or describe security checks (e.g., HMAC validation) that aren't enforced. Verify the code enforces a contract before documenting it. + - Honor reduced-motion preferences in web UI: do not add continuous or non-essential animation without a `prefers-reduced-motion` fallback. Keep the static state distinguishable when animation is disabled; prefer gating animation under `@media (prefers-reduced-motion: no-preference)`. - Do not use Gomega's global `Expect()` inside `Eventually` polling blocks: Gomega does not retry on `Expect` failures, so a transient API error short-circuits the poller. In e2e `WaitFor*` helpers, either inline the call and return a zero-value on error, or use the `Eventually(func(g Gomega) { ... })` form so failed assertions are caught and retried. - 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. From c8f1d4c5c226799ec55ae03f41a83b1a4754cb0e Mon Sep 17 00:00:00 2001 From: Gunju Kim Date: Mon, 27 Jul 2026 18:10:25 +0000 Subject: [PATCH 05/21] self-development: add security, state, and behavior-test conventions --- AGENTS.md | 3 +++ self-development/agentconfig.yaml | 3 +++ 2 files changed, 6 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 5645b713f..1fd343c0e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -10,7 +10,9 @@ - **Kubernetes resource comparison.** Use semantic `.Equal()` or `.Cmp()` methods for `resource.Quantity` comparisons, not `reflect.DeepEqual` — structurally different Quantity values can be semantically identical (e.g., `1000m` vs `1` CPU). - **Never use `os.Getenv()` for secrets as Go `flag` defaults.** Go's `flag` package prints default values in usage/help output, which leaks secret values. Instead, use an empty default and read the env var after `flag.Parse()`. - **Fail fast on invalid configuration.** Do not silently fall back to degraded behavior (e.g., unauthenticated requests) when configuration or credentials are invalid or missing. Return an error or exit immediately instead of returning nil or empty values that mask the failure. +- **Protect credentials in configuration and transport.** Values that may contain tokens, authorization headers, or other credentials must come from Kubernetes Secret references, not plaintext Helm values or rendered manifests. Require HTTPS for configurable endpoints that receive credentials unless the API has an explicit insecure opt-in. - **Distinguish "not found" from "lookup failed" in request handlers.** In a webhook/HTTP handler or reconciler, a `List`/`Get` that errors is not the same as an empty result. Do not treat a failed lookup as "no match" and answer `200` — that silently drops the event with no redelivery. Propagate the error so the handler returns `5xx` and the sender retries; return `404` only on `IsNotFound`, and `5xx` on RBAC or transient errors. +- **Protect asynchronous state transitions from stale work.** Before an asynchronous completion mutates shared state, verify it still belongs to the current resource, Session, dialog, or request generation, or prevent that state from being replaced while the operation is pending. Before a destructive lifecycle transition based on observed status, coordinate or revalidate so newly accepted work cannot race with the transition. - **Keep API surfaces minimal.** When adding new API fields, types, or CRD changes, include only what is immediately needed. Do not add speculative fields — API is hard to change once shipped. Start with the minimum viable API and extend in follow-up PRs. - **API changes must preserve backward compatibility for existing manifests.** Existing in-cluster resources must continue to apply after a CRD update. Do not change a field's kind (scalar ↔ array, string ↔ object) on an existing field; do not add `MinLength`, `Required`, or other tightening validation to a field that previously accepted absence/empty values; when replacing a field, mark the old one `+deprecated` and keep it functional rather than removing it. When the schema must change, sweep `examples/`, `self-development/`, and any in-tree YAMLs that use the old form and update them in the same PR. - **Maintain API changes only in the latest Kelos API version.** Add new CRD fields, validation, enum values, constants, and user-facing API behavior only to the latest served/storage API version. Older served versions are compatibility surfaces: keep them served, convertible, and backward-compatible, but do not add new capabilities there unless required to preserve existing manifests or implement conversion/migration compatibility. @@ -21,6 +23,7 @@ - **Do not use Gomega's global `Expect()` inside `Eventually` polling blocks.** Gomega does not retry on `Expect` failures — a transient API error short-circuits the poller and fails the test on the first blip instead of retrying. In e2e `WaitFor*` helpers, either inline the call and return a zero-value on error, or use the `Eventually(func(g Gomega) { ... })` form so failed assertions are caught and retried. - **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. +- **Test behavior through the layer that owns it.** Do not rely only on helper tests or source-fragment assertions when behavior depends on controller scheduling, rendered configuration, or stateful UI interactions. Exercise the behavior through `Reconcile`, rendered output, or executable UI tests, and cover non-default configuration branches that change the result. - **Avoid vacuous substring assertions in printer/formatter and stream-forwarding 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. When asserting that a writer or pipe forwards its input, compare the full normalized output for byte equality, not `strings.Contains` per line — a per-line substring check still passes when lines are dropped, duplicated, or reordered. - **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 eacc75437..3b08b5374 100644 --- a/self-development/agentconfig.yaml +++ b/self-development/agentconfig.yaml @@ -34,7 +34,9 @@ spec: - Kubernetes resource comparison: use semantic `.Equal()` or `.Cmp()` methods for `resource.Quantity` comparisons, not `reflect.DeepEqual` - Never use `os.Getenv()` for secrets as Go `flag` defaults: Go's `flag` package prints default values in usage/help output, which leaks secret values; use an empty default and read the env var after `flag.Parse()` - Fail fast on invalid configuration: do not silently fall back to degraded behavior (e.g., unauthenticated requests) when configuration or credentials are invalid or missing; return an error or exit immediately + - Protect credentials in configuration and transport: values that may contain tokens, authorization headers, or other credentials must come from Kubernetes Secret references, not plaintext Helm values or rendered manifests. Require HTTPS for configurable endpoints that receive credentials unless the API has an explicit insecure opt-in. - Distinguish "not found" from "lookup failed" in request handlers: in a webhook/HTTP handler or reconciler, a `List`/`Get` that errors is not the same as an empty result; do not treat a failed lookup as "no match" and answer `200` (which silently drops the event with no redelivery), but propagate the error so the handler returns `5xx` and the sender retries — return `404` only on `IsNotFound`, and `5xx` on RBAC or transient errors + - Protect asynchronous state transitions from stale work: before an asynchronous completion mutates shared state, verify it still belongs to the current resource, Session, dialog, or request generation, or prevent that state from being replaced while the operation is pending. Before a destructive lifecycle transition based on observed status, coordinate or revalidate so newly accepted work cannot race with the transition. - Keep API surfaces minimal: when adding new API fields, types, or CRD changes, include only what is immediately needed; do not add speculative fields — API is hard to change once shipped - API changes must preserve backward compatibility for existing manifests: existing in-cluster resources must continue to apply after a CRD update. Do not change a field's kind (scalar ↔ array) on an existing field; do not add `MinLength`/`Required` to a field that previously accepted absence/empty values; replace by deprecating (mark `+deprecated`, keep functional) rather than removing. When the schema must change, sweep `examples/` and `self-development/` for YAMLs using the old form and update them in the same PR. - Docs must match implementation, not aspiration: describe only what the code actually does; do not document unimplemented behavior, overstate guarantees, or describe security checks (e.g., HMAC validation) that aren't enforced. Verify the code enforces a contract before documenting it. @@ -42,6 +44,7 @@ spec: - Do not use Gomega's global `Expect()` inside `Eventually` polling blocks: Gomega does not retry on `Expect` failures, so a transient API error short-circuits the poller. In e2e `WaitFor*` helpers, either inline the call and return a zero-value on error, or use the `Eventually(func(g Gomega) { ... })` form so failed assertions are caught and retried. - 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. + - Test behavior through the layer that owns it: do not rely only on helper tests or source-fragment assertions when behavior depends on controller scheduling, rendered configuration, or stateful UI interactions. Exercise the behavior through `Reconcile`, rendered output, or executable UI tests, and cover non-default configuration branches that change the result. - Avoid vacuous substring assertions in printer/formatter and stream-forwarding 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. When asserting that a writer or pipe forwards its input, compare the full normalized output for byte equality, not `strings.Contains` per line — a substring check still passes when lines are dropped, duplicated, or reordered. - 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. From 00c274910a6a8760bdcdcb393aa35b20b977e5e5 Mon Sep 17 00:00:00 2001 From: Gunju Kim Date: Sat, 8 Aug 2026 18:12:11 +0000 Subject: [PATCH 06/21] self-development: add replay and documentation conventions --- AGENTS.md | 2 ++ self-development/agentconfig.yaml | 2 ++ 2 files changed, 4 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 1fd343c0e..b2cdf87c3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -18,12 +18,14 @@ - **Maintain API changes only in the latest Kelos API version.** Add new CRD fields, validation, enum values, constants, and user-facing API behavior only to the latest served/storage API version. Older served versions are compatibility surfaces: keep them served, convertible, and backward-compatible, but do not add new capabilities there unless required to preserve existing manifests or implement conversion/migration compatibility. - **Import versioned Kelos APIs with aliases.** Prefer aliasing the current versioned API import as `kelos` when a file uses only one API version. Use explicit aliases like `kelosv1alpha1` only when a file genuinely needs multiple versions. - **Docs must match implementation, not aspiration.** When writing or updating docs, READMEs, or comments, describe only what the code actually does. Do not document unimplemented behavior, overstate guarantees, or describe security checks (e.g., HMAC validation) that aren't enforced. Before describing a contract ("X is filtered", "Y is validated"), verify the code enforces it — partial enforcement should be documented as partial. +- **Keep documentation complete and internally consistent.** When changing documented behavior, search for every related overview, per-component section, trigger table, setup example, and command/flag reference, and update all affected mentions together. Do not remove documentation for still-supported behavior when replacing a nearby section. - **Keep `docs/reference.md` user-facing.** Document API fields, configuration, validation, defaults, and observable operational behavior. Do not describe controller or runtime implementation details unless users need them to configure, operate, or troubleshoot Kelos. Prefer outcomes over mechanisms, and remove nonessential internal detail instead of moving it to a separate architecture document. - **Honor reduced-motion preferences in web UI.** Do not add continuous or non-essential animation without a `prefers-reduced-motion` fallback. Keep the static state distinguishable when animation is disabled; prefer gating animation under `@media (prefers-reduced-motion: no-preference)`. - **Do not use Gomega's global `Expect()` inside `Eventually` polling blocks.** Gomega does not retry on `Expect` failures — a transient API error short-circuits the poller and fails the test on the first blip instead of retrying. In e2e `WaitFor*` helpers, either inline the call and return a zero-value on error, or use the `Eventually(func(g Gomega) { ... })` form so failed assertions are caught and retried. - **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. - **Test behavior through the layer that owns it.** Do not rely only on helper tests or source-fragment assertions when behavior depends on controller scheduling, rendered configuration, or stateful UI interactions. Exercise the behavior through `Reconcile`, rendered output, or executable UI tests, and cover non-default configuration branches that change the result. +- **Separate historical replay from live event handling.** When a UI or CLI paginates or replays retained events while receiving live events, keep replay rendering and state separate from the live stream and its live-only side effects. Preserve cursors, in-flight request state, and independently delivered current state. Test page loading during an active stream, duplicate requests, reconnects, live-only notifications, and viewport and ordering preservation. - **Avoid vacuous substring assertions in printer/formatter and stream-forwarding 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. When asserting that a writer or pipe forwards its input, compare the full normalized output for byte equality, not `strings.Contains` per line — a per-line substring check still passes when lines are dropped, duplicated, or reordered. - **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 3b08b5374..1e4e07f5a 100644 --- a/self-development/agentconfig.yaml +++ b/self-development/agentconfig.yaml @@ -40,11 +40,13 @@ spec: - Keep API surfaces minimal: when adding new API fields, types, or CRD changes, include only what is immediately needed; do not add speculative fields — API is hard to change once shipped - API changes must preserve backward compatibility for existing manifests: existing in-cluster resources must continue to apply after a CRD update. Do not change a field's kind (scalar ↔ array) on an existing field; do not add `MinLength`/`Required` to a field that previously accepted absence/empty values; replace by deprecating (mark `+deprecated`, keep functional) rather than removing. When the schema must change, sweep `examples/` and `self-development/` for YAMLs using the old form and update them in the same PR. - Docs must match implementation, not aspiration: describe only what the code actually does; do not document unimplemented behavior, overstate guarantees, or describe security checks (e.g., HMAC validation) that aren't enforced. Verify the code enforces a contract before documenting it. + - Keep documentation complete and internally consistent: when changing documented behavior, search for every related overview, per-component section, trigger table, setup example, and command/flag reference, and update all affected mentions together. Do not remove documentation for still-supported behavior when replacing a nearby section. - Honor reduced-motion preferences in web UI: do not add continuous or non-essential animation without a `prefers-reduced-motion` fallback. Keep the static state distinguishable when animation is disabled; prefer gating animation under `@media (prefers-reduced-motion: no-preference)`. - Do not use Gomega's global `Expect()` inside `Eventually` polling blocks: Gomega does not retry on `Expect` failures, so a transient API error short-circuits the poller. In e2e `WaitFor*` helpers, either inline the call and return a zero-value on error, or use the `Eventually(func(g Gomega) { ... })` form so failed assertions are caught and retried. - 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. - Test behavior through the layer that owns it: do not rely only on helper tests or source-fragment assertions when behavior depends on controller scheduling, rendered configuration, or stateful UI interactions. Exercise the behavior through `Reconcile`, rendered output, or executable UI tests, and cover non-default configuration branches that change the result. + - Separate historical replay from live event handling: when a UI or CLI paginates or replays retained events while receiving live events, keep replay rendering and state separate from the live stream and its live-only side effects. Preserve cursors, in-flight request state, and independently delivered current state. Test page loading during an active stream, duplicate requests, reconnects, live-only notifications, and viewport and ordering preservation. - Avoid vacuous substring assertions in printer/formatter and stream-forwarding 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. When asserting that a writer or pipe forwards its input, compare the full normalized output for byte equality, not `strings.Contains` per line — a substring check still passes when lines are dropped, duplicated, or reordered. - 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. From 24234085f595a6afece7204b9abb5476937ccd47 Mon Sep 17 00:00:00 2001 From: Gunju Kim Date: Mon, 10 Aug 2026 18:10:48 +0000 Subject: [PATCH 07/21] self-development: add identity and client-contract conventions PR #1629 reviews showed that bounding a controller-owned StatefulSet name without accounting for ControllerRevision suffixes could either remain invalid or break workload and PVC identity. PR #1630 reviews found inconsistent attachment limits and empty-input behavior across web, TUI, and plain terminal clients, plus server failures classified as client errors by matching message text. Encode the durable-name, shared-client-contract, and typed error-classification expectations in the project and shared Kelos agent instructions. --- AGENTS.md | 4 +++- self-development/agentconfig.yaml | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index b2cdf87c3..08beb05ed 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -11,10 +11,11 @@ - **Never use `os.Getenv()` for secrets as Go `flag` defaults.** Go's `flag` package prints default values in usage/help output, which leaks secret values. Instead, use an empty default and read the env var after `flag.Parse()`. - **Fail fast on invalid configuration.** Do not silently fall back to degraded behavior (e.g., unauthenticated requests) when configuration or credentials are invalid or missing. Return an error or exit immediately instead of returning nil or empty values that mask the failure. - **Protect credentials in configuration and transport.** Values that may contain tokens, authorization headers, or other credentials must come from Kubernetes Secret references, not plaintext Helm values or rendered manifests. Require HTTPS for configurable endpoints that receive credentials unless the API has an explicit insecure opt-in. -- **Distinguish "not found" from "lookup failed" in request handlers.** In a webhook/HTTP handler or reconciler, a `List`/`Get` that errors is not the same as an empty result. Do not treat a failed lookup as "no match" and answer `200` — that silently drops the event with no redelivery. Propagate the error so the handler returns `5xx` and the sender retries; return `404` only on `IsNotFound`, and `5xx` on RBAC or transient errors. +- **Classify request failures by cause.** When mapping errors to HTTP status, reserve `4xx` for client-correctable input and confirmed not-found cases; return `5xx` for lookup, RBAC, storage, and transport failures. A `List`/`Get` error is not an empty result — propagate it so webhook senders can retry. Use typed or sentinel errors such as `errors.Is` and `apierrors.IsNotFound` instead of matching message text, because wrapped server failures must not become client errors. - **Protect asynchronous state transitions from stale work.** Before an asynchronous completion mutates shared state, verify it still belongs to the current resource, Session, dialog, or request generation, or prevent that state from being replaced while the operation is pending. Before a destructive lifecycle transition based on observed status, coordinate or revalidate so newly accepted work cannot race with the transition. - **Keep API surfaces minimal.** When adding new API fields, types, or CRD changes, include only what is immediately needed. Do not add speculative fields — API is hard to change once shipped. Start with the minimum viable API and extend in follow-up PRs. - **API changes must preserve backward compatibility for existing manifests.** Existing in-cluster resources must continue to apply after a CRD update. Do not change a field's kind (scalar ↔ array, string ↔ object) on an existing field; do not add `MinLength`, `Required`, or other tightening validation to a field that previously accepted absence/empty values; when replacing a field, mark the old one `+deprecated` and keep it functional rather than removing it. When the schema must change, sweep `examples/`, `self-development/`, and any in-tree YAMLs that use the old form and update them in the same PR. +- **Treat controller-generated Kubernetes names as durable identities.** Account for every downstream suffix and label limit when deriving names, including Pod, PVC, and ControllerRevision names, rather than checking only the 63-character resource-name limit. Do not rename existing controller-owned resources just to satisfy a bound unless the change includes a collision-safe migration that preserves ownership and persistent data. - **Maintain API changes only in the latest Kelos API version.** Add new CRD fields, validation, enum values, constants, and user-facing API behavior only to the latest served/storage API version. Older served versions are compatibility surfaces: keep them served, convertible, and backward-compatible, but do not add new capabilities there unless required to preserve existing manifests or implement conversion/migration compatibility. - **Import versioned Kelos APIs with aliases.** Prefer aliasing the current versioned API import as `kelos` when a file uses only one API version. Use explicit aliases like `kelosv1alpha1` only when a file genuinely needs multiple versions. - **Docs must match implementation, not aspiration.** When writing or updating docs, READMEs, or comments, describe only what the code actually does. Do not document unimplemented behavior, overstate guarantees, or describe security checks (e.g., HMAC validation) that aren't enforced. Before describing a contract ("X is filtered", "Y is validated"), verify the code enforces it — partial enforcement should be documented as partial. @@ -25,6 +26,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. - **Test behavior through the layer that owns it.** Do not rely only on helper tests or source-fragment assertions when behavior depends on controller scheduling, rendered configuration, or stateful UI interactions. Exercise the behavior through `Reconcile`, rendered output, or executable UI tests, and cover non-default configuration branches that change the result. +- **Keep shared feature contracts consistent across clients.** When the web UI, TUI, and plain CLI expose the same feature, apply the same limits, empty-input rules, failure semantics, and discoverability. Test each surface's contract instead of assuming shared backend validation makes the clients equivalent. - **Separate historical replay from live event handling.** When a UI or CLI paginates or replays retained events while receiving live events, keep replay rendering and state separate from the live stream and its live-only side effects. Preserve cursors, in-flight request state, and independently delivered current state. Test page loading during an active stream, duplicate requests, reconnects, live-only notifications, and viewport and ordering preservation. - **Avoid vacuous substring assertions in printer/formatter and stream-forwarding 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. When asserting that a writer or pipe forwards its input, compare the full normalized output for byte equality, not `strings.Contains` per line — a per-line substring check still passes when lines are dropped, duplicated, or reordered. - **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. diff --git a/self-development/agentconfig.yaml b/self-development/agentconfig.yaml index 1e4e07f5a..58c6fed99 100644 --- a/self-development/agentconfig.yaml +++ b/self-development/agentconfig.yaml @@ -35,10 +35,11 @@ spec: - Never use `os.Getenv()` for secrets as Go `flag` defaults: Go's `flag` package prints default values in usage/help output, which leaks secret values; use an empty default and read the env var after `flag.Parse()` - Fail fast on invalid configuration: do not silently fall back to degraded behavior (e.g., unauthenticated requests) when configuration or credentials are invalid or missing; return an error or exit immediately - Protect credentials in configuration and transport: values that may contain tokens, authorization headers, or other credentials must come from Kubernetes Secret references, not plaintext Helm values or rendered manifests. Require HTTPS for configurable endpoints that receive credentials unless the API has an explicit insecure opt-in. - - Distinguish "not found" from "lookup failed" in request handlers: in a webhook/HTTP handler or reconciler, a `List`/`Get` that errors is not the same as an empty result; do not treat a failed lookup as "no match" and answer `200` (which silently drops the event with no redelivery), but propagate the error so the handler returns `5xx` and the sender retries — return `404` only on `IsNotFound`, and `5xx` on RBAC or transient errors + - Classify request failures by cause: when mapping errors to HTTP status, reserve `4xx` for client-correctable input and confirmed not-found cases, and return `5xx` for lookup, RBAC, storage, and transport failures. A `List`/`Get` error is not an empty result; propagate it so webhook senders can retry. Use typed or sentinel errors such as `errors.Is` and `apierrors.IsNotFound` instead of matching message text, because wrapped server failures must not become client errors. - Protect asynchronous state transitions from stale work: before an asynchronous completion mutates shared state, verify it still belongs to the current resource, Session, dialog, or request generation, or prevent that state from being replaced while the operation is pending. Before a destructive lifecycle transition based on observed status, coordinate or revalidate so newly accepted work cannot race with the transition. - Keep API surfaces minimal: when adding new API fields, types, or CRD changes, include only what is immediately needed; do not add speculative fields — API is hard to change once shipped - API changes must preserve backward compatibility for existing manifests: existing in-cluster resources must continue to apply after a CRD update. Do not change a field's kind (scalar ↔ array) on an existing field; do not add `MinLength`/`Required` to a field that previously accepted absence/empty values; replace by deprecating (mark `+deprecated`, keep functional) rather than removing. When the schema must change, sweep `examples/` and `self-development/` for YAMLs using the old form and update them in the same PR. + - Treat controller-generated Kubernetes names as durable identities: account for every downstream suffix and label limit when deriving names, including Pod, PVC, and ControllerRevision names, rather than checking only the 63-character resource-name limit. Do not rename existing controller-owned resources just to satisfy a bound unless the change includes a collision-safe migration that preserves ownership and persistent data. - Docs must match implementation, not aspiration: describe only what the code actually does; do not document unimplemented behavior, overstate guarantees, or describe security checks (e.g., HMAC validation) that aren't enforced. Verify the code enforces a contract before documenting it. - Keep documentation complete and internally consistent: when changing documented behavior, search for every related overview, per-component section, trigger table, setup example, and command/flag reference, and update all affected mentions together. Do not remove documentation for still-supported behavior when replacing a nearby section. - Honor reduced-motion preferences in web UI: do not add continuous or non-essential animation without a `prefers-reduced-motion` fallback. Keep the static state distinguishable when animation is disabled; prefer gating animation under `@media (prefers-reduced-motion: no-preference)`. @@ -46,6 +47,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. - Test behavior through the layer that owns it: do not rely only on helper tests or source-fragment assertions when behavior depends on controller scheduling, rendered configuration, or stateful UI interactions. Exercise the behavior through `Reconcile`, rendered output, or executable UI tests, and cover non-default configuration branches that change the result. + - Keep shared feature contracts consistent across clients: when the web UI, TUI, and plain CLI expose the same feature, apply the same limits, empty-input rules, failure semantics, and discoverability. Test each surface's contract instead of assuming shared backend validation makes the clients equivalent. - Separate historical replay from live event handling: when a UI or CLI paginates or replays retained events while receiving live events, keep replay rendering and state separate from the live stream and its live-only side effects. Preserve cursors, in-flight request state, and independently delivered current state. Test page loading during an active stream, duplicate requests, reconnects, live-only notifications, and viewport and ordering preservation. - Avoid vacuous substring assertions in printer/formatter and stream-forwarding 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. When asserting that a writer or pipe forwards its input, compare the full normalized output for byte equality, not `strings.Contains` per line — a substring check still passes when lines are dropped, duplicated, or reordered. - 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. From e9833eeb0b29a29588c56eb19b3f7cf7cfcb1c14 Mon Sep 17 00:00:00 2001 From: Gunju Kim Date: Tue, 11 Aug 2026 18:08:40 +0000 Subject: [PATCH 08/21] self-development: preserve prompt and name contracts --- AGENTS.md | 3 ++- self-development/agentconfig.yaml | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 08beb05ed..7f156b9c9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -15,11 +15,12 @@ - **Protect asynchronous state transitions from stale work.** Before an asynchronous completion mutates shared state, verify it still belongs to the current resource, Session, dialog, or request generation, or prevent that state from being replaced while the operation is pending. Before a destructive lifecycle transition based on observed status, coordinate or revalidate so newly accepted work cannot race with the transition. - **Keep API surfaces minimal.** When adding new API fields, types, or CRD changes, include only what is immediately needed. Do not add speculative fields — API is hard to change once shipped. Start with the minimum viable API and extend in follow-up PRs. - **API changes must preserve backward compatibility for existing manifests.** Existing in-cluster resources must continue to apply after a CRD update. Do not change a field's kind (scalar ↔ array, string ↔ object) on an existing field; do not add `MinLength`, `Required`, or other tightening validation to a field that previously accepted absence/empty values; when replacing a field, mark the old one `+deprecated` and keep it functional rather than removing it. When the schema must change, sweep `examples/`, `self-development/`, and any in-tree YAMLs that use the old form and update them in the same PR. -- **Treat controller-generated Kubernetes names as durable identities.** Account for every downstream suffix and label limit when deriving names, including Pod, PVC, and ControllerRevision names, rather than checking only the 63-character resource-name limit. Do not rename existing controller-owned resources just to satisfy a bound unless the change includes a collision-safe migration that preserves ownership and persistent data. +- **Treat controller-generated Kubernetes names as durable identities.** Validate derived names against every generated resource kind's exact name grammar, then account for every downstream suffix and label limit, including Pod, PVC, and ControllerRevision names, rather than checking only the source resource or the 63-character resource-name limit. Do not rename existing controller-owned resources just to satisfy a bound unless the change includes a collision-safe migration that preserves ownership and persistent data. - **Maintain API changes only in the latest Kelos API version.** Add new CRD fields, validation, enum values, constants, and user-facing API behavior only to the latest served/storage API version. Older served versions are compatibility surfaces: keep them served, convertible, and backward-compatible, but do not add new capabilities there unless required to preserve existing manifests or implement conversion/migration compatibility. - **Import versioned Kelos APIs with aliases.** Prefer aliasing the current versioned API import as `kelos` when a file uses only one API version. Use explicit aliases like `kelosv1alpha1` only when a file genuinely needs multiple versions. - **Docs must match implementation, not aspiration.** When writing or updating docs, READMEs, or comments, describe only what the code actually does. Do not document unimplemented behavior, overstate guarantees, or describe security checks (e.g., HMAC validation) that aren't enforced. Before describing a contract ("X is filtered", "Y is validated"), verify the code enforces it — partial enforcement should be documented as partial. - **Keep documentation complete and internally consistent.** When changing documented behavior, search for every related overview, per-component section, trigger table, setup example, and command/flag reference, and update all affected mentions together. Do not remove documentation for still-supported behavior when replacing a nearby section. +- **Preserve prompt contracts when consolidating agent instructions.** Before removing or shortening a TaskSpawner or SessionSpawner prompt because behavior is shared, map every removed requirement to a concrete instruction source that the spawned agent actually loads, such as a referenced AgentConfig, repository `AGENTS.md`, or skill. Retain requirements with no loaded owner, and update prompt-contract tests and every README section that claims the behavior in the same change. - **Keep `docs/reference.md` user-facing.** Document API fields, configuration, validation, defaults, and observable operational behavior. Do not describe controller or runtime implementation details unless users need them to configure, operate, or troubleshoot Kelos. Prefer outcomes over mechanisms, and remove nonessential internal detail instead of moving it to a separate architecture document. - **Honor reduced-motion preferences in web UI.** Do not add continuous or non-essential animation without a `prefers-reduced-motion` fallback. Keep the static state distinguishable when animation is disabled; prefer gating animation under `@media (prefers-reduced-motion: no-preference)`. - **Do not use Gomega's global `Expect()` inside `Eventually` polling blocks.** Gomega does not retry on `Expect` failures — a transient API error short-circuits the poller and fails the test on the first blip instead of retrying. In e2e `WaitFor*` helpers, either inline the call and return a zero-value on error, or use the `Eventually(func(g Gomega) { ... })` form so failed assertions are caught and retried. diff --git a/self-development/agentconfig.yaml b/self-development/agentconfig.yaml index 58c6fed99..2d979d0ae 100644 --- a/self-development/agentconfig.yaml +++ b/self-development/agentconfig.yaml @@ -39,9 +39,10 @@ spec: - Protect asynchronous state transitions from stale work: before an asynchronous completion mutates shared state, verify it still belongs to the current resource, Session, dialog, or request generation, or prevent that state from being replaced while the operation is pending. Before a destructive lifecycle transition based on observed status, coordinate or revalidate so newly accepted work cannot race with the transition. - Keep API surfaces minimal: when adding new API fields, types, or CRD changes, include only what is immediately needed; do not add speculative fields — API is hard to change once shipped - API changes must preserve backward compatibility for existing manifests: existing in-cluster resources must continue to apply after a CRD update. Do not change a field's kind (scalar ↔ array) on an existing field; do not add `MinLength`/`Required` to a field that previously accepted absence/empty values; replace by deprecating (mark `+deprecated`, keep functional) rather than removing. When the schema must change, sweep `examples/` and `self-development/` for YAMLs using the old form and update them in the same PR. - - Treat controller-generated Kubernetes names as durable identities: account for every downstream suffix and label limit when deriving names, including Pod, PVC, and ControllerRevision names, rather than checking only the 63-character resource-name limit. Do not rename existing controller-owned resources just to satisfy a bound unless the change includes a collision-safe migration that preserves ownership and persistent data. + - Treat controller-generated Kubernetes names as durable identities: validate derived names against every generated resource kind's exact name grammar, then account for every downstream suffix and label limit, including Pod, PVC, and ControllerRevision names, rather than checking only the source resource or the 63-character resource-name limit. Do not rename existing controller-owned resources just to satisfy a bound unless the change includes a collision-safe migration that preserves ownership and persistent data. - Docs must match implementation, not aspiration: describe only what the code actually does; do not document unimplemented behavior, overstate guarantees, or describe security checks (e.g., HMAC validation) that aren't enforced. Verify the code enforces a contract before documenting it. - Keep documentation complete and internally consistent: when changing documented behavior, search for every related overview, per-component section, trigger table, setup example, and command/flag reference, and update all affected mentions together. Do not remove documentation for still-supported behavior when replacing a nearby section. + - Preserve prompt contracts when consolidating agent instructions: before removing or shortening a TaskSpawner or SessionSpawner prompt because behavior is shared, map every removed requirement to a concrete instruction source that the spawned agent actually loads, such as a referenced AgentConfig, repository `AGENTS.md`, or skill. Retain requirements with no loaded owner, and update prompt-contract tests and every README section that claims the behavior in the same change. - Honor reduced-motion preferences in web UI: do not add continuous or non-essential animation without a `prefers-reduced-motion` fallback. Keep the static state distinguishable when animation is disabled; prefer gating animation under `@media (prefers-reduced-motion: no-preference)`. - Do not use Gomega's global `Expect()` inside `Eventually` polling blocks: Gomega does not retry on `Expect` failures, so a transient API error short-circuits the poller. In e2e `WaitFor*` helpers, either inline the call and return a zero-value on error, or use the `Eventually(func(g Gomega) { ... })` form so failed assertions are caught and retried. - 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")`. From ef9644d05a97cbf174662f92042937d15f8abeac Mon Sep 17 00:00:00 2001 From: Gunju Kim Date: Wed, 12 Aug 2026 18:11:23 +0000 Subject: [PATCH 09/21] self-development: refine review-derived agent rules --- AGENTS.md | 2 ++ self-development/agentconfig.yaml | 2 ++ 2 files changed, 4 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 7f156b9c9..0263a3f9f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -23,6 +23,7 @@ - **Preserve prompt contracts when consolidating agent instructions.** Before removing or shortening a TaskSpawner or SessionSpawner prompt because behavior is shared, map every removed requirement to a concrete instruction source that the spawned agent actually loads, such as a referenced AgentConfig, repository `AGENTS.md`, or skill. Retain requirements with no loaded owner, and update prompt-contract tests and every README section that claims the behavior in the same change. - **Keep `docs/reference.md` user-facing.** Document API fields, configuration, validation, defaults, and observable operational behavior. Do not describe controller or runtime implementation details unless users need them to configure, operate, or troubleshoot Kelos. Prefer outcomes over mechanisms, and remove nonessential internal detail instead of moving it to a separate architecture document. - **Honor reduced-motion preferences in web UI.** Do not add continuous or non-essential animation without a `prefers-reduced-motion` fallback. Keep the static state distinguishable when animation is disabled; prefer gating animation under `@media (prefers-reduced-motion: no-preference)`. +- **Audit responsive rules when changing web controls.** When adding or renaming a form field or control, include it in every applicable selector group and breakpoint override. In particular, preserve mobile font sizing that prevents focus zoom, touch-target sizing, and alignment with adjacent controls. - **Do not use Gomega's global `Expect()` inside `Eventually` polling blocks.** Gomega does not retry on `Expect` failures — a transient API error short-circuits the poller and fails the test on the first blip instead of retrying. In e2e `WaitFor*` helpers, either inline the call and return a zero-value on error, or use the `Eventually(func(g Gomega) { ... })` form so failed assertions are caught and retried. - **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. @@ -30,6 +31,7 @@ - **Keep shared feature contracts consistent across clients.** When the web UI, TUI, and plain CLI expose the same feature, apply the same limits, empty-input rules, failure semantics, and discoverability. Test each surface's contract instead of assuming shared backend validation makes the clients equivalent. - **Separate historical replay from live event handling.** When a UI or CLI paginates or replays retained events while receiving live events, keep replay rendering and state separate from the live stream and its live-only side effects. Preserve cursors, in-flight request state, and independently delivered current state. Test page loading during an active stream, duplicate requests, reconnects, live-only notifications, and viewport and ordering preservation. - **Avoid vacuous substring assertions in printer/formatter and stream-forwarding 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. When asserting that a writer or pipe forwards its input, compare the full normalized output for byte equality, not `strings.Contains` per line — a per-line substring check still passes when lines are dropped, duplicated, or reordered. +- **Use lease protection after rebasing published branches.** Rebasing rewrites commit IDs, so push an already-published rebased branch with `git push --force-with-lease`, never `--force`. A normal push will be rejected as non-fast-forward, while an unleased force-push can overwrite concurrent remote work. - **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 2d979d0ae..274604257 100644 --- a/self-development/agentconfig.yaml +++ b/self-development/agentconfig.yaml @@ -44,6 +44,7 @@ spec: - Keep documentation complete and internally consistent: when changing documented behavior, search for every related overview, per-component section, trigger table, setup example, and command/flag reference, and update all affected mentions together. Do not remove documentation for still-supported behavior when replacing a nearby section. - Preserve prompt contracts when consolidating agent instructions: before removing or shortening a TaskSpawner or SessionSpawner prompt because behavior is shared, map every removed requirement to a concrete instruction source that the spawned agent actually loads, such as a referenced AgentConfig, repository `AGENTS.md`, or skill. Retain requirements with no loaded owner, and update prompt-contract tests and every README section that claims the behavior in the same change. - Honor reduced-motion preferences in web UI: do not add continuous or non-essential animation without a `prefers-reduced-motion` fallback. Keep the static state distinguishable when animation is disabled; prefer gating animation under `@media (prefers-reduced-motion: no-preference)`. + - Audit responsive rules when changing web controls: when adding or renaming a form field or control, include it in every applicable selector group and breakpoint override. In particular, preserve mobile font sizing that prevents focus zoom, touch-target sizing, and alignment with adjacent controls. - Do not use Gomega's global `Expect()` inside `Eventually` polling blocks: Gomega does not retry on `Expect` failures, so a transient API error short-circuits the poller. In e2e `WaitFor*` helpers, either inline the call and return a zero-value on error, or use the `Eventually(func(g Gomega) { ... })` form so failed assertions are caught and retried. - 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. @@ -51,6 +52,7 @@ spec: - Keep shared feature contracts consistent across clients: when the web UI, TUI, and plain CLI expose the same feature, apply the same limits, empty-input rules, failure semantics, and discoverability. Test each surface's contract instead of assuming shared backend validation makes the clients equivalent. - Separate historical replay from live event handling: when a UI or CLI paginates or replays retained events while receiving live events, keep replay rendering and state separate from the live stream and its live-only side effects. Preserve cursors, in-flight request state, and independently delivered current state. Test page loading during an active stream, duplicate requests, reconnects, live-only notifications, and viewport and ordering preservation. - Avoid vacuous substring assertions in printer/formatter and stream-forwarding 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. When asserting that a writer or pipe forwards its input, compare the full normalized output for byte equality, not `strings.Contains` per line — a substring check still passes when lines are dropped, duplicated, or reordered. + - Use lease protection after rebasing published branches: rebasing rewrites commit IDs, so push an already-published rebased branch with `git push --force-with-lease`, never `--force`. A normal push will be rejected as non-fast-forward, while an unleased force-push can overwrite concurrent remote work. - 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. From d3692d11d2d7c3618f7b3d3be1157f0cfa4bda1a Mon Sep 17 00:00:00 2001 From: Gunju Kim Date: Thu, 13 Aug 2026 18:09:41 +0000 Subject: [PATCH 10/21] self-development: refine stale-state and assertion guidance --- AGENTS.md | 4 ++-- self-development/agentconfig.yaml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 0263a3f9f..6d80f0037 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -12,7 +12,7 @@ - **Fail fast on invalid configuration.** Do not silently fall back to degraded behavior (e.g., unauthenticated requests) when configuration or credentials are invalid or missing. Return an error or exit immediately instead of returning nil or empty values that mask the failure. - **Protect credentials in configuration and transport.** Values that may contain tokens, authorization headers, or other credentials must come from Kubernetes Secret references, not plaintext Helm values or rendered manifests. Require HTTPS for configurable endpoints that receive credentials unless the API has an explicit insecure opt-in. - **Classify request failures by cause.** When mapping errors to HTTP status, reserve `4xx` for client-correctable input and confirmed not-found cases; return `5xx` for lookup, RBAC, storage, and transport failures. A `List`/`Get` error is not an empty result — propagate it so webhook senders can retry. Use typed or sentinel errors such as `errors.Is` and `apierrors.IsNotFound` instead of matching message text, because wrapped server failures must not become client errors. -- **Protect asynchronous state transitions from stale work.** Before an asynchronous completion mutates shared state, verify it still belongs to the current resource, Session, dialog, or request generation, or prevent that state from being replaced while the operation is pending. Before a destructive lifecycle transition based on observed status, coordinate or revalidate so newly accepted work cannot race with the transition. +- **Protect asynchronous state transitions from stale work.** After asynchronous work completes, re-read current state instead of rendering or restoring a captured pre-request snapshot, and verify the operation still belongs to the current resource, Session, dialog, or request generation before mutating shared state. Derive completion, interruption, and control behavior from the exact active operation's identity and kind, not provider-global state. Coordinate startup, handoff, and destructive lifecycle transitions so stale notifications or newly accepted work cannot act on a different operation. - **Keep API surfaces minimal.** When adding new API fields, types, or CRD changes, include only what is immediately needed. Do not add speculative fields — API is hard to change once shipped. Start with the minimum viable API and extend in follow-up PRs. - **API changes must preserve backward compatibility for existing manifests.** Existing in-cluster resources must continue to apply after a CRD update. Do not change a field's kind (scalar ↔ array, string ↔ object) on an existing field; do not add `MinLength`, `Required`, or other tightening validation to a field that previously accepted absence/empty values; when replacing a field, mark the old one `+deprecated` and keep it functional rather than removing it. When the schema must change, sweep `examples/`, `self-development/`, and any in-tree YAMLs that use the old form and update them in the same PR. - **Treat controller-generated Kubernetes names as durable identities.** Validate derived names against every generated resource kind's exact name grammar, then account for every downstream suffix and label limit, including Pod, PVC, and ControllerRevision names, rather than checking only the source resource or the 63-character resource-name limit. Do not rename existing controller-owned resources just to satisfy a bound unless the change includes a collision-safe migration that preserves ownership and persistent data. @@ -27,7 +27,7 @@ - **Do not use Gomega's global `Expect()` inside `Eventually` polling blocks.** Gomega does not retry on `Expect` failures — a transient API error short-circuits the poller and fails the test on the first blip instead of retrying. In e2e `WaitFor*` helpers, either inline the call and return a zero-value on error, or use the `Eventually(func(g Gomega) { ... })` form so failed assertions are caught and retried. - **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. -- **Test behavior through the layer that owns it.** Do not rely only on helper tests or source-fragment assertions when behavior depends on controller scheduling, rendered configuration, or stateful UI interactions. Exercise the behavior through `Reconcile`, rendered output, or executable UI tests, and cover non-default configuration branches that change the result. +- **Test behavior through the layer that owns it.** Do not rely only on helper tests or source-fragment assertions when behavior depends on controller scheduling, rendered configuration, or stateful UI interactions. Exercise the behavior through `Reconcile`, rendered output, or executable UI tests, and cover non-default configuration branches that change the result. Assert the primary effect directly, such as verifying that a process exited or that exact identifiers reached a fake, instead of relying only on logs, journals, or status that can succeed independently. When static-source assertions are appropriate, such as for CSS rules, assert behavior-relevant declarations and the absence of conflicting declarations instead of matching an exact serialized or minified block. - **Keep shared feature contracts consistent across clients.** When the web UI, TUI, and plain CLI expose the same feature, apply the same limits, empty-input rules, failure semantics, and discoverability. Test each surface's contract instead of assuming shared backend validation makes the clients equivalent. - **Separate historical replay from live event handling.** When a UI or CLI paginates or replays retained events while receiving live events, keep replay rendering and state separate from the live stream and its live-only side effects. Preserve cursors, in-flight request state, and independently delivered current state. Test page loading during an active stream, duplicate requests, reconnects, live-only notifications, and viewport and ordering preservation. - **Avoid vacuous substring assertions in printer/formatter and stream-forwarding 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. When asserting that a writer or pipe forwards its input, compare the full normalized output for byte equality, not `strings.Contains` per line — a per-line substring check still passes when lines are dropped, duplicated, or reordered. diff --git a/self-development/agentconfig.yaml b/self-development/agentconfig.yaml index 274604257..29e6fa023 100644 --- a/self-development/agentconfig.yaml +++ b/self-development/agentconfig.yaml @@ -36,7 +36,7 @@ spec: - Fail fast on invalid configuration: do not silently fall back to degraded behavior (e.g., unauthenticated requests) when configuration or credentials are invalid or missing; return an error or exit immediately - Protect credentials in configuration and transport: values that may contain tokens, authorization headers, or other credentials must come from Kubernetes Secret references, not plaintext Helm values or rendered manifests. Require HTTPS for configurable endpoints that receive credentials unless the API has an explicit insecure opt-in. - Classify request failures by cause: when mapping errors to HTTP status, reserve `4xx` for client-correctable input and confirmed not-found cases, and return `5xx` for lookup, RBAC, storage, and transport failures. A `List`/`Get` error is not an empty result; propagate it so webhook senders can retry. Use typed or sentinel errors such as `errors.Is` and `apierrors.IsNotFound` instead of matching message text, because wrapped server failures must not become client errors. - - Protect asynchronous state transitions from stale work: before an asynchronous completion mutates shared state, verify it still belongs to the current resource, Session, dialog, or request generation, or prevent that state from being replaced while the operation is pending. Before a destructive lifecycle transition based on observed status, coordinate or revalidate so newly accepted work cannot race with the transition. + - Protect asynchronous state transitions from stale work: after asynchronous work completes, re-read current state instead of rendering or restoring a captured pre-request snapshot, and verify the operation still belongs to the current resource, Session, dialog, or request generation before mutating shared state. Derive completion, interruption, and control behavior from the exact active operation's identity and kind, not provider-global state. Coordinate startup, handoff, and destructive lifecycle transitions so stale notifications or newly accepted work cannot act on a different operation. - Keep API surfaces minimal: when adding new API fields, types, or CRD changes, include only what is immediately needed; do not add speculative fields — API is hard to change once shipped - API changes must preserve backward compatibility for existing manifests: existing in-cluster resources must continue to apply after a CRD update. Do not change a field's kind (scalar ↔ array) on an existing field; do not add `MinLength`/`Required` to a field that previously accepted absence/empty values; replace by deprecating (mark `+deprecated`, keep functional) rather than removing. When the schema must change, sweep `examples/` and `self-development/` for YAMLs using the old form and update them in the same PR. - Treat controller-generated Kubernetes names as durable identities: validate derived names against every generated resource kind's exact name grammar, then account for every downstream suffix and label limit, including Pod, PVC, and ControllerRevision names, rather than checking only the source resource or the 63-character resource-name limit. Do not rename existing controller-owned resources just to satisfy a bound unless the change includes a collision-safe migration that preserves ownership and persistent data. @@ -48,7 +48,7 @@ spec: - Do not use Gomega's global `Expect()` inside `Eventually` polling blocks: Gomega does not retry on `Expect` failures, so a transient API error short-circuits the poller. In e2e `WaitFor*` helpers, either inline the call and return a zero-value on error, or use the `Eventually(func(g Gomega) { ... })` form so failed assertions are caught and retried. - 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. - - Test behavior through the layer that owns it: do not rely only on helper tests or source-fragment assertions when behavior depends on controller scheduling, rendered configuration, or stateful UI interactions. Exercise the behavior through `Reconcile`, rendered output, or executable UI tests, and cover non-default configuration branches that change the result. + - Test behavior through the layer that owns it: do not rely only on helper tests or source-fragment assertions when behavior depends on controller scheduling, rendered configuration, or stateful UI interactions. Exercise the behavior through `Reconcile`, rendered output, or executable UI tests, and cover non-default configuration branches that change the result. Assert the primary effect directly, such as verifying that a process exited or that exact identifiers reached a fake, instead of relying only on logs, journals, or status that can succeed independently. When static-source assertions are appropriate, such as for CSS rules, assert behavior-relevant declarations and the absence of conflicting declarations instead of matching an exact serialized or minified block. - Keep shared feature contracts consistent across clients: when the web UI, TUI, and plain CLI expose the same feature, apply the same limits, empty-input rules, failure semantics, and discoverability. Test each surface's contract instead of assuming shared backend validation makes the clients equivalent. - Separate historical replay from live event handling: when a UI or CLI paginates or replays retained events while receiving live events, keep replay rendering and state separate from the live stream and its live-only side effects. Preserve cursors, in-flight request state, and independently delivered current state. Test page loading during an active stream, duplicate requests, reconnects, live-only notifications, and viewport and ordering preservation. - Avoid vacuous substring assertions in printer/formatter and stream-forwarding 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. When asserting that a writer or pipe forwards its input, compare the full normalized output for byte equality, not `strings.Contains` per line — a substring check still passes when lines are dropped, duplicated, or reordered. From 8a1fc1cc057499c34d2e46e1eee454e8aa461a6f Mon Sep 17 00:00:00 2001 From: Gunju Kim Date: Sat, 15 Aug 2026 18:10:41 +0000 Subject: [PATCH 11/21] self-development: require cleanup of test background work --- AGENTS.md | 2 +- self-development/agentconfig.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 6d80f0037..3e190b58e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -27,7 +27,7 @@ - **Do not use Gomega's global `Expect()` inside `Eventually` polling blocks.** Gomega does not retry on `Expect` failures — a transient API error short-circuits the poller and fails the test on the first blip instead of retrying. In e2e `WaitFor*` helpers, either inline the call and return a zero-value on error, or use the `Eventually(func(g Gomega) { ... })` form so failed assertions are caught and retried. - **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. -- **Test behavior through the layer that owns it.** Do not rely only on helper tests or source-fragment assertions when behavior depends on controller scheduling, rendered configuration, or stateful UI interactions. Exercise the behavior through `Reconcile`, rendered output, or executable UI tests, and cover non-default configuration branches that change the result. Assert the primary effect directly, such as verifying that a process exited or that exact identifiers reached a fake, instead of relying only on logs, journals, or status that can succeed independently. When static-source assertions are appropriate, such as for CSS rules, assert behavior-relevant declarations and the absence of conflicting declarations instead of matching an exact serialized or minified block. +- **Test behavior through the layer that owns it.** Do not rely only on helper tests or source-fragment assertions when behavior depends on controller scheduling, rendered configuration, or stateful UI interactions. Exercise the behavior through `Reconcile`, rendered output, or executable UI tests, and cover non-default configuration branches that change the result. Assert the primary effect directly, such as verifying that a process exited or that exact identifiers reached a fake, instead of relying only on logs, journals, or status that can succeed independently. Tests that start goroutines or child processes must make blocking operations cancellation-aware and verify cleanup before returning so background work cannot leak after a passing assertion. When static-source assertions are appropriate, such as for CSS rules, assert behavior-relevant declarations and the absence of conflicting declarations instead of matching an exact serialized or minified block. - **Keep shared feature contracts consistent across clients.** When the web UI, TUI, and plain CLI expose the same feature, apply the same limits, empty-input rules, failure semantics, and discoverability. Test each surface's contract instead of assuming shared backend validation makes the clients equivalent. - **Separate historical replay from live event handling.** When a UI or CLI paginates or replays retained events while receiving live events, keep replay rendering and state separate from the live stream and its live-only side effects. Preserve cursors, in-flight request state, and independently delivered current state. Test page loading during an active stream, duplicate requests, reconnects, live-only notifications, and viewport and ordering preservation. - **Avoid vacuous substring assertions in printer/formatter and stream-forwarding 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. When asserting that a writer or pipe forwards its input, compare the full normalized output for byte equality, not `strings.Contains` per line — a per-line substring check still passes when lines are dropped, duplicated, or reordered. diff --git a/self-development/agentconfig.yaml b/self-development/agentconfig.yaml index 29e6fa023..d83dc1012 100644 --- a/self-development/agentconfig.yaml +++ b/self-development/agentconfig.yaml @@ -48,7 +48,7 @@ spec: - Do not use Gomega's global `Expect()` inside `Eventually` polling blocks: Gomega does not retry on `Expect` failures, so a transient API error short-circuits the poller. In e2e `WaitFor*` helpers, either inline the call and return a zero-value on error, or use the `Eventually(func(g Gomega) { ... })` form so failed assertions are caught and retried. - 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. - - Test behavior through the layer that owns it: do not rely only on helper tests or source-fragment assertions when behavior depends on controller scheduling, rendered configuration, or stateful UI interactions. Exercise the behavior through `Reconcile`, rendered output, or executable UI tests, and cover non-default configuration branches that change the result. Assert the primary effect directly, such as verifying that a process exited or that exact identifiers reached a fake, instead of relying only on logs, journals, or status that can succeed independently. When static-source assertions are appropriate, such as for CSS rules, assert behavior-relevant declarations and the absence of conflicting declarations instead of matching an exact serialized or minified block. + - Test behavior through the layer that owns it: do not rely only on helper tests or source-fragment assertions when behavior depends on controller scheduling, rendered configuration, or stateful UI interactions. Exercise the behavior through `Reconcile`, rendered output, or executable UI tests, and cover non-default configuration branches that change the result. Assert the primary effect directly, such as verifying that a process exited or that exact identifiers reached a fake, instead of relying only on logs, journals, or status that can succeed independently. Tests that start goroutines or child processes must make blocking operations cancellation-aware and verify cleanup before returning so background work cannot leak after a passing assertion. When static-source assertions are appropriate, such as for CSS rules, assert behavior-relevant declarations and the absence of conflicting declarations instead of matching an exact serialized or minified block. - Keep shared feature contracts consistent across clients: when the web UI, TUI, and plain CLI expose the same feature, apply the same limits, empty-input rules, failure semantics, and discoverability. Test each surface's contract instead of assuming shared backend validation makes the clients equivalent. - Separate historical replay from live event handling: when a UI or CLI paginates or replays retained events while receiving live events, keep replay rendering and state separate from the live stream and its live-only side effects. Preserve cursors, in-flight request state, and independently delivered current state. Test page loading during an active stream, duplicate requests, reconnects, live-only notifications, and viewport and ordering preservation. - Avoid vacuous substring assertions in printer/formatter and stream-forwarding 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. When asserting that a writer or pipe forwards its input, compare the full normalized output for byte equality, not `strings.Contains` per line — a substring check still passes when lines are dropped, duplicated, or reordered. From f532787bb096f675d3a533fb49306092f62c6d6c Mon Sep 17 00:00:00 2001 From: Gunju Kim Date: Sun, 16 Aug 2026 18:09:22 +0000 Subject: [PATCH 12/21] self-development: tighten test-double contracts --- AGENTS.md | 2 +- self-development/agentconfig.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 3e190b58e..10664da23 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -27,7 +27,7 @@ - **Do not use Gomega's global `Expect()` inside `Eventually` polling blocks.** Gomega does not retry on `Expect` failures — a transient API error short-circuits the poller and fails the test on the first blip instead of retrying. In e2e `WaitFor*` helpers, either inline the call and return a zero-value on error, or use the `Eventually(func(g Gomega) { ... })` form so failed assertions are caught and retried. - **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. -- **Test behavior through the layer that owns it.** Do not rely only on helper tests or source-fragment assertions when behavior depends on controller scheduling, rendered configuration, or stateful UI interactions. Exercise the behavior through `Reconcile`, rendered output, or executable UI tests, and cover non-default configuration branches that change the result. Assert the primary effect directly, such as verifying that a process exited or that exact identifiers reached a fake, instead of relying only on logs, journals, or status that can succeed independently. Tests that start goroutines or child processes must make blocking operations cancellation-aware and verify cleanup before returning so background work cannot leak after a passing assertion. When static-source assertions are appropriate, such as for CSS rules, assert behavior-relevant declarations and the absence of conflicting declarations instead of matching an exact serialized or minified block. +- **Test behavior through the layer that owns it.** Do not rely only on helper tests or source-fragment assertions when behavior depends on controller scheduling, rendered configuration, or stateful UI interactions. Exercise the behavior through `Reconcile`, rendered output, or executable UI tests, and cover non-default configuration branches that change the result. Assert the primary effect directly, such as verifying that a process exited or that exact identifiers reached a fake, instead of relying only on logs, journals, or status that can succeed independently. Fakes, fixtures, and command shims must exercise changed response fields, validate the complete request shape, reject unexpected calls, and expose call counts when batching or request reduction is part of the contract. Tests that start goroutines or child processes must make blocking operations cancellation-aware and verify cleanup before returning so background work cannot leak after a passing assertion. When static-source assertions are appropriate, such as for CSS rules, assert behavior-relevant declarations and the absence of conflicting declarations instead of matching an exact serialized or minified block. - **Keep shared feature contracts consistent across clients.** When the web UI, TUI, and plain CLI expose the same feature, apply the same limits, empty-input rules, failure semantics, and discoverability. Test each surface's contract instead of assuming shared backend validation makes the clients equivalent. - **Separate historical replay from live event handling.** When a UI or CLI paginates or replays retained events while receiving live events, keep replay rendering and state separate from the live stream and its live-only side effects. Preserve cursors, in-flight request state, and independently delivered current state. Test page loading during an active stream, duplicate requests, reconnects, live-only notifications, and viewport and ordering preservation. - **Avoid vacuous substring assertions in printer/formatter and stream-forwarding 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. When asserting that a writer or pipe forwards its input, compare the full normalized output for byte equality, not `strings.Contains` per line — a per-line substring check still passes when lines are dropped, duplicated, or reordered. diff --git a/self-development/agentconfig.yaml b/self-development/agentconfig.yaml index d83dc1012..9296e27d3 100644 --- a/self-development/agentconfig.yaml +++ b/self-development/agentconfig.yaml @@ -48,7 +48,7 @@ spec: - Do not use Gomega's global `Expect()` inside `Eventually` polling blocks: Gomega does not retry on `Expect` failures, so a transient API error short-circuits the poller. In e2e `WaitFor*` helpers, either inline the call and return a zero-value on error, or use the `Eventually(func(g Gomega) { ... })` form so failed assertions are caught and retried. - 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. - - Test behavior through the layer that owns it: do not rely only on helper tests or source-fragment assertions when behavior depends on controller scheduling, rendered configuration, or stateful UI interactions. Exercise the behavior through `Reconcile`, rendered output, or executable UI tests, and cover non-default configuration branches that change the result. Assert the primary effect directly, such as verifying that a process exited or that exact identifiers reached a fake, instead of relying only on logs, journals, or status that can succeed independently. Tests that start goroutines or child processes must make blocking operations cancellation-aware and verify cleanup before returning so background work cannot leak after a passing assertion. When static-source assertions are appropriate, such as for CSS rules, assert behavior-relevant declarations and the absence of conflicting declarations instead of matching an exact serialized or minified block. + - Test behavior through the layer that owns it: do not rely only on helper tests or source-fragment assertions when behavior depends on controller scheduling, rendered configuration, or stateful UI interactions. Exercise the behavior through `Reconcile`, rendered output, or executable UI tests, and cover non-default configuration branches that change the result. Assert the primary effect directly, such as verifying that a process exited or that exact identifiers reached a fake, instead of relying only on logs, journals, or status that can succeed independently. Fakes, fixtures, and command shims must exercise changed response fields, validate the complete request shape, reject unexpected calls, and expose call counts when batching or request reduction is part of the contract. Tests that start goroutines or child processes must make blocking operations cancellation-aware and verify cleanup before returning so background work cannot leak after a passing assertion. When static-source assertions are appropriate, such as for CSS rules, assert behavior-relevant declarations and the absence of conflicting declarations instead of matching an exact serialized or minified block. - Keep shared feature contracts consistent across clients: when the web UI, TUI, and plain CLI expose the same feature, apply the same limits, empty-input rules, failure semantics, and discoverability. Test each surface's contract instead of assuming shared backend validation makes the clients equivalent. - Separate historical replay from live event handling: when a UI or CLI paginates or replays retained events while receiving live events, keep replay rendering and state separate from the live stream and its live-only side effects. Preserve cursors, in-flight request state, and independently delivered current state. Test page loading during an active stream, duplicate requests, reconnects, live-only notifications, and viewport and ordering preservation. - Avoid vacuous substring assertions in printer/formatter and stream-forwarding 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. When asserting that a writer or pipe forwards its input, compare the full normalized output for byte equality, not `strings.Contains` per line — a substring check still passes when lines are dropped, duplicated, or reordered. From dfb13d1e6f6e1ca6a9586e375800e0897f00b145 Mon Sep 17 00:00:00 2001 From: Gunju Kim Date: Mon, 17 Aug 2026 18:14:27 +0000 Subject: [PATCH 13/21] self-development: cover lifecycle state precedence --- AGENTS.md | 4 ++-- self-development/agentconfig.yaml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 10664da23..7fa849f8c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -12,7 +12,7 @@ - **Fail fast on invalid configuration.** Do not silently fall back to degraded behavior (e.g., unauthenticated requests) when configuration or credentials are invalid or missing. Return an error or exit immediately instead of returning nil or empty values that mask the failure. - **Protect credentials in configuration and transport.** Values that may contain tokens, authorization headers, or other credentials must come from Kubernetes Secret references, not plaintext Helm values or rendered manifests. Require HTTPS for configurable endpoints that receive credentials unless the API has an explicit insecure opt-in. - **Classify request failures by cause.** When mapping errors to HTTP status, reserve `4xx` for client-correctable input and confirmed not-found cases; return `5xx` for lookup, RBAC, storage, and transport failures. A `List`/`Get` error is not an empty result — propagate it so webhook senders can retry. Use typed or sentinel errors such as `errors.Is` and `apierrors.IsNotFound` instead of matching message text, because wrapped server failures must not become client errors. -- **Protect asynchronous state transitions from stale work.** After asynchronous work completes, re-read current state instead of rendering or restoring a captured pre-request snapshot, and verify the operation still belongs to the current resource, Session, dialog, or request generation before mutating shared state. Derive completion, interruption, and control behavior from the exact active operation's identity and kind, not provider-global state. Coordinate startup, handoff, and destructive lifecycle transitions so stale notifications or newly accepted work cannot act on a different operation. +- **Protect state-dependent transitions from stale work.** After asynchronous work completes, re-read current state instead of rendering or restoring a captured pre-request snapshot, and verify the operation still belongs to the current resource, Session, dialog, or request generation before mutating shared state. When lifecycle signals can disagree or lag, inspect terminal and failure evidence before returning a transient or retry decision. Derive completion, interruption, and control behavior from the exact active operation's identity and kind, not provider-global state. Coordinate startup, handoff, and destructive lifecycle transitions so stale notifications or newly accepted work cannot act on a different operation. - **Keep API surfaces minimal.** When adding new API fields, types, or CRD changes, include only what is immediately needed. Do not add speculative fields — API is hard to change once shipped. Start with the minimum viable API and extend in follow-up PRs. - **API changes must preserve backward compatibility for existing manifests.** Existing in-cluster resources must continue to apply after a CRD update. Do not change a field's kind (scalar ↔ array, string ↔ object) on an existing field; do not add `MinLength`, `Required`, or other tightening validation to a field that previously accepted absence/empty values; when replacing a field, mark the old one `+deprecated` and keep it functional rather than removing it. When the schema must change, sweep `examples/`, `self-development/`, and any in-tree YAMLs that use the old form and update them in the same PR. - **Treat controller-generated Kubernetes names as durable identities.** Validate derived names against every generated resource kind's exact name grammar, then account for every downstream suffix and label limit, including Pod, PVC, and ControllerRevision names, rather than checking only the source resource or the 63-character resource-name limit. Do not rename existing controller-owned resources just to satisfy a bound unless the change includes a collision-safe migration that preserves ownership and persistent data. @@ -27,7 +27,7 @@ - **Do not use Gomega's global `Expect()` inside `Eventually` polling blocks.** Gomega does not retry on `Expect` failures — a transient API error short-circuits the poller and fails the test on the first blip instead of retrying. In e2e `WaitFor*` helpers, either inline the call and return a zero-value on error, or use the `Eventually(func(g Gomega) { ... })` form so failed assertions are caught and retried. - **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. -- **Test behavior through the layer that owns it.** Do not rely only on helper tests or source-fragment assertions when behavior depends on controller scheduling, rendered configuration, or stateful UI interactions. Exercise the behavior through `Reconcile`, rendered output, or executable UI tests, and cover non-default configuration branches that change the result. Assert the primary effect directly, such as verifying that a process exited or that exact identifiers reached a fake, instead of relying only on logs, journals, or status that can succeed independently. Fakes, fixtures, and command shims must exercise changed response fields, validate the complete request shape, reject unexpected calls, and expose call counts when batching or request reduction is part of the contract. Tests that start goroutines or child processes must make blocking operations cancellation-aware and verify cleanup before returning so background work cannot leak after a passing assertion. When static-source assertions are appropriate, such as for CSS rules, assert behavior-relevant declarations and the absence of conflicting declarations instead of matching an exact serialized or minified block. +- **Test behavior through the layer that owns it.** Do not rely only on helper tests or source-fragment assertions when behavior depends on controller scheduling, rendered configuration, or stateful UI interactions. Exercise the behavior through `Reconcile`, rendered output, or executable UI tests, and cover non-default configuration branches that change the result. For state-dependent decision logic, cover each relevant terminal, transient, and unknown class plus combinations where independently updated status sources lag one another. Assert the primary effect directly, such as verifying that a process exited or that exact identifiers reached a fake, instead of relying only on logs, journals, or status that can succeed independently. Fakes, fixtures, and command shims must exercise changed response fields, validate the complete request shape, reject unexpected calls, and expose call counts when batching or request reduction is part of the contract. Tests that start goroutines or child processes must make blocking operations cancellation-aware and verify cleanup before returning so background work cannot leak after a passing assertion. When static-source assertions are appropriate, such as for CSS rules, assert behavior-relevant declarations and the absence of conflicting declarations instead of matching an exact serialized or minified block. - **Keep shared feature contracts consistent across clients.** When the web UI, TUI, and plain CLI expose the same feature, apply the same limits, empty-input rules, failure semantics, and discoverability. Test each surface's contract instead of assuming shared backend validation makes the clients equivalent. - **Separate historical replay from live event handling.** When a UI or CLI paginates or replays retained events while receiving live events, keep replay rendering and state separate from the live stream and its live-only side effects. Preserve cursors, in-flight request state, and independently delivered current state. Test page loading during an active stream, duplicate requests, reconnects, live-only notifications, and viewport and ordering preservation. - **Avoid vacuous substring assertions in printer/formatter and stream-forwarding 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. When asserting that a writer or pipe forwards its input, compare the full normalized output for byte equality, not `strings.Contains` per line — a per-line substring check still passes when lines are dropped, duplicated, or reordered. diff --git a/self-development/agentconfig.yaml b/self-development/agentconfig.yaml index 9296e27d3..faf5f3604 100644 --- a/self-development/agentconfig.yaml +++ b/self-development/agentconfig.yaml @@ -36,7 +36,7 @@ spec: - Fail fast on invalid configuration: do not silently fall back to degraded behavior (e.g., unauthenticated requests) when configuration or credentials are invalid or missing; return an error or exit immediately - Protect credentials in configuration and transport: values that may contain tokens, authorization headers, or other credentials must come from Kubernetes Secret references, not plaintext Helm values or rendered manifests. Require HTTPS for configurable endpoints that receive credentials unless the API has an explicit insecure opt-in. - Classify request failures by cause: when mapping errors to HTTP status, reserve `4xx` for client-correctable input and confirmed not-found cases, and return `5xx` for lookup, RBAC, storage, and transport failures. A `List`/`Get` error is not an empty result; propagate it so webhook senders can retry. Use typed or sentinel errors such as `errors.Is` and `apierrors.IsNotFound` instead of matching message text, because wrapped server failures must not become client errors. - - Protect asynchronous state transitions from stale work: after asynchronous work completes, re-read current state instead of rendering or restoring a captured pre-request snapshot, and verify the operation still belongs to the current resource, Session, dialog, or request generation before mutating shared state. Derive completion, interruption, and control behavior from the exact active operation's identity and kind, not provider-global state. Coordinate startup, handoff, and destructive lifecycle transitions so stale notifications or newly accepted work cannot act on a different operation. + - Protect state-dependent transitions from stale work: after asynchronous work completes, re-read current state instead of rendering or restoring a captured pre-request snapshot, and verify the operation still belongs to the current resource, Session, dialog, or request generation before mutating shared state. When lifecycle signals can disagree or lag, inspect terminal and failure evidence before returning a transient or retry decision. Derive completion, interruption, and control behavior from the exact active operation's identity and kind, not provider-global state. Coordinate startup, handoff, and destructive lifecycle transitions so stale notifications or newly accepted work cannot act on a different operation. - Keep API surfaces minimal: when adding new API fields, types, or CRD changes, include only what is immediately needed; do not add speculative fields — API is hard to change once shipped - API changes must preserve backward compatibility for existing manifests: existing in-cluster resources must continue to apply after a CRD update. Do not change a field's kind (scalar ↔ array) on an existing field; do not add `MinLength`/`Required` to a field that previously accepted absence/empty values; replace by deprecating (mark `+deprecated`, keep functional) rather than removing. When the schema must change, sweep `examples/` and `self-development/` for YAMLs using the old form and update them in the same PR. - Treat controller-generated Kubernetes names as durable identities: validate derived names against every generated resource kind's exact name grammar, then account for every downstream suffix and label limit, including Pod, PVC, and ControllerRevision names, rather than checking only the source resource or the 63-character resource-name limit. Do not rename existing controller-owned resources just to satisfy a bound unless the change includes a collision-safe migration that preserves ownership and persistent data. @@ -48,7 +48,7 @@ spec: - Do not use Gomega's global `Expect()` inside `Eventually` polling blocks: Gomega does not retry on `Expect` failures, so a transient API error short-circuits the poller. In e2e `WaitFor*` helpers, either inline the call and return a zero-value on error, or use the `Eventually(func(g Gomega) { ... })` form so failed assertions are caught and retried. - 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. - - Test behavior through the layer that owns it: do not rely only on helper tests or source-fragment assertions when behavior depends on controller scheduling, rendered configuration, or stateful UI interactions. Exercise the behavior through `Reconcile`, rendered output, or executable UI tests, and cover non-default configuration branches that change the result. Assert the primary effect directly, such as verifying that a process exited or that exact identifiers reached a fake, instead of relying only on logs, journals, or status that can succeed independently. Fakes, fixtures, and command shims must exercise changed response fields, validate the complete request shape, reject unexpected calls, and expose call counts when batching or request reduction is part of the contract. Tests that start goroutines or child processes must make blocking operations cancellation-aware and verify cleanup before returning so background work cannot leak after a passing assertion. When static-source assertions are appropriate, such as for CSS rules, assert behavior-relevant declarations and the absence of conflicting declarations instead of matching an exact serialized or minified block. + - Test behavior through the layer that owns it: do not rely only on helper tests or source-fragment assertions when behavior depends on controller scheduling, rendered configuration, or stateful UI interactions. Exercise the behavior through `Reconcile`, rendered output, or executable UI tests, and cover non-default configuration branches that change the result. For state-dependent decision logic, cover each relevant terminal, transient, and unknown class plus combinations where independently updated status sources lag one another. Assert the primary effect directly, such as verifying that a process exited or that exact identifiers reached a fake, instead of relying only on logs, journals, or status that can succeed independently. Fakes, fixtures, and command shims must exercise changed response fields, validate the complete request shape, reject unexpected calls, and expose call counts when batching or request reduction is part of the contract. Tests that start goroutines or child processes must make blocking operations cancellation-aware and verify cleanup before returning so background work cannot leak after a passing assertion. When static-source assertions are appropriate, such as for CSS rules, assert behavior-relevant declarations and the absence of conflicting declarations instead of matching an exact serialized or minified block. - Keep shared feature contracts consistent across clients: when the web UI, TUI, and plain CLI expose the same feature, apply the same limits, empty-input rules, failure semantics, and discoverability. Test each surface's contract instead of assuming shared backend validation makes the clients equivalent. - Separate historical replay from live event handling: when a UI or CLI paginates or replays retained events while receiving live events, keep replay rendering and state separate from the live stream and its live-only side effects. Preserve cursors, in-flight request state, and independently delivered current state. Test page loading during an active stream, duplicate requests, reconnects, live-only notifications, and viewport and ordering preservation. - Avoid vacuous substring assertions in printer/formatter and stream-forwarding 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. When asserting that a writer or pipe forwards its input, compare the full normalized output for byte equality, not `strings.Contains` per line — a substring check still passes when lines are dropped, duplicated, or reordered. From a962f922278523fd46a9afb2a1116ab9ba90183d Mon Sep 17 00:00:00 2001 From: Gunju Kim Date: Wed, 19 Aug 2026 18:06:04 +0000 Subject: [PATCH 14/21] self-development: require realistic keyed and set fixtures --- AGENTS.md | 2 +- self-development/agentconfig.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 7fa849f8c..175dd7b5a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -27,7 +27,7 @@ - **Do not use Gomega's global `Expect()` inside `Eventually` polling blocks.** Gomega does not retry on `Expect` failures — a transient API error short-circuits the poller and fails the test on the first blip instead of retrying. In e2e `WaitFor*` helpers, either inline the call and return a zero-value on error, or use the `Eventually(func(g Gomega) { ... })` form so failed assertions are caught and retried. - **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. -- **Test behavior through the layer that owns it.** Do not rely only on helper tests or source-fragment assertions when behavior depends on controller scheduling, rendered configuration, or stateful UI interactions. Exercise the behavior through `Reconcile`, rendered output, or executable UI tests, and cover non-default configuration branches that change the result. For state-dependent decision logic, cover each relevant terminal, transient, and unknown class plus combinations where independently updated status sources lag one another. Assert the primary effect directly, such as verifying that a process exited or that exact identifiers reached a fake, instead of relying only on logs, journals, or status that can succeed independently. Fakes, fixtures, and command shims must exercise changed response fields, validate the complete request shape, reject unexpected calls, and expose call counts when batching or request reduction is part of the contract. Tests that start goroutines or child processes must make blocking operations cancellation-aware and verify cleanup before returning so background work cannot leak after a passing assertion. When static-source assertions are appropriate, such as for CSS rules, assert behavior-relevant declarations and the absence of conflicting declarations instead of matching an exact serialized or minified block. +- **Test behavior through the layer that owns it.** Do not rely only on helper tests or source-fragment assertions when behavior depends on controller scheduling, rendered configuration, or stateful UI interactions. Exercise the behavior through `Reconcile`, rendered output, or executable UI tests, and cover non-default configuration branches that change the result. For state-dependent decision logic, cover each relevant terminal, transient, and unknown class plus combinations where independently updated status sources lag one another. Assert the primary effect directly, such as verifying that a process exited or that exact identifiers reached a fake, instead of relying only on logs, journals, or status that can succeed independently. Fakes, fixtures, and command shims must exercise changed response fields, validate the complete request shape, reject unexpected calls, and expose call counts when batching or request reduction is part of the contract. Populate production identity fields used as keys, and use multi-element fixtures when behavior depends on collection membership or aggregation; empty identities and singleton-only cases can let keyed or set-based logic pass vacuously. Tests that start goroutines or child processes must make blocking operations cancellation-aware and verify cleanup before returning so background work cannot leak after a passing assertion. When static-source assertions are appropriate, such as for CSS rules, assert behavior-relevant declarations and the absence of conflicting declarations instead of matching an exact serialized or minified block. - **Keep shared feature contracts consistent across clients.** When the web UI, TUI, and plain CLI expose the same feature, apply the same limits, empty-input rules, failure semantics, and discoverability. Test each surface's contract instead of assuming shared backend validation makes the clients equivalent. - **Separate historical replay from live event handling.** When a UI or CLI paginates or replays retained events while receiving live events, keep replay rendering and state separate from the live stream and its live-only side effects. Preserve cursors, in-flight request state, and independently delivered current state. Test page loading during an active stream, duplicate requests, reconnects, live-only notifications, and viewport and ordering preservation. - **Avoid vacuous substring assertions in printer/formatter and stream-forwarding 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. When asserting that a writer or pipe forwards its input, compare the full normalized output for byte equality, not `strings.Contains` per line — a per-line substring check still passes when lines are dropped, duplicated, or reordered. diff --git a/self-development/agentconfig.yaml b/self-development/agentconfig.yaml index faf5f3604..c4cf8b862 100644 --- a/self-development/agentconfig.yaml +++ b/self-development/agentconfig.yaml @@ -48,7 +48,7 @@ spec: - Do not use Gomega's global `Expect()` inside `Eventually` polling blocks: Gomega does not retry on `Expect` failures, so a transient API error short-circuits the poller. In e2e `WaitFor*` helpers, either inline the call and return a zero-value on error, or use the `Eventually(func(g Gomega) { ... })` form so failed assertions are caught and retried. - 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. - - Test behavior through the layer that owns it: do not rely only on helper tests or source-fragment assertions when behavior depends on controller scheduling, rendered configuration, or stateful UI interactions. Exercise the behavior through `Reconcile`, rendered output, or executable UI tests, and cover non-default configuration branches that change the result. For state-dependent decision logic, cover each relevant terminal, transient, and unknown class plus combinations where independently updated status sources lag one another. Assert the primary effect directly, such as verifying that a process exited or that exact identifiers reached a fake, instead of relying only on logs, journals, or status that can succeed independently. Fakes, fixtures, and command shims must exercise changed response fields, validate the complete request shape, reject unexpected calls, and expose call counts when batching or request reduction is part of the contract. Tests that start goroutines or child processes must make blocking operations cancellation-aware and verify cleanup before returning so background work cannot leak after a passing assertion. When static-source assertions are appropriate, such as for CSS rules, assert behavior-relevant declarations and the absence of conflicting declarations instead of matching an exact serialized or minified block. + - Test behavior through the layer that owns it: do not rely only on helper tests or source-fragment assertions when behavior depends on controller scheduling, rendered configuration, or stateful UI interactions. Exercise the behavior through `Reconcile`, rendered output, or executable UI tests, and cover non-default configuration branches that change the result. For state-dependent decision logic, cover each relevant terminal, transient, and unknown class plus combinations where independently updated status sources lag one another. Assert the primary effect directly, such as verifying that a process exited or that exact identifiers reached a fake, instead of relying only on logs, journals, or status that can succeed independently. Fakes, fixtures, and command shims must exercise changed response fields, validate the complete request shape, reject unexpected calls, and expose call counts when batching or request reduction is part of the contract. Populate production identity fields used as keys, and use multi-element fixtures when behavior depends on collection membership or aggregation; empty identities and singleton-only cases can let keyed or set-based logic pass vacuously. Tests that start goroutines or child processes must make blocking operations cancellation-aware and verify cleanup before returning so background work cannot leak after a passing assertion. When static-source assertions are appropriate, such as for CSS rules, assert behavior-relevant declarations and the absence of conflicting declarations instead of matching an exact serialized or minified block. - Keep shared feature contracts consistent across clients: when the web UI, TUI, and plain CLI expose the same feature, apply the same limits, empty-input rules, failure semantics, and discoverability. Test each surface's contract instead of assuming shared backend validation makes the clients equivalent. - Separate historical replay from live event handling: when a UI or CLI paginates or replays retained events while receiving live events, keep replay rendering and state separate from the live stream and its live-only side effects. Preserve cursors, in-flight request state, and independently delivered current state. Test page loading during an active stream, duplicate requests, reconnects, live-only notifications, and viewport and ordering preservation. - Avoid vacuous substring assertions in printer/formatter and stream-forwarding 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. When asserting that a writer or pipe forwards its input, compare the full normalized output for byte equality, not `strings.Contains` per line — a substring check still passes when lines are dropped, duplicated, or reordered. From 35f6b450b44847caa119e519d86c1857d70e74a4 Mon Sep 17 00:00:00 2001 From: Gunju Kim Date: Thu, 20 Aug 2026 18:10:16 +0000 Subject: [PATCH 15/21] self-development: add upgrade, API, and keyboard conventions --- AGENTS.md | 3 +++ self-development/agentconfig.yaml | 3 +++ 2 files changed, 6 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 175dd7b5a..6f11ac19a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -15,7 +15,9 @@ - **Protect state-dependent transitions from stale work.** After asynchronous work completes, re-read current state instead of rendering or restoring a captured pre-request snapshot, and verify the operation still belongs to the current resource, Session, dialog, or request generation before mutating shared state. When lifecycle signals can disagree or lag, inspect terminal and failure evidence before returning a transient or retry decision. Derive completion, interruption, and control behavior from the exact active operation's identity and kind, not provider-global state. Coordinate startup, handoff, and destructive lifecycle transitions so stale notifications or newly accepted work cannot act on a different operation. - **Keep API surfaces minimal.** When adding new API fields, types, or CRD changes, include only what is immediately needed. Do not add speculative fields — API is hard to change once shipped. Start with the minimum viable API and extend in follow-up PRs. - **API changes must preserve backward compatibility for existing manifests.** Existing in-cluster resources must continue to apply after a CRD update. Do not change a field's kind (scalar ↔ array, string ↔ object) on an existing field; do not add `MinLength`, `Required`, or other tightening validation to a field that previously accepted absence/empty values; when replacing a field, mark the old one `+deprecated` and keep it functional rather than removing it. When the schema must change, sweep `examples/`, `self-development/`, and any in-tree YAMLs that use the old form and update them in the same PR. +- **Test API mutability through updates.** When making an API field mutable or changing transition validation, cover every newly allowed presence transition the contract promises, such as unset-to-set and set-to-unset, and at least one invalid `Update` that proves the remaining validation still applies. Create-only validation tests do not pin update behavior. - **Treat controller-generated Kubernetes names as durable identities.** Validate derived names against every generated resource kind's exact name grammar, then account for every downstream suffix and label limit, including Pod, PVC, and ControllerRevision names, rather than checking only the source resource or the 63-character resource-name limit. Do not rename existing controller-owned resources just to satisfy a bound unless the change includes a collision-safe migration that preserves ownership and persistent data. +- **Make component renames upgrade-safe.** Before renaming installed Kubernetes objects, Helm value paths, or persisted client keys, inventory the identifiers used by existing deployments. Preserve or explicitly migrate stored state, clean up superseded resources when installation does not prune, keep uninstall cleanup for legacy names, and reject unsupported old configuration with an actionable error. Test an upgrade from prior configuration and state in addition to a fresh install. - **Maintain API changes only in the latest Kelos API version.** Add new CRD fields, validation, enum values, constants, and user-facing API behavior only to the latest served/storage API version. Older served versions are compatibility surfaces: keep them served, convertible, and backward-compatible, but do not add new capabilities there unless required to preserve existing manifests or implement conversion/migration compatibility. - **Import versioned Kelos APIs with aliases.** Prefer aliasing the current versioned API import as `kelos` when a file uses only one API version. Use explicit aliases like `kelosv1alpha1` only when a file genuinely needs multiple versions. - **Docs must match implementation, not aspiration.** When writing or updating docs, READMEs, or comments, describe only what the code actually does. Do not document unimplemented behavior, overstate guarantees, or describe security checks (e.g., HMAC validation) that aren't enforced. Before describing a contract ("X is filtered", "Y is validated"), verify the code enforces it — partial enforcement should be documented as partial. @@ -24,6 +26,7 @@ - **Keep `docs/reference.md` user-facing.** Document API fields, configuration, validation, defaults, and observable operational behavior. Do not describe controller or runtime implementation details unless users need them to configure, operate, or troubleshoot Kelos. Prefer outcomes over mechanisms, and remove nonessential internal detail instead of moving it to a separate architecture document. - **Honor reduced-motion preferences in web UI.** Do not add continuous or non-essential animation without a `prefers-reduced-motion` fallback. Keep the static state distinguishable when animation is disabled; prefer gating animation under `@media (prefers-reduced-motion: no-preference)`. - **Audit responsive rules when changing web controls.** When adding or renaming a form field or control, include it in every applicable selector group and breakpoint override. In particular, preserve mobile font sizing that prevents focus zoom, touch-target sizing, and alignment with adjacent controls. +- **Preserve keyboard focus in dynamic web controls.** When adding menus, dialogs, or actions that close or rerender their trigger, keep a clearly visible `:focus-visible` indicator, dismiss overlays through expected keyboard navigation such as Escape and Tab, and restore focus to the current replacement trigger after success and failure. Exercise keyboard activation, dismissal, and focus restoration in browser-level tests. - **Do not use Gomega's global `Expect()` inside `Eventually` polling blocks.** Gomega does not retry on `Expect` failures — a transient API error short-circuits the poller and fails the test on the first blip instead of retrying. In e2e `WaitFor*` helpers, either inline the call and return a zero-value on error, or use the `Eventually(func(g Gomega) { ... })` form so failed assertions are caught and retried. - **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. diff --git a/self-development/agentconfig.yaml b/self-development/agentconfig.yaml index c4cf8b862..32090ff19 100644 --- a/self-development/agentconfig.yaml +++ b/self-development/agentconfig.yaml @@ -39,12 +39,15 @@ spec: - Protect state-dependent transitions from stale work: after asynchronous work completes, re-read current state instead of rendering or restoring a captured pre-request snapshot, and verify the operation still belongs to the current resource, Session, dialog, or request generation before mutating shared state. When lifecycle signals can disagree or lag, inspect terminal and failure evidence before returning a transient or retry decision. Derive completion, interruption, and control behavior from the exact active operation's identity and kind, not provider-global state. Coordinate startup, handoff, and destructive lifecycle transitions so stale notifications or newly accepted work cannot act on a different operation. - Keep API surfaces minimal: when adding new API fields, types, or CRD changes, include only what is immediately needed; do not add speculative fields — API is hard to change once shipped - API changes must preserve backward compatibility for existing manifests: existing in-cluster resources must continue to apply after a CRD update. Do not change a field's kind (scalar ↔ array) on an existing field; do not add `MinLength`/`Required` to a field that previously accepted absence/empty values; replace by deprecating (mark `+deprecated`, keep functional) rather than removing. When the schema must change, sweep `examples/` and `self-development/` for YAMLs using the old form and update them in the same PR. + - Test API mutability through updates: when making an API field mutable or changing transition validation, cover every newly allowed presence transition the contract promises, such as unset-to-set and set-to-unset, and at least one invalid `Update` that proves the remaining validation still applies. Create-only validation tests do not pin update behavior. - Treat controller-generated Kubernetes names as durable identities: validate derived names against every generated resource kind's exact name grammar, then account for every downstream suffix and label limit, including Pod, PVC, and ControllerRevision names, rather than checking only the source resource or the 63-character resource-name limit. Do not rename existing controller-owned resources just to satisfy a bound unless the change includes a collision-safe migration that preserves ownership and persistent data. + - Make component renames upgrade-safe: before renaming installed Kubernetes objects, Helm value paths, or persisted client keys, inventory the identifiers used by existing deployments. Preserve or explicitly migrate stored state, clean up superseded resources when installation does not prune, keep uninstall cleanup for legacy names, and reject unsupported old configuration with an actionable error. Test an upgrade from prior configuration and state in addition to a fresh install. - Docs must match implementation, not aspiration: describe only what the code actually does; do not document unimplemented behavior, overstate guarantees, or describe security checks (e.g., HMAC validation) that aren't enforced. Verify the code enforces a contract before documenting it. - Keep documentation complete and internally consistent: when changing documented behavior, search for every related overview, per-component section, trigger table, setup example, and command/flag reference, and update all affected mentions together. Do not remove documentation for still-supported behavior when replacing a nearby section. - Preserve prompt contracts when consolidating agent instructions: before removing or shortening a TaskSpawner or SessionSpawner prompt because behavior is shared, map every removed requirement to a concrete instruction source that the spawned agent actually loads, such as a referenced AgentConfig, repository `AGENTS.md`, or skill. Retain requirements with no loaded owner, and update prompt-contract tests and every README section that claims the behavior in the same change. - Honor reduced-motion preferences in web UI: do not add continuous or non-essential animation without a `prefers-reduced-motion` fallback. Keep the static state distinguishable when animation is disabled; prefer gating animation under `@media (prefers-reduced-motion: no-preference)`. - Audit responsive rules when changing web controls: when adding or renaming a form field or control, include it in every applicable selector group and breakpoint override. In particular, preserve mobile font sizing that prevents focus zoom, touch-target sizing, and alignment with adjacent controls. + - Preserve keyboard focus in dynamic web controls: when adding menus, dialogs, or actions that close or rerender their trigger, keep a clearly visible `:focus-visible` indicator, dismiss overlays through expected keyboard navigation such as Escape and Tab, and restore focus to the current replacement trigger after success and failure. Exercise keyboard activation, dismissal, and focus restoration in browser-level tests. - Do not use Gomega's global `Expect()` inside `Eventually` polling blocks: Gomega does not retry on `Expect` failures, so a transient API error short-circuits the poller. In e2e `WaitFor*` helpers, either inline the call and return a zero-value on error, or use the `Eventually(func(g Gomega) { ... })` form so failed assertions are caught and retried. - 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. From fdc61c59822ff1ea83ff97164dd26ce8a01258dd Mon Sep 17 00:00:00 2001 From: Gunju Kim Date: Fri, 21 Aug 2026 18:11:04 +0000 Subject: [PATCH 16/21] self-development: require keyboard navigation for custom widgets Roving tabindex removes inactive controls from sequential focus, so custom tablists and menus need explicit Arrow and Home/End navigation. Extend the existing focus-restoration rule to cover the complete keyboard interaction model and browser-level navigation tests. --- AGENTS.md | 2 +- self-development/agentconfig.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 6f11ac19a..e3d2520a8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -26,7 +26,7 @@ - **Keep `docs/reference.md` user-facing.** Document API fields, configuration, validation, defaults, and observable operational behavior. Do not describe controller or runtime implementation details unless users need them to configure, operate, or troubleshoot Kelos. Prefer outcomes over mechanisms, and remove nonessential internal detail instead of moving it to a separate architecture document. - **Honor reduced-motion preferences in web UI.** Do not add continuous or non-essential animation without a `prefers-reduced-motion` fallback. Keep the static state distinguishable when animation is disabled; prefer gating animation under `@media (prefers-reduced-motion: no-preference)`. - **Audit responsive rules when changing web controls.** When adding or renaming a form field or control, include it in every applicable selector group and breakpoint override. In particular, preserve mobile font sizing that prevents focus zoom, touch-target sizing, and alignment with adjacent controls. -- **Preserve keyboard focus in dynamic web controls.** When adding menus, dialogs, or actions that close or rerender their trigger, keep a clearly visible `:focus-visible` indicator, dismiss overlays through expected keyboard navigation such as Escape and Tab, and restore focus to the current replacement trigger after success and failure. Exercise keyboard activation, dismissal, and focus restoration in browser-level tests. +- **Preserve keyboard operability in dynamic web controls.** Implement the expected keyboard interaction model for custom widgets. When tabs, menu items, or similar controls use roving `tabindex`, add Arrow-key and Home/End navigation so every item remains reachable. For menus, dialogs, or actions that close or rerender their trigger, keep a clearly visible `:focus-visible` indicator, dismiss overlays through expected keyboard navigation such as Escape and Tab, and restore focus to the current replacement trigger after success and failure. Exercise keyboard activation, navigation, dismissal, and focus restoration in browser-level tests. - **Do not use Gomega's global `Expect()` inside `Eventually` polling blocks.** Gomega does not retry on `Expect` failures — a transient API error short-circuits the poller and fails the test on the first blip instead of retrying. In e2e `WaitFor*` helpers, either inline the call and return a zero-value on error, or use the `Eventually(func(g Gomega) { ... })` form so failed assertions are caught and retried. - **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. diff --git a/self-development/agentconfig.yaml b/self-development/agentconfig.yaml index 32090ff19..cc58fee7c 100644 --- a/self-development/agentconfig.yaml +++ b/self-development/agentconfig.yaml @@ -47,7 +47,7 @@ spec: - Preserve prompt contracts when consolidating agent instructions: before removing or shortening a TaskSpawner or SessionSpawner prompt because behavior is shared, map every removed requirement to a concrete instruction source that the spawned agent actually loads, such as a referenced AgentConfig, repository `AGENTS.md`, or skill. Retain requirements with no loaded owner, and update prompt-contract tests and every README section that claims the behavior in the same change. - Honor reduced-motion preferences in web UI: do not add continuous or non-essential animation without a `prefers-reduced-motion` fallback. Keep the static state distinguishable when animation is disabled; prefer gating animation under `@media (prefers-reduced-motion: no-preference)`. - Audit responsive rules when changing web controls: when adding or renaming a form field or control, include it in every applicable selector group and breakpoint override. In particular, preserve mobile font sizing that prevents focus zoom, touch-target sizing, and alignment with adjacent controls. - - Preserve keyboard focus in dynamic web controls: when adding menus, dialogs, or actions that close or rerender their trigger, keep a clearly visible `:focus-visible` indicator, dismiss overlays through expected keyboard navigation such as Escape and Tab, and restore focus to the current replacement trigger after success and failure. Exercise keyboard activation, dismissal, and focus restoration in browser-level tests. + - Preserve keyboard operability in dynamic web controls: implement the expected keyboard interaction model for custom widgets. When tabs, menu items, or similar controls use roving `tabindex`, add Arrow-key and Home/End navigation so every item remains reachable. For menus, dialogs, or actions that close or rerender their trigger, keep a clearly visible `:focus-visible` indicator, dismiss overlays through expected keyboard navigation such as Escape and Tab, and restore focus to the current replacement trigger after success and failure. Exercise keyboard activation, navigation, dismissal, and focus restoration in browser-level tests. - Do not use Gomega's global `Expect()` inside `Eventually` polling blocks: Gomega does not retry on `Expect` failures, so a transient API error short-circuits the poller. In e2e `WaitFor*` helpers, either inline the call and return a zero-value on error, or use the `Eventually(func(g Gomega) { ... })` form so failed assertions are caught and retried. - 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. From 122435292d1a56961fdb725773ea22f5661adfe9 Mon Sep 17 00:00:00 2001 From: Gunju Kim Date: Sat, 22 Aug 2026 18:11:51 +0000 Subject: [PATCH 17/21] self-development: require complete independent behavior tests --- AGENTS.md | 2 +- self-development/agentconfig.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index e3d2520a8..df6bffe94 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -29,7 +29,7 @@ - **Preserve keyboard operability in dynamic web controls.** Implement the expected keyboard interaction model for custom widgets. When tabs, menu items, or similar controls use roving `tabindex`, add Arrow-key and Home/End navigation so every item remains reachable. For menus, dialogs, or actions that close or rerender their trigger, keep a clearly visible `:focus-visible` indicator, dismiss overlays through expected keyboard navigation such as Escape and Tab, and restore focus to the current replacement trigger after success and failure. Exercise keyboard activation, navigation, dismissal, and focus restoration in browser-level tests. - **Do not use Gomega's global `Expect()` inside `Eventually` polling blocks.** Gomega does not retry on `Expect` failures — a transient API error short-circuits the poller and fails the test on the first blip instead of retrying. In e2e `WaitFor*` helpers, either inline the call and return a zero-value on error, or use the `Eventually(func(g Gomega) { ... })` form so failed assertions are caught and retried. - **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. +- **Test every distinct behavior path.** When changed code has early-return or idempotent guards and a primary action, cover the primary action and every contractually distinct guard. For switch, table, or derivation logic, inventory and exercise every relevant source variant; use exact-set assertions when omissions matter. Arrange each case so it reaches the intended path and would fail if that path were removed or bypassed by an earlier condition. Tests must establish their own prerequisites or use explicit shared setup, include every runtime dependency, and remain executable independently. If production code lacks a seam, add one (interface, function field, fake client) so the behavior is testable. - **Test behavior through the layer that owns it.** Do not rely only on helper tests or source-fragment assertions when behavior depends on controller scheduling, rendered configuration, or stateful UI interactions. Exercise the behavior through `Reconcile`, rendered output, or executable UI tests, and cover non-default configuration branches that change the result. For state-dependent decision logic, cover each relevant terminal, transient, and unknown class plus combinations where independently updated status sources lag one another. Assert the primary effect directly, such as verifying that a process exited or that exact identifiers reached a fake, instead of relying only on logs, journals, or status that can succeed independently. Fakes, fixtures, and command shims must exercise changed response fields, validate the complete request shape, reject unexpected calls, and expose call counts when batching or request reduction is part of the contract. Populate production identity fields used as keys, and use multi-element fixtures when behavior depends on collection membership or aggregation; empty identities and singleton-only cases can let keyed or set-based logic pass vacuously. Tests that start goroutines or child processes must make blocking operations cancellation-aware and verify cleanup before returning so background work cannot leak after a passing assertion. When static-source assertions are appropriate, such as for CSS rules, assert behavior-relevant declarations and the absence of conflicting declarations instead of matching an exact serialized or minified block. - **Keep shared feature contracts consistent across clients.** When the web UI, TUI, and plain CLI expose the same feature, apply the same limits, empty-input rules, failure semantics, and discoverability. Test each surface's contract instead of assuming shared backend validation makes the clients equivalent. - **Separate historical replay from live event handling.** When a UI or CLI paginates or replays retained events while receiving live events, keep replay rendering and state separate from the live stream and its live-only side effects. Preserve cursors, in-flight request state, and independently delivered current state. Test page loading during an active stream, duplicate requests, reconnects, live-only notifications, and viewport and ordering preservation. diff --git a/self-development/agentconfig.yaml b/self-development/agentconfig.yaml index cc58fee7c..d3dd190e0 100644 --- a/self-development/agentconfig.yaml +++ b/self-development/agentconfig.yaml @@ -50,7 +50,7 @@ spec: - Preserve keyboard operability in dynamic web controls: implement the expected keyboard interaction model for custom widgets. When tabs, menu items, or similar controls use roving `tabindex`, add Arrow-key and Home/End navigation so every item remains reachable. For menus, dialogs, or actions that close or rerender their trigger, keep a clearly visible `:focus-visible` indicator, dismiss overlays through expected keyboard navigation such as Escape and Tab, and restore focus to the current replacement trigger after success and failure. Exercise keyboard activation, navigation, dismissal, and focus restoration in browser-level tests. - Do not use Gomega's global `Expect()` inside `Eventually` polling blocks: Gomega does not retry on `Expect` failures, so a transient API error short-circuits the poller. In e2e `WaitFor*` helpers, either inline the call and return a zero-value on error, or use the `Eventually(func(g Gomega) { ... })` form so failed assertions are caught and retried. - 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. + - Test every distinct behavior path: when changed code has early-return or idempotent guards and a primary action, cover the primary action and every contractually distinct guard. For switch, table, or derivation logic, inventory and exercise every relevant source variant; use exact-set assertions when omissions matter. Arrange each case so it reaches the intended path and would fail if that path were removed or bypassed by an earlier condition. Tests must establish their own prerequisites or use explicit shared setup, include every runtime dependency, and remain executable independently. If production code lacks a seam, add one (interface, function field, fake client) so the behavior is testable. - Test behavior through the layer that owns it: do not rely only on helper tests or source-fragment assertions when behavior depends on controller scheduling, rendered configuration, or stateful UI interactions. Exercise the behavior through `Reconcile`, rendered output, or executable UI tests, and cover non-default configuration branches that change the result. For state-dependent decision logic, cover each relevant terminal, transient, and unknown class plus combinations where independently updated status sources lag one another. Assert the primary effect directly, such as verifying that a process exited or that exact identifiers reached a fake, instead of relying only on logs, journals, or status that can succeed independently. Fakes, fixtures, and command shims must exercise changed response fields, validate the complete request shape, reject unexpected calls, and expose call counts when batching or request reduction is part of the contract. Populate production identity fields used as keys, and use multi-element fixtures when behavior depends on collection membership or aggregation; empty identities and singleton-only cases can let keyed or set-based logic pass vacuously. Tests that start goroutines or child processes must make blocking operations cancellation-aware and verify cleanup before returning so background work cannot leak after a passing assertion. When static-source assertions are appropriate, such as for CSS rules, assert behavior-relevant declarations and the absence of conflicting declarations instead of matching an exact serialized or minified block. - Keep shared feature contracts consistent across clients: when the web UI, TUI, and plain CLI expose the same feature, apply the same limits, empty-input rules, failure semantics, and discoverability. Test each surface's contract instead of assuming shared backend validation makes the clients equivalent. - Separate historical replay from live event handling: when a UI or CLI paginates or replays retained events while receiving live events, keep replay rendering and state separate from the live stream and its live-only side effects. Preserve cursors, in-flight request state, and independently delivered current state. Test page loading during an active stream, duplicate requests, reconnects, live-only notifications, and viewport and ordering preservation. From 0e3f61a27669354873dc522711753f0036b72998 Mon Sep 17 00:00:00 2001 From: Gunju Kim Date: Sun, 23 Aug 2026 18:10:56 +0000 Subject: [PATCH 18/21] self-development: centralize lockstep behavior --- AGENTS.md | 1 + self-development/agentconfig.yaml | 1 + 2 files changed, 2 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index df6bffe94..fc6ae523b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -4,6 +4,7 @@ - **Use Makefile targets** instead of discovering build/test commands yourself. - **Keep changes minimal.** Do not refactor, reorganize, or 'improve' code beyond what was explicitly requested. - **For CI/release workflows**, always use existing Makefile targets rather than reimplementing build logic in YAML. +- **Centralize behavior that must stay in lockstep.** When two code paths intentionally implement the same behavior and differ only by an input or action, share the implementation instead of copying it. Route repeated build and generation commands through one Makefile target so their flags cannot drift. Do not abstract incidental similarity. - **Better tests.** Always try to add or improve tests(including integration, e2e) when modifying code. - **Logging conventions.** Start log messages with capital letters and do not end with punctuation. - **Commit messages.** Do not include PR links in commit messages. diff --git a/self-development/agentconfig.yaml b/self-development/agentconfig.yaml index d3dd190e0..6d4f0a7df 100644 --- a/self-development/agentconfig.yaml +++ b/self-development/agentconfig.yaml @@ -26,6 +26,7 @@ spec: - `make test` — run all unit tests - `make test-integration` — run integration tests - `make build` — build binary + - Centralize behavior that must stay in lockstep: when two code paths intentionally implement the same behavior and differ only by an input or action, share the implementation instead of copying it. Route repeated build and generation commands through one Makefile target so their flags cannot drift. Do not abstract incidental similarity. - When polling for a process to finish in shell, do not use `pgrep -f 'CMD_NAME'`: the caller's own argv contains the pattern, so pgrep matches itself and the loop never exits (deadlocks the pod). Either capture the PID at launch (`cmd & PID=$!`) and poll with `kill -0 "$PID"` / `wait "$PID"`, or exclude self from the match (`pgrep -f 'CMD_NAME' | grep -vw $$`). - Always try to add or improve tests when modifying code - Logging conventions: start log messages with capital letters and do not end with punctuation From 418fcce8cf382f02a53cf3cf5cd6085fb046861a Mon Sep 17 00:00:00 2001 From: Gunju Kim Date: Mon, 24 Aug 2026 18:09:57 +0000 Subject: [PATCH 19/21] self-development: preserve web accessibility across layouts --- AGENTS.md | 3 ++- self-development/agentconfig.yaml | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index fc6ae523b..4f4fb9145 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -26,7 +26,8 @@ - **Preserve prompt contracts when consolidating agent instructions.** Before removing or shortening a TaskSpawner or SessionSpawner prompt because behavior is shared, map every removed requirement to a concrete instruction source that the spawned agent actually loads, such as a referenced AgentConfig, repository `AGENTS.md`, or skill. Retain requirements with no loaded owner, and update prompt-contract tests and every README section that claims the behavior in the same change. - **Keep `docs/reference.md` user-facing.** Document API fields, configuration, validation, defaults, and observable operational behavior. Do not describe controller or runtime implementation details unless users need them to configure, operate, or troubleshoot Kelos. Prefer outcomes over mechanisms, and remove nonessential internal detail instead of moving it to a separate architecture document. - **Honor reduced-motion preferences in web UI.** Do not add continuous or non-essential animation without a `prefers-reduced-motion` fallback. Keep the static state distinguishable when animation is disabled; prefer gating animation under `@media (prefers-reduced-motion: no-preference)`. -- **Audit responsive rules when changing web controls.** When adding or renaming a form field or control, include it in every applicable selector group and breakpoint override. In particular, preserve mobile font sizing that prevents focus zoom, touch-target sizing, and alignment with adjacent controls. +- **Preserve readable contrast across web UI themes.** When changing colors, theme tokens, or control styling, verify text, interactive labels, and focus indicators in every supported theme against the applicable WCAG AA contrast requirement. Do not assume a light-theme color remains legible on a dark background. +- **Audit responsive rules when changing web controls.** When adding or renaming a form field or control, include it in every applicable selector group and breakpoint override. In particular, preserve mobile font sizing that prevents focus zoom, touch-target sizing, and alignment with adjacent controls. When a breakpoint hides or replaces a control, keep every supported action available through an equivalent touch-operable path and test that path. - **Preserve keyboard operability in dynamic web controls.** Implement the expected keyboard interaction model for custom widgets. When tabs, menu items, or similar controls use roving `tabindex`, add Arrow-key and Home/End navigation so every item remains reachable. For menus, dialogs, or actions that close or rerender their trigger, keep a clearly visible `:focus-visible` indicator, dismiss overlays through expected keyboard navigation such as Escape and Tab, and restore focus to the current replacement trigger after success and failure. Exercise keyboard activation, navigation, dismissal, and focus restoration in browser-level tests. - **Do not use Gomega's global `Expect()` inside `Eventually` polling blocks.** Gomega does not retry on `Expect` failures — a transient API error short-circuits the poller and fails the test on the first blip instead of retrying. In e2e `WaitFor*` helpers, either inline the call and return a zero-value on error, or use the `Eventually(func(g Gomega) { ... })` form so failed assertions are caught and retried. - **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")`. diff --git a/self-development/agentconfig.yaml b/self-development/agentconfig.yaml index 6d4f0a7df..641c8cefb 100644 --- a/self-development/agentconfig.yaml +++ b/self-development/agentconfig.yaml @@ -47,7 +47,8 @@ spec: - Keep documentation complete and internally consistent: when changing documented behavior, search for every related overview, per-component section, trigger table, setup example, and command/flag reference, and update all affected mentions together. Do not remove documentation for still-supported behavior when replacing a nearby section. - Preserve prompt contracts when consolidating agent instructions: before removing or shortening a TaskSpawner or SessionSpawner prompt because behavior is shared, map every removed requirement to a concrete instruction source that the spawned agent actually loads, such as a referenced AgentConfig, repository `AGENTS.md`, or skill. Retain requirements with no loaded owner, and update prompt-contract tests and every README section that claims the behavior in the same change. - Honor reduced-motion preferences in web UI: do not add continuous or non-essential animation without a `prefers-reduced-motion` fallback. Keep the static state distinguishable when animation is disabled; prefer gating animation under `@media (prefers-reduced-motion: no-preference)`. - - Audit responsive rules when changing web controls: when adding or renaming a form field or control, include it in every applicable selector group and breakpoint override. In particular, preserve mobile font sizing that prevents focus zoom, touch-target sizing, and alignment with adjacent controls. + - Preserve readable contrast across web UI themes: when changing colors, theme tokens, or control styling, verify text, interactive labels, and focus indicators in every supported theme against the applicable WCAG AA contrast requirement. Do not assume a light-theme color remains legible on a dark background. + - Audit responsive rules when changing web controls: when adding or renaming a form field or control, include it in every applicable selector group and breakpoint override. In particular, preserve mobile font sizing that prevents focus zoom, touch-target sizing, and alignment with adjacent controls. When a breakpoint hides or replaces a control, keep every supported action available through an equivalent touch-operable path and test that path. - Preserve keyboard operability in dynamic web controls: implement the expected keyboard interaction model for custom widgets. When tabs, menu items, or similar controls use roving `tabindex`, add Arrow-key and Home/End navigation so every item remains reachable. For menus, dialogs, or actions that close or rerender their trigger, keep a clearly visible `:focus-visible` indicator, dismiss overlays through expected keyboard navigation such as Escape and Tab, and restore focus to the current replacement trigger after success and failure. Exercise keyboard activation, navigation, dismissal, and focus restoration in browser-level tests. - Do not use Gomega's global `Expect()` inside `Eventually` polling blocks: Gomega does not retry on `Expect` failures, so a transient API error short-circuits the poller. In e2e `WaitFor*` helpers, either inline the call and return a zero-value on error, or use the `Eventually(func(g Gomega) { ... })` form so failed assertions are caught and retried. - 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")`. From 2315e5250444c64126bba8945a9ad29721420478 Mon Sep 17 00:00:00 2001 From: Gunju Kim Date: Tue, 25 Aug 2026 18:09:21 +0000 Subject: [PATCH 20/21] self-development: preserve Unicode text semantics --- AGENTS.md | 1 + self-development/agentconfig.yaml | 1 + 2 files changed, 2 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 4f4fb9145..f85f64bcc 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -9,6 +9,7 @@ - **Logging conventions.** Start log messages with capital letters and do not end with punctuation. - **Commit messages.** Do not include PR links in commit messages. - **Kubernetes resource comparison.** Use semantic `.Equal()` or `.Cmp()` methods for `resource.Quantity` comparisons, not `reflect.DeepEqual` — structurally different Quantity values can be semantically identical (e.g., `1000m` vs `1` CPU). +- **Preserve Unicode semantics in text processing.** When a limit is defined in characters, count Unicode code points rather than bytes. When enforcing byte limits or truncating UTF-8 text, cut only at valid code-point boundaries so user-visible output remains valid UTF-8. Cover affected limits with non-ASCII test cases. - **Never use `os.Getenv()` for secrets as Go `flag` defaults.** Go's `flag` package prints default values in usage/help output, which leaks secret values. Instead, use an empty default and read the env var after `flag.Parse()`. - **Fail fast on invalid configuration.** Do not silently fall back to degraded behavior (e.g., unauthenticated requests) when configuration or credentials are invalid or missing. Return an error or exit immediately instead of returning nil or empty values that mask the failure. - **Protect credentials in configuration and transport.** Values that may contain tokens, authorization headers, or other credentials must come from Kubernetes Secret references, not plaintext Helm values or rendered manifests. Require HTTPS for configurable endpoints that receive credentials unless the API has an explicit insecure opt-in. diff --git a/self-development/agentconfig.yaml b/self-development/agentconfig.yaml index 641c8cefb..b3cbaf093 100644 --- a/self-development/agentconfig.yaml +++ b/self-development/agentconfig.yaml @@ -33,6 +33,7 @@ spec: - Commit messages: do not include PR links in commit messages - When making structural changes (adding new files, configs, or components), update related documentation (especially README files) to stay in sync - Kubernetes resource comparison: use semantic `.Equal()` or `.Cmp()` methods for `resource.Quantity` comparisons, not `reflect.DeepEqual` + - Preserve Unicode semantics in text processing: when a limit is defined in characters, count Unicode code points rather than bytes. When enforcing byte limits or truncating UTF-8 text, cut only at valid code-point boundaries so user-visible output remains valid UTF-8. Cover affected limits with non-ASCII test cases. - Never use `os.Getenv()` for secrets as Go `flag` defaults: Go's `flag` package prints default values in usage/help output, which leaks secret values; use an empty default and read the env var after `flag.Parse()` - Fail fast on invalid configuration: do not silently fall back to degraded behavior (e.g., unauthenticated requests) when configuration or credentials are invalid or missing; return an error or exit immediately - Protect credentials in configuration and transport: values that may contain tokens, authorization headers, or other credentials must come from Kubernetes Secret references, not plaintext Helm values or rendered manifests. Require HTTPS for configurable endpoints that receive credentials unless the API has an explicit insecure opt-in. From d1b8925f4e8192657015999101645f607f694518 Mon Sep 17 00:00:00 2001 From: Gunju Kim Date: Wed, 26 Aug 2026 18:08:16 +0000 Subject: [PATCH 21/21] self-development: preserve behavior across web layout changes --- AGENTS.md | 1 + self-development/agentconfig.yaml | 1 + 2 files changed, 2 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index f85f64bcc..64599a1c3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -29,6 +29,7 @@ - **Honor reduced-motion preferences in web UI.** Do not add continuous or non-essential animation without a `prefers-reduced-motion` fallback. Keep the static state distinguishable when animation is disabled; prefer gating animation under `@media (prefers-reduced-motion: no-preference)`. - **Preserve readable contrast across web UI themes.** When changing colors, theme tokens, or control styling, verify text, interactive labels, and focus indicators in every supported theme against the applicable WCAG AA contrast requirement. Do not assume a light-theme color remains legible on a dark background. - **Audit responsive rules when changing web controls.** When adding or renaming a form field or control, include it in every applicable selector group and breakpoint override. In particular, preserve mobile font sizing that prevents focus zoom, touch-target sizing, and alignment with adjacent controls. When a breakpoint hides or replaces a control, keep every supported action available through an equivalent touch-operable path and test that path. +- **Preserve UI behavior when restructuring web views.** When moving scroll ownership, hiding or revealing a view, or replacing a DOM container, inventory every event listener and geometry- or visibility-derived state tied to the affected elements. Bind behavior to the element that actually scrolls, do not overwrite derived state from hidden layout geometry, and recompute it when the view becomes visible. Test the affected scroll interaction and hide/reveal transition through executable UI behavior. - **Preserve keyboard operability in dynamic web controls.** Implement the expected keyboard interaction model for custom widgets. When tabs, menu items, or similar controls use roving `tabindex`, add Arrow-key and Home/End navigation so every item remains reachable. For menus, dialogs, or actions that close or rerender their trigger, keep a clearly visible `:focus-visible` indicator, dismiss overlays through expected keyboard navigation such as Escape and Tab, and restore focus to the current replacement trigger after success and failure. Exercise keyboard activation, navigation, dismissal, and focus restoration in browser-level tests. - **Do not use Gomega's global `Expect()` inside `Eventually` polling blocks.** Gomega does not retry on `Expect` failures — a transient API error short-circuits the poller and fails the test on the first blip instead of retrying. In e2e `WaitFor*` helpers, either inline the call and return a zero-value on error, or use the `Eventually(func(g Gomega) { ... })` form so failed assertions are caught and retried. - **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")`. diff --git a/self-development/agentconfig.yaml b/self-development/agentconfig.yaml index b3cbaf093..17c355a47 100644 --- a/self-development/agentconfig.yaml +++ b/self-development/agentconfig.yaml @@ -50,6 +50,7 @@ spec: - Honor reduced-motion preferences in web UI: do not add continuous or non-essential animation without a `prefers-reduced-motion` fallback. Keep the static state distinguishable when animation is disabled; prefer gating animation under `@media (prefers-reduced-motion: no-preference)`. - Preserve readable contrast across web UI themes: when changing colors, theme tokens, or control styling, verify text, interactive labels, and focus indicators in every supported theme against the applicable WCAG AA contrast requirement. Do not assume a light-theme color remains legible on a dark background. - Audit responsive rules when changing web controls: when adding or renaming a form field or control, include it in every applicable selector group and breakpoint override. In particular, preserve mobile font sizing that prevents focus zoom, touch-target sizing, and alignment with adjacent controls. When a breakpoint hides or replaces a control, keep every supported action available through an equivalent touch-operable path and test that path. + - Preserve UI behavior when restructuring web views: when moving scroll ownership, hiding or revealing a view, or replacing a DOM container, inventory every event listener and geometry- or visibility-derived state tied to the affected elements. Bind behavior to the element that actually scrolls, do not overwrite derived state from hidden layout geometry, and recompute it when the view becomes visible. Test the affected scroll interaction and hide/reveal transition through executable UI behavior. - Preserve keyboard operability in dynamic web controls: implement the expected keyboard interaction model for custom widgets. When tabs, menu items, or similar controls use roving `tabindex`, add Arrow-key and Home/End navigation so every item remains reachable. For menus, dialogs, or actions that close or rerender their trigger, keep a clearly visible `:focus-visible` indicator, dismiss overlays through expected keyboard navigation such as Escape and Tab, and restore focus to the current replacement trigger after success and failure. Exercise keyboard activation, navigation, dismissal, and focus restoration in browser-level tests. - Do not use Gomega's global `Expect()` inside `Eventually` polling blocks: Gomega does not retry on `Expect` failures, so a transient API error short-circuits the poller. In e2e `WaitFor*` helpers, either inline the call and return a zero-value on error, or use the `Eventually(func(g Gomega) { ... })` form so failed assertions are caught and retried. - 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")`.