self-development: distill recurring conventions from PR review feedback

[kelos-bot]

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

Distills five recurring conventions from recent PR review feedback and propagates them across AGENTS.md, the shared and worker AgentConfigs, and the reviewer prompts. All changes are backed by specific PR review findings — no speculative rules.

1. Never use os.Getenv() for secrets as Go flag defaults

• Evidence: PR Remove token-refresher sidecar, generate tokens in-process #971 — P1 security findings in cmd/kelos-webhook-server/main.go and cmd/ghproxy/main.go where os.Getenv("GITHUB_TOKEN") was used as flag.StringVar default, leaking the secret in --help output.
• Applied to: AGENTS.md, agentconfig.yaml, kelos-workers.yaml, kelos-reviewer.yaml (new security checklist item).

2. Fail fast on invalid configuration

• Evidence: PR Remove token-refresher sidecar, generate tokens in-process #971 — multiple P1/P2 findings where invalid GitHub App credentials, missing token resolvers, or failed token refreshes caused silent degradation (unauthenticated requests, skipped reporting) instead of erroring out.
• Applied to: AGENTS.md, agentconfig.yaml, kelos-workers.yaml, kelos-reviewer.yaml (new correctness checklist item).

3. Keep API surfaces minimal

• Evidence: PR feat(api): Add generic webhook source for arbitrary event-driven task spawning #902 — repeated maintainer feedback on the generic webhook API: "API is hard to fix, so I'd like to find the best way to design it" / "Can we defer these fields for now?" / "Let's make the minimal API for this PR."
• Applied to: AGENTS.md, agentconfig.yaml, kelos-workers.yaml, kelos-api-reviewer.yaml (new compatibility checklist item).

4. Docs must match implementation, not aspiration

• Evidence:
  • PR docs: Document GenericWebhook TaskSpawner source #1035 (kelos-bot[bot] P1): docs claimed HMAC signature validation per source, but the GenericSource branch in internal/webhook/handler.go never reads <SOURCE>_WEBHOOK_SECRET, inspects X-Hub-Signature-256, or calls validateHMACSignature — giving users a false sense of security.
  • PR API: Add Workspace.spec.setupCommand for pre-agent setup #1056 (kelos-bot[bot] P2): doc note claimed PodOverrides.Env entries reusing reserved names are dropped so the built-in always wins, but the actual filter only drops collisions against names already populated on mainContainer.Env.
• Applied to: AGENTS.md, agentconfig.yaml, kelos-workers.yaml, kelos-reviewer.yaml (new "Documentation accuracy" checklist subsection).

5. TaskSpawner conventions

• Evidence: PR self-development: add kelos-api-reviewer TaskSpawner #965 — three distinct corrections from maintainer (gjkim42): use webhook-based triggers over poll-based; handle empty {{.Branch}} for issue events; design issue_comment prompts for both issue and PR contexts. PR Add API contract validation example (example 12) #974 — P2: "Remove the manual PR branch checkout instruction; Kelos already handles PR branch checkout automatically."
• Applied to: agentconfig.yaml, kelos-workers.yaml. README template-variable table corrected from "Usually PR head branch" to "PR head branch; empty for issue events".

Also fixes the kelos-reviewer TaskSpawner branch field to use the safe {{with index . "Branch"}}{{.}}{{else}}main{{end}} fallback form instead of bare {{.Branch}}. Flagged P2 by Greptile in this PR's prior review (#786): the spawner listens on issue_comment, so a /kelos review triggered on a plain issue would land on whatever default state Kelos falls back to rather than main.

Which issue(s) this PR is related to:

N/A

Special notes for your reviewer:

This PR was rebased onto current main (was 109 commits behind). All changes are documentation/configuration; no runtime code changes.

Does this PR introduce a user-facing change?

NONE

[cubic-dev-ai]

No issues found across 4 files

[gjkim42]

/kelos pick-up
rebase on origin/main

[kelos-bot]

🤖 Kelos Task Status

Task kelos-pr-responder-786 has succeeded. ✅

[kelos-bot]

