self-development: distill recurring conventions from PR review feedback

AGENTS.md

@@ -8,6 +8,11 @@
 - **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.
+- **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
 - `make verify` — run all verification checks (lint, fmt, vet, etc.).

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 * * * *"`) |

self-development/agentconfig.yaml

@@ -31,3 +31,13 @@ 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
+    - 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
+      - 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

self-development/kelos-api-reviewer.yaml

@@ -171,6 +171,19 @@ 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.
+      - 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?

self-development/kelos-reviewer.yaml

@@ -85,7 +85,7 @@ spec:
           ephemeral-storage: "4Gi"
         limits:
           ephemeral-storage: "4Gi"
-    branch: "{{.Branch}}"
+    branch: '{{with index . "Branch"}}{{.}}{{else}}main{{end}}'
     agentConfigRef:
       name: kelos-reviewer-agent
     promptTemplate: |
@@ -148,6 +148,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?
@@ -165,6 +169,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

self-development/kelos-workers.yaml

@@ -29,6 +29,17 @@ 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
+    - 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
+      - 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