docs: improve harness and issue templates (#1681)

This commit is contained in:
Carlos Florêncio
2026-07-14 17:58:21 +01:00
committed by GitHub
parent 39ee058f0c
commit 6831b80878
8 changed files with 62 additions and 5 deletions
+30
View File
@@ -21,6 +21,21 @@ Determine the right diff scope:
- **Local changes**: `git diff --name-only` (unstaged) and `git diff --cached --name-only` (staged)
- **PR review**: `git diff origin/<base_branch>...HEAD --name-only` to diff against the base branch
For an existing PR, first confirm the exact head under review. Do not assume the
local checkout is current: inspect the PR's base branch and head SHA, fetch the
head if needed, and use that immutable SHA in the diff. If the current PR head
cannot be fetched, say so rather than reporting a stale checkout as a review of
the current PR.
Record the base and head SHA for each review round. A new contributor push starts
a new round: reassess prior findings and the verdict against the new head, and
verify checks or workflow results for that head rather than relying on a PR
number or author summary.
Compare the PR description, checklist, and claimed manual validation with that
exact head, especially after a major refactor. Report stale claims separately;
they are not verification evidence for the current diff.
Read each changed file in full — understand surrounding code, not just the diff. Navigate callers, interfaces, and tests to understand changes end-to-end.
For each file, identify which requirement or intent it serves. Flag any changes that don't map to the task — scope creep is a blocker.
@@ -32,6 +47,8 @@ Before reviewing implementation details:
- Check whether tests assert behavior, not implementation details.
- Check whether the selected test level is appropriate: unit for pure logic, integration for boundaries, E2E for critical browser flows.
- Identify missing coverage for happy path, key error paths, edge cases, auth/workspace boundaries, and concurrency/order-sensitive behavior.
- For concurrent or event-driven changes, require a deterministic schedule that checks ownership or generation identity, stale-event handling, cancellation, and lock scope. Channel/barrier coordination is preferable to timing sleeps.
- For stale-event races, cover both event-before-successor and delayed-old-event-after-successor orderings. Prefer integration coverage for cross-package event or callback paths when practical.
- Treat missing tests for new or changed non-UI logic as a blocker unless the change is explicitly untestable and says why.
### 3. Review for issues
@@ -50,6 +67,7 @@ Check every changed file for the following layers. Skip layers that don't apply
**Architecture:**
- Frontend: no direct data fetching in components (must go through store), shadcn imports from `@kandev/ui` not `@/components/ui/*`
- Backend: provider pattern for DI, context passed through call chains, event bus for cross-component communication
- Search `docs/specs/` and `docs/decisions/` for the affected subsystem; flag an accepted spec or ADR that the change makes inaccurate
- New abstractions justified — no over-engineering
- Concerns cleanly separated (single responsibility)
@@ -57,6 +75,11 @@ Check every changed file for the following layers. Skip layers that don't apply
- Edge cases handled (empty input, nil/null, zero, max values)
- Error paths covered and not silently swallowed
- Race conditions or concurrency issues in concurrent code
- Async events carry an immutable identity when they can outlive the operation that created them; stale events cannot mutate a replacement operation
- Locks protect only the atomic ownership boundary and are not held across unbounded I/O or a full asynchronous operation
- Synchronous callbacks cannot re-enter a lock they already need; moving publication asynchronous also requires an immutable value snapshot, clear shutdown ownership, and protection against a delayed event changing successor state
- When a generation, token, or lease authorizes a side effect, validate and mutate within one critical section. Check every terminal path separately: success, raw error, cancellation, timeout, and disconnect.
- Detached goroutines have immutable snapshots and a real happens-before relationship before reading state that can otherwise transition underneath them
**Performance:**
- No N+1 queries (loop with individual DB calls)
@@ -93,6 +116,13 @@ Check every changed file for the following layers. Skip layers that don't apply
### 4. Fix or report
When the user says not to post or modify the PR, do not make any GitHub
mutation: no fixes, comments, review submissions, or thread resolution. When
the user asks for a review only, or when reviewing an external contributor's
branch, do not edit the checkout or push code; report findings through the
channel the user requested. Do not submit or resolve reviews unless explicitly
asked.
- **Fix directly** any issues you can resolve confidently (dead code, unused imports, simple duplication, missing early returns)
- **Report** issues that need the author's input — always explain *why* the issue matters and provide a concrete suggested fix
+2
View File
@@ -44,6 +44,7 @@ If task has no UI surface, say why this skill does not apply and continue.
- In this repo, name mobile test files `mobile-*.spec.ts` so the `mobile-chrome` Playwright project picks them up automatically.
- If no mobile project exists, configure the test or project with a realistic mobile viewport plus touch/mobile settings.
- Cover mobile-specific controls such as drawers, overflow menus, stacked actions, responsive tables, or bottom controls.
- When a touch-only control is replaced or hidden, run `rg` across mobile E2E tests for the removed control. Replace every affected interaction with the intended gesture or alternate control, then run those tests together.
5. Verify visually and behaviorally.
- Run the narrowest relevant viewport locally or with screenshots when possible.
@@ -98,6 +99,7 @@ test.describe("feature on mobile", () => {
- Mobile path has designed layout and interaction behavior.
- Required controls are reachable by touch.
- No required workflow depends on hover, wide viewport, or hidden desktop-only UI.
- Mobile E2E tests no longer invoke touch controls that the change replaced or hid.
- Mobile Playwright coverage exists or absence is justified.
- Focused rendered/visual verification was run for UI tweaks, or exact "not run" reason is reported.
- Focused tests were run, or exact blocker is reported.
+4 -2
View File
@@ -17,7 +17,7 @@ Wait for CI and code review to complete on a pull request, fix any failures or v
- **`/e2e`** — Read for debugging guidance when E2E tests fail in CI. Covers test patterns, run commands, failure triage, and local reproduction.
- **`/commit`** — Use for staging and committing fixes with Conventional Commits format.
Prefer the `pr-poller` and `verify` helpers when the runtime supports delegated helpers. If runtime policy forbids delegated helpers/subagents unless explicitly requested by the user, treat helper delegation as unavailable and use the direct-command fallback. Do not substitute a generic/general subagent for polling; it returns too slowly for the 30s cadence and can look hung. If helper delegation is unavailable, follow the direct-command fallback sections below and keep the same output contract: compact CI state, compact review state, bounded polling, and full local verification before pushing.
Prefer the `pr-poller` and `verify` helpers when the runtime supports delegated helpers. If runtime policy forbids delegated helpers/subagents unless explicitly requested by the user, or either helper fails to start or initialize, treat helper delegation as unavailable and use the direct-command fallback. Do not substitute a generic/general subagent for polling; it returns too slowly for the 30s cadence and can look hung. If helper delegation is unavailable, follow the direct-command fallback sections below and keep the same output contract: compact CI state, compact review state, bounded polling, and full local verification before pushing.
## Context
@@ -28,7 +28,9 @@ Prefer the `pr-poller` and `verify` helpers when the runtime supports delegated
The first thing you do — before fetching PR state, before reading logs, before any fixes — is create a task list for the full pipeline. This is non-negotiable because it keeps you accountable to the process and lets the user see where you are.
Create these tasks immediately (use your task/todo tracking tool if available):
Create these tasks immediately in the current session's todo/checklist tool. Do
not create Kandev subtasks or persistent work items unless the user explicitly
requests task tracking:
1. **Gather PR state** — Use `pr-poller` when available; otherwise gather compact CI + bot review state via `scripts/pr-state`; always check PR mergeability and local conflict state
2. **Fix failing CI checks** — Read failing run logs (via `scripts/run-quiet gh-run -- gh run view ...`), fix issues, run E2E tests locally if needed
+13
View File
@@ -43,6 +43,19 @@ Choose the right level:
Prefer state/output assertions over interaction assertions. Mock only slow, nondeterministic, or external boundaries; use real implementations or fakes when they keep the test deterministic.
### Concurrent and event-driven behavior
Test ordering-sensitive behavior with channels, barriers, or controllable fakes;
do not use sleeps to create a race, except for a bounded, named delay that
models a known poll-loop schedule when synchronization would alter that
relationship. Pause at the ownership boundary, start the
competing operation, then release. Exercise the real delivery path where
practical, and prove the old interleaving fails before the fix. Assert both the
winner state and the untouched replacement state, including relevant buffers,
signals, or queue ownership. Cover stale events acting after a replacement
operation begins, cancellation/retry ownership, and at-most-once delivery when
they apply. Run affected Go packages with `-race`.
## Steps
### 1. RED — Write a failing test
+1 -1
View File
@@ -7,7 +7,7 @@ description: Run format, typecheck, test, and lint across the monorepo. Use afte
Delegate to the **`verify` subagent** to run the full verification pipeline (rebase, format, typecheck, test, lint) and fix any issues it finds when the runtime supports delegated helpers. The subagent runs on Sonnet, which is cheaper than the main session and well-suited to the mechanical run-parse-fix loop.
If runtime policy forbids delegated helpers/subagents unless the user explicitly requested them, treat delegation as unavailable and use the direct-command fallback below. Do not stop at a partial check just because delegation is unavailable.
If runtime policy forbids delegated helpers/subagents unless the user explicitly requested them, or the helper fails to start or initialize, treat delegation as unavailable and use the direct-command fallback below. Do not stop at a partial check just because delegation is unavailable.
## What to do
+1 -1
View File
@@ -1,7 +1,7 @@
name: Bug report
description: Report a reproducible problem in Kandev.
title: "[Bug]: "
labels: ["bug", "needs-triage"]
labels: ["bug"]
body:
- type: markdown
attributes:
+1 -1
View File
@@ -1,7 +1,7 @@
name: Feature request
description: Propose a Kandev improvement with clear user value.
title: "[Feature]: "
labels: ["enhancement", "needs-triage"]
labels: ["enhancement"]
body:
- type: markdown
attributes:
+10
View File
@@ -83,6 +83,10 @@ Every code change must include tests for new or changed logic. Backend: `*_test.
### GitHub Operations
Skills use `gh` CLI by default. If a `gh` command fails (not installed, not authenticated, etc.), use whatever GitHub tools are available in the environment (MCP GitHub tools, API tools, etc.) to accomplish the same operation. The goal is the same — the tool may differ.
For multiline Markdown issue or PR bodies, write the body to a file and pass it
with the relevant `gh ... --body-file <path>` option. Do not send escaped
newlines through `--body`; GitHub will render them literally.
For PR review/fixup workflows, prefer the repo helpers before manually querying GitHub/GraphQL: `scripts/pr-state --summary <PR>` for checks and unresolved-thread state, `scripts/pr-state --comment <comment_id>` for a full review-comment body, `scripts/pr-resolve list <PR>` for actionable unresolved review threads, and `scripts/pr-resolve reply <PR> <comment_id> <thread_id> "<body>"` to reply, resolve, and react in one call.
When a Kandev system message references an MCP tool that is not visible in the active tool list, use the runtime's tool discovery mechanism, such as `tool_search` when available, before falling back to a less specific workflow. Some task messaging and platform helpers are exposed on demand.
@@ -91,6 +95,12 @@ When a Kandev system message references an MCP tool that is not visible in the a
When creating follow-up or delegated Kandev work from an active task, use `create_task_kandev` with `parent_id: "self"` when the work is related. That preserves workspace, workflow, repository, agent profile, and executor context from the current task. For genuinely unrelated top-level tasks, do not rely on workspace defaults when the user expects continuity; explicitly preserve the current task's `agent_profile_id` / `executor_profile_id`, or ask if the intended profile is ambiguous.
For remediation discovered while reviewing a PR that must start after the PR
merges, create a related subtask with `parent_id: "self"`,
`workspace_mode: "new_workspace"`, and the reviewed PR's base branch; otherwise
a same-repository subtask inherits the reviewed branch. Set `start_agent: false`
when the follow-up is intentionally queued until merge.
### Third-party integrations
Jira and Linear are the model (per-workspace credentials, 90s auth-health poller via `internal/integrations/healthpoll`, settings page with status banner). New integrations should **reuse the shared shapes** rather than copying either. Full layout, file conventions, and Jira-vs-Linear divergence notes in `apps/backend/internal/integrations/AGENTS.md` and the `/add-integration` skill — load either when scaffolding a new integration.