🤖 Kelos Agent @gjkim42

/kelos needs-input

Rebased on origin/main (was already up to date). Added kind/cleanup label to fix check-pr-labels CI check. Updated PR body formatting.

The only failing CI check is comment-label — this is a repo-level permissions issue (kelos-bot[bot] lacks write access), not related to this PR. All core checks (build, test, verify, e2e, integration) passed on the current commit.

Ready for re-review.

[greptile-apps]

Greptile Summary

This PR codifies six conventions learned from recent PR review findings into AGENTS.md, agentconfig.yaml, kelos-workers.yaml, kelos-reviewer.yaml, and kelos-api-reviewer.yaml: fail-fast on invalid config, no secrets as flag defaults, minimal API surfaces, docs-must-match-implementation, no manual PR branch checkout in prompts, and correct {{.Branch}} handling for issue events. It also fixes the live branch: template in kelos-reviewer.yaml to fall back to main when {{.Branch}} is empty.

Confidence Score: 5/5

Safe to merge; all findings are P2 style suggestions that do not affect runtime behavior

All changes are documentation and agent-configuration text. The one live template fix (kelos-reviewer.yaml branch field) is correct. The only issues found are a minor inconsistency between the convention example syntax and the adopted implementation syntax, which does not affect correctness.

self-development/agentconfig.yaml and self-development/kelos-workers.yaml — convention example for {{.Branch}} uses a different template syntax than the live fix

Important Files Changed

Filename	Overview
AGENTS.md	Adds four new conventions (secrets-as-flag-defaults, fail-fast, minimal API, docs-accuracy); clean and well-worded additions
self-development/README.md	Corrects {{.Branch}} description from "usually PR head branch" to "empty for issue events"; accurate and minimal change
self-development/agentconfig.yaml	Propagates all new conventions to the worker agent config; convention example for {{.Branch}} uses a different syntax than the live fix in kelos-reviewer.yaml
self-development/kelos-reviewer.yaml	Fixes branch template to handle empty {{.Branch}} for issue events and adds correctness, security, and docs-accuracy checklist items
self-development/kelos-workers.yaml	Propagates all new conventions; same minor convention/example inconsistency as agentconfig.yaml
self-development/kelos-api-reviewer.yaml	Adds minimal-API checklist item to the API compatibility section; well-placed and concise

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[GitHub Webhook Event] --> B{Event type?}
    B -->|pull_request / pull_request_review| C["{{.Branch}} = PR head branch"]
    B -->|issue_comment on issue| D["{{.Branch}} = empty"]
    B -->|issue_comment on PR| C
    C --> E["branch: 'main' fallback not needed"]
    D --> F["branch: '{{with index . Branch}}{{.}}{{else}}main{{end}}'"]
    F --> G["Falls back to 'main'"]
    E --> H[Task spawned with correct branch]
    G --> H

Loading

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
self-development/agentconfig.yaml:40
**Convention example inconsistent with actual implementation**

The convention documents `{{if .Branch}}{{.Branch}}{{else}}main{{end}}`, but the actual fix applied in `kelos-reviewer.yaml` (line 88) uses `{{with index . "Branch"}}{{.}}{{else}}main{{end}}`. Both handle the empty-branch case, but a future author following the convention example would produce a different (though likely equivalent) template than what is already in use. Aligning the example to the adopted pattern avoids the inconsistency:

```suggestion
      - The `{{.Branch}}` template variable is empty for issue-only events; use `{{with index . "Branch"}}{{.}}{{else}}main{{end}}` when it may be empty
```

### Issue 2 of 2
self-development/kelos-workers.yaml:39
**Same convention/implementation mismatch as in agentconfig.yaml**

Same inconsistency as `agentconfig.yaml` line 40 — the example shows `{{if .Branch}}...` but the live fix in `kelos-reviewer.yaml` uses `{{with index . "Branch"}}...`. Consider using the same pattern here for consistency.

_{Reviews (2): Last reviewed commit: "self-development: distill recurring conv..." | Re-trigger Greptile}