chore: improve harness verification guidance (#1777)
This commit is contained in:
Carlos Florêncio
+32
-38
@@ -12,50 +12,44 @@ Run the full verification pipeline for the monorepo, then fix any issues found.
|
||||
|
||||
## Steps
|
||||
|
||||
1. **Rebase on main** (ensures CI parity — CI merges main into the PR branch):
|
||||
```bash
|
||||
git fetch origin main --quiet
|
||||
```
|
||||
- If the current branch is `main`, skip this step.
|
||||
- If the branch is stacked on another feature branch (not main), skip this step.
|
||||
Detect: `git rev-parse --abbrev-ref --symbolic-full-name @{upstream} 2>/dev/null` — if the upstream is NOT `origin/main` (e.g., another feature branch), skip rebase.
|
||||
- Otherwise, rebase:
|
||||
1. **Prepare and rebase against the current PR base:**
|
||||
- Fresh worktrees share `.git/` but not dependencies. If `apps/node_modules` is missing, run `pnpm install --frozen-lockfile` from `apps/`.
|
||||
- Resolve the PR base from GitHub because stacked PRs may not target `main` and can be retargeted after a parent merges:
|
||||
```bash
|
||||
git rebase origin/main
|
||||
PR_BASE="$(gh pr view --json baseRefName --jq .baseRefName 2>/dev/null || true)"
|
||||
if [[ -n "$PR_BASE" ]]; then git fetch origin "$PR_BASE" --quiet; fi
|
||||
```
|
||||
- If the rebase has conflicts, resolve them:
|
||||
1. For each conflicted file, read the file and understand both sides of the conflict
|
||||
2. Resolve by keeping the correct combination of both changes (don't just pick one side)
|
||||
3. `git add <resolved-file>` then `git rebase --continue`
|
||||
4. Repeat for each conflicting commit until the rebase completes
|
||||
5. If a conflict is too ambiguous to resolve confidently, abort (`git rebase --abort`) and report the issue
|
||||
- If the PR base is unavailable, skip rebasing and report it. Otherwise, if the current branch is not the base branch, rebase onto `origin/$PR_BASE`.
|
||||
- Resolve conflicts by preserving the intended behavior from both sides. Stage each resolved file and continue. If a conflict is ambiguous, abort and report it rather than guessing.
|
||||
|
||||
2. **Format** (prevents formatter-induced lint failures):
|
||||
- `make -C apps/backend fmt`
|
||||
- `cd apps && pnpm format`
|
||||
|
||||
3. **Verify** — always wrap heavy commands with `scripts/run-quiet <tag> -- <cmd>`. The wrapper redirects all output to a unique `/tmp/kandev-run.<tag>.<random>.log`, prints `exit=0 log=...` on success or an auto-grepped failure summary on error, and exits with the underlying command's exit code. Random suffix ⇒ safe under parallel invocations.
|
||||
2. **Format and generate metadata:**
|
||||
```bash
|
||||
scripts/run-quiet backend-test -- make -C apps/backend test lint
|
||||
scripts/run-quiet web-lint -- bash -c 'cd apps && pnpm --filter @kandev/web lint'
|
||||
scripts/run-quiet format -- make fmt
|
||||
git status --short
|
||||
node apps/web/scripts/generate-release-notes.mjs
|
||||
node apps/web/scripts/generate-changelog.mjs
|
||||
```
|
||||
- If both print `exit=0`: done, nothing more to read.
|
||||
- If non-zero: the failure summary is already in your context. `Read` only specific line ranges of the printed log path if you need more context — never `cat` the whole log.
|
||||
Review formatter changes before continuing.
|
||||
|
||||
4. **Fix issues** — do NOT just report them:
|
||||
- Read each failing file at the reported line number (use the `Read` tool with `offset`/`limit`, not `cat` of the whole file)
|
||||
- Fix the root cause (don't suppress warnings or add ignores)
|
||||
- Common fixes:
|
||||
- **Type errors**: fix the type, add a missing import, or correct the function signature
|
||||
- **Lint — function too long**: extract a helper function or sub-component
|
||||
- **Lint — file too long**: split the file into smaller, cohesive files grouped by responsibility
|
||||
- **Lint — cyclomatic/cognitive complexity**: simplify conditionals, extract early returns, break into smaller functions
|
||||
- **Lint — unused imports**: remove them
|
||||
- **Lint — duplicate strings**: extract to a constant
|
||||
- **Test failures**: read the test, understand the assertion, fix the code (not the test) unless the test is outdated
|
||||
- **Test failures — goleak (package passes, TestMain fails):** loop the failing package with race to match CI: `go test -race -count=10 ./internal/<pkg>/...`. If stack traces mention `sleepOrStop` / `connectWorkspaceStream`, fix test cleanup (see backend AGENTS.md leak-testing section).
|
||||
- After fixing, re-run **only** the failed command via `scripts/run-quiet` to confirm the fix. Do not re-run the whole pipeline on every iteration.
|
||||
3. **Run the complete pipeline** through `scripts/run-quiet`:
|
||||
```bash
|
||||
scripts/run-quiet typecheck -- make typecheck
|
||||
scripts/run-quiet test -- make test
|
||||
scripts/run-quiet lint -- make lint
|
||||
```
|
||||
- `make test` covers backend, web, CLI, `test-scripts`, and desktop smoke checks. Do not skip a failed subtarget while reporting full verification as green.
|
||||
- If a command fails, inspect only targeted ranges of the returned log path; never stream or `cat` the entire log.
|
||||
|
||||
4. **Fix issues** - do not just report them:
|
||||
- Read each failing file at the reported line number.
|
||||
- Fix the root cause; do not suppress warnings or add ignores.
|
||||
- For type, lint, or test failures, fix implementation behavior unless the test is demonstrably outdated.
|
||||
- For goleak failures after otherwise passing Go tests, loop the affected package with `go test -race -count=10 ./internal/<pkg>/...` and inspect cleanup ownership.
|
||||
- If Go tests fail because `httptest.NewServer` cannot bind loopback in a restricted sandbox, rerun the exact command with normal network/loopback escalation before diagnosing code.
|
||||
- For `ENOSPC`, read-only cache, or cache-initialization failures, create invocation-specific writable `TMPDIR`, `GOCACHE`, and `GOLANGCI_LINT_CACHE` directories and rerun the exact command. Clean only those directories.
|
||||
- For Rust/Tauri changes, compare `rustc --version` with `apps/desktop/src-tauri/Cargo.toml`'s `rust-version`. Activate a matching installed toolchain without replacing `PATH`; report or request installation if unavailable.
|
||||
- After fixing, rerun only the failed command through `scripts/run-quiet`.
|
||||
|
||||
5. **Repeat** steps 3-4 until all commands pass. If a fix introduces new issues, address those too.
|
||||
|
||||
6. **Done** when all pass cleanly: rebase, fmt, typecheck, test, lint.
|
||||
6. **Done** only when rebase, format, metadata generation, typecheck, the complete test target, lint, and any scoped Rust tests all pass.
|
||||
|
||||
@@ -63,6 +63,14 @@ pnpm e2e:clean # remove build/test artifacts, in
|
||||
|
||||
The runner solves the sharp edges hand-rolling would hit: in docker it builds the CGO backend on the **host** and runs it in the runtime image (forward-compatible when the host glibc ≤ the image's — the usual case; it smoke-tests this and only falls back to the build image if the host is newer), builds the Vite web assets on the host, runs them through the Go-served SPA, and keeps Playwright output container-local. See `apps/web/e2e/README.md` → "the managed runner".
|
||||
|
||||
`--no-build` reuses every production E2E artifact, not only Vite assets and the
|
||||
backend executable. On a fresh worktree or after rebasing, confirm packaged
|
||||
fixtures also exist; global setup currently requires
|
||||
`apps/backend/.build/kandev-plugin-e2e-1.0.0.tar.gz`. If it is absent, run
|
||||
without `--no-build` or first run `make -C apps/backend e2e-plugin-package`.
|
||||
Prefer a normal managed build after source or base-branch changes and reserve
|
||||
`--no-build` for repeated runs against unchanged artifacts.
|
||||
|
||||
### Raw commands (when you need fine control)
|
||||
|
||||
```bash
|
||||
@@ -365,6 +373,12 @@ A test that flakes under parallel/sharded load is one of two things — decide w
|
||||
## Selector guidelines
|
||||
|
||||
- **Prefer `data-testid` selectors** over text-based locators. Text content can change when UI is updated (e.g., hiding a badge), breaking tests that match by text. Use `getByTestId()` or `locator("[data-testid='...']")` for stable targeting.
|
||||
- **Scope Radix tooltip locators to the visible portal.** Radix can render an
|
||||
accessibility copy as well as the visible tooltip, so global test-id, text,
|
||||
or role locators may match multiple elements in strict mode. Start from a
|
||||
open portal `[data-slot="tooltip-content"][data-state="open"]`, then locate its visible
|
||||
descendant. Do not assume the trigger's `aria-describedby` target is the
|
||||
visual portal. Use bounding-box assertions when relative placement matters.
|
||||
- **Use page object methods** like `clickSessionChatTab()` (stable `data-testid`) instead of `sessionTabByText("1")` (fragile text match) for session tabs.
|
||||
- **Dropdown menus can detach** from the DOM when React re-renders the parent (e.g., WS events updating the sidebar). The `openSidebarMenuAndClick()` helper in `session-page.ts` retries the full open-click sequence on detachment — use this pattern for similar interactions.
|
||||
|
||||
|
||||
@@ -124,7 +124,7 @@ scripts/pr-resolve list <PR>
|
||||
|
||||
If `unresolved_review_thread_count` is nonzero but `unresolved_threads` is empty, or if `hidden_unresolved_threads` is non-empty, run `scripts/pr-resolve list <PR>` before acting. Fetch each full body with `scripts/pr-state --comment <comment_id>` or `scripts/pr-resolve show <PR> <THREAD_ID_OR_COMMENT_ID>`; hidden threads can contain valid current blockers even when the current-head filtered view is empty. `pr-state` can briefly report total unresolved count from historical state while current-head unresolved threads are empty; `pr-resolve list` is the authoritative actionable thread set for fixup work. If `scripts/pr-state --summary` reports a nonzero unresolved count but `scripts/pr-resolve show <PR> <THREAD_ID>` says the listed thread is `resolved: true`, treat the summary as stale state. Wait briefly, rerun `scripts/pr-state --summary <PR>`, and do not reply again to an already-resolved thread.
|
||||
|
||||
Poll at a 30s cadence with a **20 min cap**. Prefer one-shot `scripts/pr-state --summary <PR>` checks, or a bounded command that can finish naturally, over long inline shell loops. In this environment, a running non-TTY loop may not receive Ctrl-C through `write_stdin`, so avoid `while sleep ...; do ...; done` polling in the main session. Run `sleep 30` and then a fresh `scripts/pr-state --summary <PR>` as separate commands; a combined command can capture blank output. Rerun a blank summary before interpreting it.
|
||||
Poll at a 30s cadence with a **20 min cap**. Prefer one-shot `scripts/pr-state --summary <PR>` checks, or a bounded command that can finish naturally, over long inline shell loops. In this environment, a running non-TTY loop may not receive Ctrl-C through `write_stdin`, so avoid `while sleep ...; do ...; done` polling in the main session. Run `sleep 30` and then a fresh `scripts/pr-state --summary <PR>` as separate commands; a combined command can capture blank output. Record wall-clock time around requested sleeps: if less elapsed than requested, treat the wait as interrupted, re-poll, and report the shorter time. Rerun a blank summary before interpreting it.
|
||||
|
||||
Stop early if any required check fails. A `queued` or `in_progress` job caused by GitHub runner capacity is pending CI, not a failure: when the requested watch window or cap ends with no failures or unresolved comments, do not make speculative code changes. Report each pending job's exact name and status, plus its `details_url` or Actions run URL. If the user explicitly asks to poll until CI is green, continue past the normal pending-E2E stopping point with bounded one-shot `sleep N; scripts/pr-state --summary <PR>` checks; stop immediately on any `failed_checks`, and continue until `failed_checks: []`, `pending_checks: []`, and `unresolved_review_thread_count: 0`. Do not run `gh pr checks --watch` in the main session unless the runtime can keep the watcher isolated and automatically clean it up. If you do use `gh pr checks --watch`, keep watching until the command exits; GitHub can expand matrix jobs after an initial aggregate "Build" check passes, so the first green build/lint/test rows are not necessarily terminal.
|
||||
|
||||
@@ -450,12 +450,12 @@ Mark task 5 as in_progress.
|
||||
git push
|
||||
```
|
||||
|
||||
If verification or conflict resolution rebased the branch onto `origin/main`, local commit SHAs changed and a plain push may fail non-fast-forward. In that case, push with:
|
||||
If verification or conflict resolution rebased the branch, local commit SHAs changed and a plain push may fail non-fast-forward. When the pre-rebase remote SHA is known, use an exact lease:
|
||||
```bash
|
||||
git push --force-with-lease
|
||||
git push --force-with-lease=refs/heads/<branch>:<expected-remote-sha> origin HEAD:refs/heads/<branch>
|
||||
```
|
||||
|
||||
Mark task 5 as completed.
|
||||
Otherwise use generic `git push --force-with-lease`; never force push unconditionally.
|
||||
Mark task 5 as completed.
|
||||
|
||||
### 6. Re-check PR state
|
||||
|
||||
@@ -474,6 +474,8 @@ After the push, CI restarts and bots may re-review. Use `pr-poller` again when a
|
||||
- If new review comments appeared after the push → loop back to task 3.
|
||||
- If the poller hit its cap (`recommendation:` mentions "timed out") → surface the remaining pending items to the user and stop.
|
||||
|
||||
Before declaring the PR complete, run `gh pr view <PR> --json headRefOid,baseRefName,mergeable,mergeStateStatus` and `git ls-files -u`. If GitHub reports `CONFLICTING` / `DIRTY` or the index is unmerged, load `references/merge-conflicts.md` and repeat conflict resolution, verification, push, and post-push checks. Long CI waits can make the initial result stale.
|
||||
|
||||
Cap re-check loops at **3 iterations** to prevent runaway sessions. After 3, surface the remaining state to the user and stop.
|
||||
|
||||
Mark task 6 as completed.
|
||||
|
||||
@@ -58,6 +58,16 @@ headers` are infrastructure/package-registry issues, not app or test failures.
|
||||
If GitHub rejects `gh run rerun <run-id> --failed` while the workflow is still
|
||||
active, wait for the workflow/report job to finish and retry.
|
||||
|
||||
**Third-party action pnpm auto-install failures:** If an action detects pnpm
|
||||
and fails with `ERR_PNPM_ADDING_TO_ROOT`, inspect the pinned action bundle and
|
||||
its supported inputs before changing the repository package manager. Do not
|
||||
switch a pnpm workspace with `workspace:*` dependencies to npm. Either
|
||||
preinstall the action's pinned tool with
|
||||
`pnpm add --workspace-root --save-dev --ignore-scripts <tool>@<version>`, or
|
||||
run the action from an isolated npm working directory when the action supports
|
||||
one. Reproduce the action's exact version-detection command locally before
|
||||
pushing the workflow fix.
|
||||
|
||||
## Go Race-Suite Flakes
|
||||
|
||||
For a backend race-suite failure, extract the named failure from the saved log:
|
||||
|
||||
@@ -12,6 +12,8 @@ gh pr view <PR> --json number,url,baseRefName,headRefName,mergeable,mergeStateSt
|
||||
|
||||
Treat `mergeable:"CONFLICTING"` or `mergeStateStatus:"DIRTY"` as an actionable merge-conflict blocker. Treat `mergeable:"UNKNOWN"` as inconclusive: wait one short cadence and query again before deciding. States such as `BEHIND`, `BLOCKED`, `UNSTABLE`, or `HAS_HOOKS` may require an update or more CI/review work, but they are not by themselves proof of file-level conflicts.
|
||||
|
||||
Always use the freshly queried `baseRefName`. GitHub can retarget a stacked child PR after its parent merges, so neither the Git upstream nor a base branch remembered from an earlier fixup round is authoritative.
|
||||
|
||||
Inspect the local worktree:
|
||||
|
||||
```bash
|
||||
@@ -42,8 +44,11 @@ If `git ls-files -u` prints entries, or conflict markers are present in tracked
|
||||
git grep -n -E '^(<<<<<<<|>>>>>>>)'
|
||||
git diff --check
|
||||
git diff --cached --check
|
||||
git diff --stat origin/<baseRefName>
|
||||
```
|
||||
|
||||
Confirm that the final diff against the current GitHub base contains only the PR's intended delta. A clean index is insufficient if a retargeted stacked PR accidentally retains its merged parent's changes.
|
||||
|
||||
### When the local index is already unmerged
|
||||
|
||||
If `git ls-files -u` shows entries, a previous merge or rebase was left incomplete. Do not start another merge. Instead:
|
||||
@@ -72,3 +77,15 @@ If `git ls-files -u` shows entries, a previous merge or rebase was left incomple
|
||||
```
|
||||
|
||||
Do not discard unrelated user changes to make a merge/rebase easier. If unrelated dirty files block the conflict-resolution attempt, stop and ask before stashing, committing, or reverting them.
|
||||
|
||||
## Push after rebasing
|
||||
|
||||
Capture the remote branch SHA before rebasing when possible. After a successful rebase, prefer an exact force lease:
|
||||
|
||||
```bash
|
||||
git push \
|
||||
--force-with-lease=refs/heads/<branch>:<expected-remote-sha> \
|
||||
origin HEAD:refs/heads/<branch>
|
||||
```
|
||||
|
||||
An intervening fetch can update the remote-tracking ref consulted by generic `--force-with-lease`. Use the generic form only when the prior remote SHA was not captured, and never use an unconditional force push.
|
||||
|
||||
@@ -78,22 +78,23 @@ explicitly requests task tracking.
|
||||
|
||||
A ready PR may still end with "CI pending" after fixup when no checks have failed and no review threads remain unresolved, especially after a late fixup push restarts CodeQL, E2E, or preview jobs. Continue fixing failed checks and unresolved review threads, but it is acceptable to report the PR as ready locally once full local verification is green, `failed_checks: []`, `unresolved_review_thread_count: 0`, and only queued/in-progress long-running checks remain. This includes CodeQL and preview deploy as well as E2E shards; do not wait indefinitely. Include the exact pending checks from the final re-check in the response, and stop immediately if a pending check fails or a new unresolved thread appears.
|
||||
|
||||
6. **PR screenshots:** After creating the PR, check if `apps/web/.pr-assets/manifest.json` exists. If it does:
|
||||
6. **PR image preparation:** After creating the PR, check if `apps/web/.pr-assets/manifest.json` exists. If it does:
|
||||
- Read the manifest to list available screenshots/GIFs
|
||||
- Run `npx tsx apps/web/e2e/scripts/upload-pr-assets.ts <PR_NUMBER>` to generate embed markdown
|
||||
- If `apps/web/.pr-assets/embed.md` exists and is non-empty, append its contents to the PR body using a body file and `gh pr edit <PR_NUMBER> --body-file <file>`
|
||||
- Run `pnpm exec tsx apps/web/e2e/scripts/upload-pr-assets.ts <PR_NUMBER>` to validate local media and generate `apps/web/.pr-assets/embed.md`; despite its historical name, the helper does not upload files to GitHub
|
||||
- Inspect `embed.md` before changing the PR body. Append it only when it contains an actual hosted image URL. If it contains only drag-and-drop placeholders, leave the PR body unchanged and report that manual GitHub attachment is required
|
||||
- When hosted embed markdown is available, append it to the PR body using a body file and `gh pr edit <PR_NUMBER> --body-file <file>`
|
||||
- If `gh pr edit --body-file` fails after PR creation, especially with the GitHub Projects classic deprecation GraphQL error, fall back to REST. Build the payload with `jq --rawfile`, never by hand-escaping shell strings:
|
||||
```bash
|
||||
jq -n --rawfile body "<body-file>" '{body: $body}' > /tmp/pr-body-payload.json
|
||||
gh api --method PATCH repos/:owner/:repo/pulls/<PR_NUMBER> --input /tmp/pr-body-payload.json
|
||||
```
|
||||
- Tell the user to drag and drop the image files from `.pr-assets/` into the PR description on GitHub for the images to render
|
||||
- When placeholders remain, tell the user to drag and drop the image files from `.pr-assets/` into the PR description on GitHub for the images to render
|
||||
|
||||
7. **Return the PR URL** when done.
|
||||
|
||||
## Azure Repos flow
|
||||
|
||||
When `git remote get-url origin` points at Azure Repos, the steps are the same up through **Push** (1–3). For step 4, create an Azure Repos pull request instead of a GitHub PR. **Skip steps 5 and 6** — `/pr-fixup` and the PR asset upload flow are GitHub-specific.
|
||||
When `git remote get-url origin` points at Azure Repos, the steps are the same up through **Push** (1–3). For step 4, create an Azure Repos pull request instead of a GitHub PR. **Skip steps 5 and 6** — `/pr-fixup` and PR image preparation are GitHub-specific.
|
||||
|
||||
Prefer the Azure CLI when it is on `PATH`:
|
||||
|
||||
|
||||
@@ -28,31 +28,95 @@ if [ ! -d apps/node_modules ]; then
|
||||
(cd apps && pnpm install --frozen-lockfile)
|
||||
fi
|
||||
|
||||
# If the branch is behind main, rebase first.
|
||||
git fetch origin main
|
||||
git merge-base --is-ancestor origin/main HEAD || echo "branch is behind origin/main"
|
||||
git rebase origin/main
|
||||
# Resolve the current PR base; stacked PRs may not target main.
|
||||
PR_BASE="$(gh pr view --json baseRefName --jq .baseRefName 2>/dev/null || true)"
|
||||
if [ -n "$PR_BASE" ]; then
|
||||
git fetch origin "$PR_BASE"
|
||||
git merge-base --is-ancestor "origin/$PR_BASE" HEAD || echo "branch is behind origin/$PR_BASE"
|
||||
git rebase "origin/$PR_BASE"
|
||||
else
|
||||
echo "No PR base resolved; skipping rebase to avoid rewriting a stacked branch."
|
||||
fi
|
||||
|
||||
# Keep verbose output out of the main agent context. The helper prints the log
|
||||
# path and extracts targeted failure lines when a command fails.
|
||||
scripts/run-quiet format -- make fmt
|
||||
git status --short
|
||||
|
||||
# make typecheck uses the top-level Makefile path and can bypass package
|
||||
# pretypecheck hooks, so generate web metadata before typecheck.
|
||||
make fmt
|
||||
node apps/web/scripts/generate-release-notes.mjs
|
||||
node apps/web/scripts/generate-changelog.mjs
|
||||
make typecheck
|
||||
make test
|
||||
make lint
|
||||
scripts/run-quiet typecheck -- make typecheck
|
||||
scripts/run-quiet test -- make test
|
||||
scripts/run-quiet lint -- make lint
|
||||
```
|
||||
|
||||
Before rebasing, check whether `origin/main` is already an ancestor of `HEAD`.
|
||||
After quiet formatting, inspect the intended diff because formatter changes
|
||||
still require review. When a quiet command fails, use its returned log path for
|
||||
targeted inspection instead of rerunning the command with streamed output.
|
||||
|
||||
### Disk-constrained runners
|
||||
|
||||
If format, typecheck, tests, lint, or E2E reports `ENOSPC`, cache
|
||||
initialization/lock errors, or an apparently unrelated secondary failure,
|
||||
inspect free space on the temp and cache filesystems before changing code:
|
||||
|
||||
```bash
|
||||
df -h /tmp /var/tmp "$PWD"
|
||||
```
|
||||
|
||||
Choose one writable filesystem with space, create one agent-owned root there,
|
||||
and use it consistently for the remaining verification commands. For example,
|
||||
replace `/var/tmp` below if a different filesystem has the available space:
|
||||
|
||||
```bash
|
||||
VERIFY_CACHE_ROOT="$(mktemp -d /var/tmp/kandev-verify.XXXXXXXX)"
|
||||
mkdir -p "$VERIFY_CACHE_ROOT/tmp" "$VERIFY_CACHE_ROOT/go-cache" "$VERIFY_CACHE_ROOT/golangci-cache"
|
||||
export TMPDIR="$VERIFY_CACHE_ROOT/tmp"
|
||||
export GOCACHE="$VERIFY_CACHE_ROOT/go-cache"
|
||||
export GOLANGCI_LINT_CACHE="$VERIFY_CACHE_ROOT/golangci-cache"
|
||||
```
|
||||
|
||||
In a managed sandbox, request the normal filesystem escalation when the chosen
|
||||
root is outside the writable roots; do not work around sandbox permissions.
|
||||
`scripts/run-quiet` honors `KANDEV_RUN_QUIET_DIR`, then `TMPDIR`, so its logs
|
||||
move to the same filesystem. Re-run the original failing command before
|
||||
diagnosing source code and verify that the disk/cache errors are gone. After all
|
||||
verification finishes, remove only `$VERIFY_CACHE_ROOT`; do not clear shared
|
||||
caches or unrelated temp files.
|
||||
|
||||
### Restricted remote-environment failures
|
||||
|
||||
If Go tests fail from `httptest.NewServer` with an error such as
|
||||
`listen tcp6 [::1]:0: socket: operation not permitted`, treat the first result
|
||||
as a sandbox limitation. Rerun the exact command with the runtime's normal
|
||||
network or loopback escalation. Diagnose test code only if the escalated rerun
|
||||
still fails.
|
||||
|
||||
For desktop Rust changes, compare `rustc --version` with the `rust-version` in
|
||||
`apps/desktop/src-tauri/Cargo.toml` before running the Rust suite. Activate an
|
||||
installed matching rustup toolchain, extending `PATH` rather than replacing it
|
||||
and losing Node/pnpm. If no matching toolchain is installed, report the exact
|
||||
requirement or request installation instead of silently skipping Rust tests.
|
||||
|
||||
When a PR base was resolved, check whether `origin/$PR_BASE` is already an
|
||||
ancestor of `HEAD` before rebasing.
|
||||
If tracked files for the intended change are dirty, stash only those pathspecs
|
||||
before `git rebase origin/main`, then pop the stash before running
|
||||
before `git rebase "origin/$PR_BASE"`, then pop the stash before running
|
||||
`make fmt/typecheck/test/lint`. Do not use a broad `git stash` that could hide
|
||||
unrelated user changes. If you miss this and `git rebase origin/main` fails
|
||||
unrelated user changes. If you miss this and `git rebase "origin/$PR_BASE"` fails
|
||||
because of unstaged tracked changes, apply the same pathspec-only stash flow,
|
||||
then rerun the rebase. Resolve conflicts before continuing verification.
|
||||
|
||||
If `make fmt` changes files, review the diff and continue with the remaining commands. If any command fails, fix the issue and re-run the failed command; for formatter-caused changes, re-run any affected checks before reporting success.
|
||||
|
||||
`make test` includes backend, web, CLI, and `test-scripts`; do not silently skip
|
||||
`test-scripts` or its desktop smoke coverage while reporting full verification
|
||||
as green. Claim full verification only after the complete format, typecheck,
|
||||
test, and lint targets pass, plus the scoped Rust suite when Rust/Tauri code
|
||||
changed.
|
||||
|
||||
If `make typecheck` still fails because `apps/web/generated/changelog.json` or
|
||||
`apps/web/generated/release-notes.json` is missing, regenerate them and rerun
|
||||
`make typecheck`:
|
||||
|
||||
@@ -6,7 +6,9 @@ sandbox_mode = "workspace-write"
|
||||
developer_instructions = """
|
||||
Run the monorepo verification pipeline and fix issues found.
|
||||
|
||||
Fetch origin/main and rebase only when appropriate for the current branch/upstream. Format first: backend fmt, then app formatting. Run heavy commands through scripts/run-quiet and inspect only targeted log ranges when failures occur.
|
||||
Install apps dependencies when missing. Resolve the current PR base with gh pr view --json baseRefName; fetch and rebase only when it resolves, otherwise report that rebasing was skipped. Do not infer stacked-PR bases from Git upstream.
|
||||
|
||||
Fix root causes rather than suppressing checks. Rerun only the failed focused command after each fix. Finish when format, typecheck, test, and lint pass, or report a concrete blocker.
|
||||
Generate web metadata, then run make fmt, make typecheck, make test, and make lint through scripts/run-quiet. Inspect only targeted log ranges. Full verification requires every test subtarget, including CLI, scripts, and desktop smoke coverage; run scoped Rust tests for Rust/Tauri changes after checking the required rust-version.
|
||||
|
||||
Fix root causes rather than suppressing checks. Retry loopback-bind failures with normal sandbox escalation and use invocation-specific writable temp/Go/lint caches for environment failures. Rerun only the failed focused command after each fix. Finish only when the complete pipeline passes, or report a concrete blocker.
|
||||
"""
|
||||
|
||||
@@ -10,4 +10,16 @@ permission:
|
||||
|
||||
Run the monorepo verification pipeline and fix issues found.
|
||||
|
||||
Fetch `origin/main` and rebase only when appropriate. Format first. Run heavy commands through `scripts/run-quiet` and inspect only targeted log ranges. Fix root causes and rerun focused failed commands until clean, or report a concrete blocker.
|
||||
Install `apps` dependencies when missing. Resolve the current PR base with
|
||||
`gh pr view --json baseRefName`, fetch and rebase only when it resolves, and
|
||||
otherwise report that rebasing was skipped; do not infer stacked-PR bases from Git upstream.
|
||||
|
||||
Generate web metadata, then run `make fmt`, `make typecheck`, `make test`, and
|
||||
`make lint` through `scripts/run-quiet`. Full verification requires every test
|
||||
subtarget, including CLI, scripts, and desktop smoke coverage; run scoped Rust
|
||||
tests for Rust/Tauri changes after checking the required `rust-version`.
|
||||
|
||||
Fix root causes and rerun focused failed commands. Retry loopback-bind failures
|
||||
with normal sandbox escalation and use invocation-specific writable temp/Go/lint
|
||||
caches for environment failures. Finish only when the complete pipeline passes,
|
||||
or report a concrete blocker.
|
||||
|
||||
@@ -431,6 +431,7 @@ test-scripts:
|
||||
@printf "$(CYAN)Running script tests...$(RESET)\n"
|
||||
@python3 .github/scripts/lint-action-pinning_test.py
|
||||
@bash scripts/pr-state.test.sh
|
||||
@bash scripts/run-quiet.test.sh
|
||||
@bash scripts/opencode-code-review.test.sh
|
||||
@python3 scripts/opencode-code-review.test.py
|
||||
@python3 scripts/lint-harness-files.test.py
|
||||
|
||||
@@ -119,6 +119,14 @@ surface.
|
||||
</Tooltip>
|
||||
```
|
||||
Keep the wrapper focusable only while disabled; when enabled, the button itself owns focus.
|
||||
- **Interactive help inside Radix tooltips:** do not nest a `Tooltip` root inside
|
||||
another `TooltipContent` when the inner trigger must remain interactive.
|
||||
Tooltip roots under one provider coordinate open state, so the inner tooltip
|
||||
can close and unmount its parent before the pointer reaches it. Render the
|
||||
secondary help inline in the existing content or use a disclosure primitive
|
||||
with independent open state. Touch-pinned help must close on a second trigger
|
||||
tap, outside interaction, and Escape; verify desktop pointer and mobile-sized
|
||||
touch flows.
|
||||
- **Renaming a `data-testid`:** set the new id as `data-testid="<new>"` and keep
|
||||
the old id as `data-legacy-testid="<old>"`, then migrate e2e specs to the new
|
||||
id in the same PR. JSX rejects two `data-testid` attributes on one element,
|
||||
@@ -173,5 +181,10 @@ When you hit a limit, extract a helper function, custom hook, or sub-component.
|
||||
## Testing notes
|
||||
|
||||
- jsdom drops `secure` cookies over `http`, so `document.cookie` reads back empty. To assert a cookie write in a Vitest unit test, intercept the setter with `Object.defineProperty(document, "cookie", { set: ... })` and restore it after.
|
||||
- jsdom synthetic mouse events do not reliably open Radix Tooltip. In component
|
||||
tests, render under `TooltipProvider` and assert the keyboard-focus path with
|
||||
`fireEvent.focus`. Cover pointer hover in Playwright with `locator.hover()` and
|
||||
assert the visible portaled `role="tooltip"`; do not remove a hover regression
|
||||
solely because `mouseenter` or `pointerMove` failed in jsdom.
|
||||
- In Playwright tests, avoid strict locators that assume only one `terminal-panel` or `.xterm` exists. Mobile and dockview layouts can mount multiple terminal instances; scope to the active panel or use `.first()` / `.last()` deliberately with a comment or helper.
|
||||
- Shared E2E helpers that inspect mounted React/DOM internals must be scoped to the active panel/container, not global selectors, because hidden or stale mounted panels can coexist in dock/mobile layouts.
|
||||
|
||||
+15
-7
@@ -1,6 +1,7 @@
|
||||
#!/usr/bin/env bash
|
||||
# run-quiet — wrap a long-running command so its output stays out of agent
|
||||
# context. Redirects stdout+stderr to a unique /tmp log, prints a one-line
|
||||
# context. Redirects stdout+stderr to a unique log under
|
||||
# ${KANDEV_RUN_QUIET_DIR:-${TMPDIR:-/tmp}}, prints a one-line
|
||||
# success summary, and on failure auto-greps for the relevant error pattern
|
||||
# based on the tag. Exits with the underlying command's exit code.
|
||||
#
|
||||
@@ -11,13 +12,14 @@
|
||||
# Tags drive the error-extraction pattern (see grep_for_tag below). Unknown
|
||||
# tags fall back to a generic error/FAIL grep, then to the last 40 lines if
|
||||
# nothing matches.
|
||||
# KANDEV_RUN_QUIET_DIR overrides TMPDIR for log placement; /tmp is the final
|
||||
# fallback.
|
||||
#
|
||||
# Example:
|
||||
# scripts/run-quiet backend-test -- make -C apps/backend test lint
|
||||
#
|
||||
# Concurrency: each invocation gets a unique mktemp path
|
||||
# (/tmp/kandev-run.<tag>.<random>.log), so parallel agents never clobber
|
||||
# each other.
|
||||
# Concurrency: each invocation gets a unique mktemp path, so parallel agents
|
||||
# never clobber each other.
|
||||
|
||||
set -u
|
||||
|
||||
@@ -57,11 +59,17 @@ fi
|
||||
|
||||
# Sanitize tag for filename use: keep alnum, dash, underscore.
|
||||
SAFE_TAG="$(printf '%s' "$TAG" | tr -c 'a-zA-Z0-9_-' '-')"
|
||||
LOG_DIR="${KANDEV_RUN_QUIET_DIR:-${TMPDIR:-/tmp}}"
|
||||
|
||||
if ! mkdir -p "$LOG_DIR" || [[ ! -w "$LOG_DIR" ]]; then
|
||||
echo "run-quiet: cannot create log directory: $LOG_DIR" >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Best-effort cleanup of old logs (>24h). Failure is non-fatal.
|
||||
find /tmp -maxdepth 1 -name 'kandev-run.*.log' -mtime +0 -delete 2>/dev/null || true
|
||||
find "$LOG_DIR" -maxdepth 1 -name 'kandev-run.*.log' -mtime +0 -delete 2>/dev/null || true
|
||||
|
||||
LOG="$(mktemp "/tmp/kandev-run.${SAFE_TAG}.XXXXXXXX")"
|
||||
LOG="$(mktemp "$LOG_DIR/kandev-run.${SAFE_TAG}.XXXXXXXX")"
|
||||
mv "$LOG" "${LOG}.log"
|
||||
LOG="${LOG}.log"
|
||||
|
||||
@@ -86,7 +94,7 @@ echo "exit=$RC log=$LOG"
|
||||
|
||||
# Strip ANSI colour codes once so grep patterns work on `\033[…m`-rich logs
|
||||
# (eslint, playwright, golangci, gh).
|
||||
PLAIN="$(mktemp "/tmp/kandev-run.${SAFE_TAG}.plain.XXXXXXXX")"
|
||||
PLAIN="$(mktemp "$LOG_DIR/kandev-run.${SAFE_TAG}.plain.XXXXXXXX")"
|
||||
mv "$PLAIN" "${PLAIN}.log"
|
||||
PLAIN="${PLAIN}.log"
|
||||
# shellcheck disable=SC2016 # we want $0 inside awk, not shell expansion
|
||||
|
||||
@@ -57,3 +57,36 @@ if [[ "$(wc -l <<<"$normal_output" | tr -d ' ')" == "1" ]] && grep -q '^exit=0 l
|
||||
else
|
||||
fail "ordinary successful tags remain compact"
|
||||
fi
|
||||
|
||||
custom_tmp="$TMP_DIR/custom-tmp"
|
||||
mkdir -p "$custom_tmp"
|
||||
custom_tmp_output="$(env -u KANDEV_RUN_QUIET_DIR TMPDIR="$custom_tmp" "$SCRIPT" ordinary -- bash -c 'printf "custom tmp success\\n"')"
|
||||
if grep -q "^exit=0 log=$custom_tmp/kandev-run\.ordinary\." <<<"$custom_tmp_output"; then
|
||||
pass "TMPDIR controls the log location"
|
||||
else
|
||||
fail "TMPDIR controls the log location"
|
||||
fi
|
||||
|
||||
quiet_dir="$TMP_DIR/nested/quiet-logs"
|
||||
quiet_dir_output="$(KANDEV_RUN_QUIET_DIR="$quiet_dir" TMPDIR="$custom_tmp" "$SCRIPT" ordinary -- bash -c 'printf "quiet dir success\\n"')"
|
||||
if grep -q "^exit=0 log=$quiet_dir/kandev-run\.ordinary\." <<<"$quiet_dir_output"; then
|
||||
pass "KANDEV_RUN_QUIET_DIR overrides TMPDIR and is created"
|
||||
else
|
||||
fail "KANDEV_RUN_QUIET_DIR overrides TMPDIR and is created"
|
||||
fi
|
||||
|
||||
if quiet_dir_failure="$(KANDEV_RUN_QUIET_DIR="$quiet_dir" "$SCRIPT" ordinary -- bash -c 'printf "Error: quiet dir failure\\n"; exit 3')"; then
|
||||
fail "KANDEV_RUN_QUIET_DIR preserves failure extraction"
|
||||
elif grep -q "^exit=3 log=$quiet_dir/kandev-run\.ordinary\." <<<"$quiet_dir_failure" && grep -q 'Error: quiet dir failure' <<<"$quiet_dir_failure"; then
|
||||
pass "KANDEV_RUN_QUIET_DIR preserves failure extraction"
|
||||
else
|
||||
fail "KANDEV_RUN_QUIET_DIR preserves failure extraction"
|
||||
fi
|
||||
|
||||
if non_writable_output="$(KANDEV_RUN_QUIET_DIR=/proc/1 "$SCRIPT" ordinary -- bash -c 'printf "ok\\n"' 2>&1)"; then
|
||||
fail "non-writable log directory should fail"
|
||||
elif grep -q 'cannot create log directory: /proc/1' <<<"$non_writable_output"; then
|
||||
pass "non-writable log directory is rejected"
|
||||
else
|
||||
fail "non-writable log directory should produce a clear error"
|
||||
fi
|
||||
|
||||
Reference in New Issue
Block a user