Skip to content

self-development: add review-derived agent conventions#1249

Open
kelos-bot[bot] wants to merge 4 commits into
mainfrom
kelos-config-update-latest
Open

self-development: add review-derived agent conventions#1249
kelos-bot[bot] wants to merge 4 commits into
mainfrom
kelos-config-update-latest

Conversation

@kelos-bot

@kelos-bot kelos-bot Bot commented May 30, 2026

Copy link
Copy Markdown
Contributor

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, and
self-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):

  • The Kelos Reviewer flagged (P3): "assertForwarded only checks substring presence; misses reordering/duplication" and recommended a byte-level comparison.
  • cubic-dev-ai independently flagged the same line (P2, confidence 9): "Forwarding check too weak. Contains can miss dropped/reordered/duplicated lines. Compare normalized full output instead."
  • The landed fix compares the forwarded stream byte-for-byte.

The existing convention was scoped narrowly to label: value printer/formatter
assertions; 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 new
event-driven triggers shipped without author scoping. The 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 can start the reviewer and make PR-only review commands fail."

Existing conventions only noted that issue_comment fires for both issues and
PRs; they did not state the baseline that every autonomous-run trigger must be
restricted to a trusted author allowlist and the intended subject
(commentOn: PullRequest for PR-only commands). This adds that rule so future
spawners 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 handlers
conflated API lookup errors with empty results. The review flagged this
repeatedly and the maintainer fixed every instance (see his summary on the PR):

  • P1 (confidence 9): "TaskSpawner list errors are treated as an empty match set, so transient API failures can silently drop webhooks with a 200 response." Fix: "listGatewayScopedSpawners now returns the error; the handler responds 5xx so a transient API failure is retried instead of being swallowed as a 200."
  • Fix: "gateway Get now returns 404 only on IsNotFound; 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/Get errors as 5xx (the sender
redelivers) instead of answering 200 with an empty result that silently
drops 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:

The shared configuration had no motion-accessibility guidance. This adds a
focused rule requiring continuous or non-essential animation to honor
prefers-reduced-motion and keep a distinguishable static state when motion
is disabled.

Files updated (the canonical agent-rule locations; CLAUDE.md is a symlink to AGENTS.md):

  • AGENTS.md / CLAUDE.md
  • self-development/agentconfig.yaml
  • self-development/kelos-workers.yaml

Which 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, and make verify passes.

Does this PR introduce a user-facing change?

NONE

@github-actions github-actions Bot added kind/cleanup needs-triage needs-kind Indicates an issue or PR lacks a kind/* label needs-priority needs-actor release-note-none and removed needs-kind Indicates an issue or PR lacks a kind/* label labels May 30, 2026
@kelos-bot kelos-bot Bot changed the title self-development: broaden substring-assertion convention to stream-forwarding tests self-development: add stream-forwarding and trigger-filter conventions from recent reviews May 31, 2026
@kelos-bot kelos-bot Bot changed the title self-development: add stream-forwarding and trigger-filter conventions from recent reviews self-development: add stream-forwarding, trigger-filter, and handler-error conventions from recent reviews Jun 1, 2026
gjkim42 and others added 4 commits July 15, 2026 18:03
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
kelos-bot Bot force-pushed the kelos-config-update-latest branch from 09c630c to 33e9563 Compare July 15, 2026 18:06
@kelos-bot kelos-bot Bot changed the title self-development: add stream-forwarding, trigger-filter, and handler-error conventions from recent reviews self-development: add review-derived agent conventions Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant