diff --git a/AGENTS.md b/AGENTS.md index 63a4cb11..64599a1c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -4,22 +4,41 @@ - **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. - **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. +- **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 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. +- **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)`. +- **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")`. -- **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. +- **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. +- **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 6f93c08c..17c355a4 100644 --- a/self-development/agentconfig.yaml +++ b/self-development/agentconfig.yaml @@ -26,21 +26,40 @@ 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 - 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. + - 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 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)`. + - 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")`. - - 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. + - 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. + - 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. @@ -48,4 +67,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