From 6f8e7d3e45888f3b6841b17e3f66997ecd55e4a1 Mon Sep 17 00:00:00 2001 From: Gunju Kim Date: Fri, 1 May 2026 18:09:06 +0000 Subject: [PATCH 1/3] self-development: distill recurring conventions from PR review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds five conventions distilled from recent PR reviews and applies them across AGENTS.md, the shared and worker AgentConfigs, and the reviewer prompts. 1. Never use os.Getenv() for secrets as Go flag defaults (PR #971) — flag prints defaults in --help output, leaking secret values. 2. Fail fast on invalid configuration (PR #971) — do not silently fall back to unauthenticated/degraded behavior when credentials or config are missing. 3. Keep API surfaces minimal (PR #902) — only fields immediately needed, no speculative additions; API is hard to change once shipped. 4. Docs must match implementation, not aspiration (PRs #1035, #1056) — describe only what the code actually does; verify enforcement before documenting a contract. 5. TaskSpawner conventions (PRs #965, #974): - Prefer webhook-based triggers over poll-based. - {{.Branch}} is empty for issue-only events; use the {{if .Branch}}{{.Branch}}{{else}}main{{end}} fallback form. - issue_comment fires for both issues and PRs; design prompts to detect and handle both contexts. - Do not include manual PR branch checkout instructions — Kelos checks out the PR branch automatically. Also fixes the kelos-reviewer TaskSpawner branch field to use the safe fallback form (was using bare {{.Branch}}, which is empty for issue events; flagged P2 in PR #786 review). Co-Authored-By: Claude Opus 4.7 --- AGENTS.md | 4 ++++ self-development/README.md | 2 +- self-development/agentconfig.yaml | 9 +++++++++ self-development/kelos-api-reviewer.yaml | 4 ++++ self-development/kelos-reviewer.yaml | 18 +++++++++++++++++- self-development/kelos-workers.yaml | 10 ++++++++++ 6 files changed, 45 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 19ab801a..0fbe063d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -8,6 +8,10 @@ - **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). +- **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. +- **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. +- **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. ## Key Makefile Targets - `make verify` — run all verification checks (lint, fmt, vet, etc.). diff --git a/self-development/README.md b/self-development/README.md index 0a5d1bca..bc1466cb 100644 --- a/self-development/README.md +++ b/self-development/README.md @@ -423,7 +423,7 @@ To adapt these examples for your own repository: | `{{.Event}}` | GitHub webhook event type | `issue_comment`, `issues`, `pull_request_review`, etc. | Empty | | `{{.Action}}` | GitHub webhook action | `created`, `labeled`, `submitted`, etc. | Empty | | `{{.Sender}}` | GitHub username that triggered the webhook | GitHub login | Empty | - | `{{.Branch}}` | Branch name when present in the webhook payload | Usually PR head branch or push branch | Empty | + | `{{.Branch}}` | Branch name when present in the webhook payload | PR head branch; empty for issue events | Empty | | `{{.Kind}}` | Type of work item | `"webhook"` | `"Issue"` | | `{{.Time}}` | Trigger time (RFC3339) | Empty | Cron tick time (e.g., `"2026-02-07T09:00:00Z"`) | | `{{.Schedule}}` | Cron schedule expression | Empty | Schedule string (e.g., `"0 * * * *"`) | diff --git a/self-development/agentconfig.yaml b/self-development/agentconfig.yaml index b08f63a8..221411bd 100644 --- a/self-development/agentconfig.yaml +++ b/self-development/agentconfig.yaml @@ -31,3 +31,12 @@ 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` + - 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 + - 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 + - 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. + - TaskSpawner conventions (for `self-development/` YAML files): + - 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 `{{if .Branch}}{{.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 + - Do not include manual PR branch checkout instructions in prompts — Kelos already checks out the PR branch automatically diff --git a/self-development/kelos-api-reviewer.yaml b/self-development/kelos-api-reviewer.yaml index ce686a9f..8d35f5e4 100644 --- a/self-development/kelos-api-reviewer.yaml +++ b/self-development/kelos-api-reviewer.yaml @@ -172,6 +172,10 @@ spec: - Are there breaking changes to existing field semantics? - Is the field or type correctly versioned (alpha/beta/stable)? - Are deprecated fields marked with `+deprecated` and documented? + - Is the API surface minimal — only fields immediately needed, no + speculative additions? API is hard to change once shipped, so + push back on fields that anticipate future use without a concrete + caller. **CRD and validation**: - Are kubebuilder validation markers correct and complete? diff --git a/self-development/kelos-reviewer.yaml b/self-development/kelos-reviewer.yaml index 03902fd6..b0fcd145 100644 --- a/self-development/kelos-reviewer.yaml +++ b/self-development/kelos-reviewer.yaml @@ -86,7 +86,7 @@ spec: ephemeral-storage: "4Gi" limits: ephemeral-storage: "4Gi" - branch: "{{.Branch}}" + branch: '{{with index . "Branch"}}{{.}}{{else}}main{{end}}' agentConfigRef: name: kelos-reviewer-agent promptTemplate: | @@ -149,6 +149,10 @@ spec: - Are there edge cases, off-by-one errors, or nil pointer risks? - Are error returns checked and handled? - Are concurrent operations safe (locks, channels, context cancellation)? + - Does the code fail fast on invalid configuration, or does it silently + fall back to degraded behavior (e.g., unauthenticated requests, nil + clients) when credentials or config are missing? Silent fallbacks + mask failures and should be flagged. **Tests**: - Are there tests for the new/changed behavior? @@ -166,6 +170,18 @@ spec: - No hardcoded secrets or credentials - Input validation at system boundaries - No command injection, path traversal, or OWASP top-10 risks + - No `os.Getenv()` of a secret used as a Go `flag` default — Go's + `flag` package prints defaults in `--help` output, which leaks + secret values. Flag the pattern even when the flag is otherwise + well-named. + + **Documentation accuracy**: + - Do new or updated docs, READMEs, and code comments describe only what + the code actually does? Aspirational claims (security checks that + aren't enforced, contracts that are partial, fields that aren't + wired up) should be flagged. Before approving a doc change that + describes a contract ("X is filtered", "Y is validated"), confirm + the diff or surrounding code actually enforces it. **Code quality**: - No unnecessary code, comments, or dead variables diff --git a/self-development/kelos-workers.yaml b/self-development/kelos-workers.yaml index 4d764c0a..b81ad3b5 100644 --- a/self-development/kelos-workers.yaml +++ b/self-development/kelos-workers.yaml @@ -29,6 +29,16 @@ spec: - Always try to add or improve tests 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 + - 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 + - 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 + - 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. + - TaskSpawner conventions (for `self-development/` YAML files): + - 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 `{{if .Branch}}{{.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 + - Do not include manual PR branch checkout instructions in prompts — Kelos already checks out the PR branch automatically --- apiVersion: kelos.dev/v1alpha1 kind: TaskSpawner From 0250f91df4e31b8e7a999bcc092a75015f341abb Mon Sep 17 00:00:00 2001 From: Gunju Kim Date: Sat, 2 May 2026 18:05:06 +0000 Subject: [PATCH 2/3] self-development: add API backward-compatibility convention PR #1058 surfaced a recurring API-change pattern not covered by the existing "Keep API surfaces minimal" rule: - A scalar -> array kind change on an existing CRD field (BodyContains) was flagged P1 because existing in-cluster resources would fail structural-schema validation; the repo itself had ~9 stale YAMLs in self-development/ and examples/ still using the old scalar form. - The maintainer asked twice about backward compatibility on a newly added BodyPattern field, and required removing +kubebuilder:validation:MinLength=1 because it would reject existing YAMLs that don't yet have the field. - The agreed migration path was to add new fields and mark the old one +deprecated rather than remove it. Adds the convention to AGENTS.md (and CLAUDE.md via symlink), to the shared agentconfig.yaml and kelos-workers.yaml, and to the kelos-api-reviewer prompt as an explicit checklist item under "API compatibility and evolution". Co-Authored-By: Claude Opus 4.7 --- AGENTS.md | 1 + self-development/agentconfig.yaml | 1 + self-development/kelos-api-reviewer.yaml | 9 +++++++++ self-development/kelos-workers.yaml | 1 + 4 files changed, 12 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 0fbe063d..4e6e5c4e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -11,6 +11,7 @@ - **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. - **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. - **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. ## Key Makefile Targets diff --git a/self-development/agentconfig.yaml b/self-development/agentconfig.yaml index 221411bd..c9a2b67c 100644 --- a/self-development/agentconfig.yaml +++ b/self-development/agentconfig.yaml @@ -34,6 +34,7 @@ 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 - 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. - TaskSpawner conventions (for `self-development/` YAML files): - Prefer webhook-based triggers (`githubWebhook`) over poll-based (`githubPullRequests`) for real-time event-driven tasks diff --git a/self-development/kelos-api-reviewer.yaml b/self-development/kelos-api-reviewer.yaml index 8d35f5e4..46fb8fcb 100644 --- a/self-development/kelos-api-reviewer.yaml +++ b/self-development/kelos-api-reviewer.yaml @@ -176,6 +176,15 @@ spec: speculative additions? API is hard to change once shipped, so push back on fields that anticipate future use without a concrete caller. + - Will existing in-cluster resources still apply after this change? + Flag scalar ↔ array kind changes on existing fields, newly added + `MinLength`/`Required`/tightened validation on fields that + previously accepted absence/empty values, and field removals + without a `+deprecated` transition. Replace by deprecation, not + deletion. + - Were stale manifests in `examples/` and `self-development/` + updated to the new form when the schema changed? An unswept tree + leaves users with broken YAMLs after the CRD upgrade. **CRD and validation**: - Are kubebuilder validation markers correct and complete? diff --git a/self-development/kelos-workers.yaml b/self-development/kelos-workers.yaml index b81ad3b5..547ce8e2 100644 --- a/self-development/kelos-workers.yaml +++ b/self-development/kelos-workers.yaml @@ -33,6 +33,7 @@ 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 - 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. - TaskSpawner conventions (for `self-development/` YAML files): - Prefer webhook-based triggers (`githubWebhook`) over poll-based (`githubPullRequests`) for real-time event-driven tasks From bdbd4389d1819811be2d5120af69888debb02df7 Mon Sep 17 00:00:00 2001 From: Gunju Kim Date: Sun, 3 May 2026 18:05:30 +0000 Subject: [PATCH 3/3] self-development: align Branch template doc form, add Eventually testing convention MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Greptile flagged a P2 inconsistency on PR #786 itself: the convention text in agentconfig.yaml and kelos-workers.yaml prescribed {{if .Branch}}{{.Branch}}{{else}}main{{end}}, but the canonical form used in the spawner YAMLs (kelos-reviewer.yaml, kelos-api-reviewer.yaml) is {{with index . "Branch"}}{{.}}{{else}}main{{end}}. The new "Docs must match implementation" rule applies to itself — fix the documented example to match the deployed form. PR #1084 fixed an e2e flake caused by GetTaskPhase calling the global Expect(err).NotTo(HaveOccurred()) inside an Eventually polling loop; Gomega does not retry on Expect failures, so a transient API error short-circuits the poller. Both Greptile and cubic-dev-ai flagged the same anti-pattern. Capture the lesson in AGENTS.md, the shared and worker AgentConfigs, and add a reviewer checklist item so future WaitFor* helpers either return a zero-value on error or use the Eventually(func(g Gomega) { ... }) form. --- AGENTS.md | 1 + self-development/agentconfig.yaml | 3 ++- self-development/kelos-reviewer.yaml | 6 ++++++ self-development/kelos-workers.yaml | 3 ++- 4 files changed, 11 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 4e6e5c4e..cc8a75b9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -13,6 +13,7 @@ - **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. - **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. +- **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. ## Key Makefile Targets - `make verify` — run all verification checks (lint, fmt, vet, etc.). diff --git a/self-development/agentconfig.yaml b/self-development/agentconfig.yaml index c9a2b67c..cb873d20 100644 --- a/self-development/agentconfig.yaml +++ b/self-development/agentconfig.yaml @@ -36,8 +36,9 @@ 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. + - 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. - TaskSpawner conventions (for `self-development/` YAML files): - 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 `{{if .Branch}}{{.Branch}}{{else}}main{{end}}` when it may be empty + - 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 - Do not include manual PR branch checkout instructions in prompts — Kelos already checks out the PR branch automatically diff --git a/self-development/kelos-reviewer.yaml b/self-development/kelos-reviewer.yaml index b0fcd145..158e226b 100644 --- a/self-development/kelos-reviewer.yaml +++ b/self-development/kelos-reviewer.yaml @@ -159,6 +159,12 @@ spec: - Do the tests cover edge cases and error paths? - Are test assertions checking the right things? - Review test adequacy from the diff and surrounding code; do not rerun validation as part of the review + - In Ginkgo/Gomega e2e helpers, flag uses of the global `Expect()` (or + `ExpectWithOffset`) inside an `Eventually` polling function. Gomega + does not retry on `Expect` failures, so a transient API error + short-circuits the poller. The polling body should either return a + zero-value on error or use the `Eventually(func(g Gomega) { ... })` + form so failed assertions are caught and retried. **Project conventions**: - Logging: messages start with capital letters, do not end with punctuation diff --git a/self-development/kelos-workers.yaml b/self-development/kelos-workers.yaml index 547ce8e2..a9c66a08 100644 --- a/self-development/kelos-workers.yaml +++ b/self-development/kelos-workers.yaml @@ -35,9 +35,10 @@ 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. + - 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. - TaskSpawner conventions (for `self-development/` YAML files): - 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 `{{if .Branch}}{{.Branch}}{{else}}main{{end}}` when it may be empty + - 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 - Do not include manual PR branch checkout instructions in prompts — Kelos already checks out the PR branch automatically ---