self-development: add review-derived agent conventions#1249
Open
kelos-bot[bot] wants to merge 4 commits into
Open
self-development: add review-derived agent conventions#1249kelos-bot[bot] wants to merge 4 commits into
kelos-bot[bot] wants to merge 4 commits into
Conversation
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 <noreply@anthropic.com>
…iggers 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 <noreply@anthropic.com>
…dlers 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 <noreply@anthropic.com>
kelos-bot
Bot
force-pushed
the
kelos-config-update-latest
branch
from
July 15, 2026 18:06
09c630c to
33e9563
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Adds four evidence-backed agent conventions to the self-development AgentConfig
(
AGENTS.md/CLAUDE.md,self-development/agentconfig.yaml, andself-development/kelos-workers.yaml), each derived from recent PR reviews.1. Broaden the "vacuous substring assertions" rule to stream-forwarding tests
Motivated by PR #1189 (
Stream agent output through kelos-capture):assertForwardedonly checks substring presence; misses reordering/duplication" and recommended a byte-level comparison.cubic-dev-aiindependently flagged the same line (P2, confidence 9): "Forwarding check too weak.Containscan miss dropped/reordered/duplicated lines. Compare normalized full output instead."The existing convention was scoped narrowly to
label: valueprinter/formatterassertions; this extends it to writer/pipe forwarding tests.
2. Require author/subject filters on autonomous-run triggers
Motivated by PR #1241 (
Add kanon-development TaskSpawners), whose newevent-driven triggers shipped without author scoping. The review flagged this
twice and both fixes were applied in that PR:
kanon-pr-responder.yaml: "Add anauthorfilter to thepull_request_reviewtrigger; without it, unapproved review comments can trigger autonomous task runs."kanon-reviewer.yaml: theissue_commenttrigger was "not scoped to pull requests, so/kelos reviewon a regular issue can start the reviewer and make PR-only review commands fail."Existing conventions only noted that
issue_commentfires for both issues andPRs; they did not state the baseline that every autonomous-run trigger must be
restricted to a trusted
authorallowlist and the intended subject(
commentOn: PullRequestfor PR-only commands). This adds that rule so futurespawners ship the filters up front instead of being corrected in review.
3. Distinguish "not found" from "lookup failed" in request handlers
Motivated by PR #1238 (
Add WebhookGateway CRD), whose request handlersconflated API lookup errors with empty results. The review flagged this
repeatedly and the maintainer fixed every instance (see his summary on the PR):
listGatewayScopedSpawnersnow returns the error; the handler responds 5xx so a transient API failure is retried instead of being swallowed as a 200."Getnow returns 404 only onIsNotFound; other errors (RBAC/transient) return 500."The existing "fail fast on invalid configuration" rule covers startup
config/secrets, not request-time API reads. This adds a distinct convention so
handlers and reconcilers propagate
List/Geterrors as5xx(the senderredelivers) instead of answering
200with an empty result that silentlydrops the event.
4. Honor reduced-motion preferences in web UI
Motivated by two recent PRs that independently added continuous running-state
animation without a reduced-motion fallback:
prefers-reduced-motion: no-preferencewhile retaining the static color and halo.prefers-reduced-motionfallback, compounding the existing accessibility gap in other pulsing elements.The shared configuration had no motion-accessibility guidance. This adds a
focused rule requiring continuous or non-essential animation to honor
prefers-reduced-motionand keep a distinguishable static state when motionis disabled.
Files updated (the canonical agent-rule locations;
CLAUDE.mdis a symlink toAGENTS.md):AGENTS.md/CLAUDE.mdself-development/agentconfig.yamlself-development/kelos-workers.yamlWhich issue(s) this PR is related to:
N/A
Special notes for your reviewer:
Convention-only change; no code or runtime behavior is affected. The branch was
rebased onto current
main, andmake verifypasses.Does this PR introduce a user-facing change?