Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions .claude/skills/investigate-issue/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -765,10 +765,11 @@ Once all approved changes are applied:
the exemptions are AGENTS.md's: no wire-visible effect, or behaviour no HTTP request can
reach, and an exempt change must say so in the report). It is a paid live sweep
against real provider accounts: the unfiltered collection is ~1,900 requests. The scope keeps
the run small and `HARNESS_MAX_REQUESTS` (below) is the enforced ceiling. Always use the shared
integration config via `APP_DIR=$(pwd)/tests/integrations/python` (that is
`tests/integrations/python/config.json`) and scope the run to the change with `PROVIDER` and
`FEATURE`, or `SMOKE=1` for a cross-cutting change. Never run the unscoped sweep, and never
the run small, and `HARNESS_MAX_REQUESTS` (below) is an optional ceiling on top of that. Start
the server against the shared integration config with `make dev
APP_DIR=$(pwd)/tests/integrations/python` (that is `tests/integrations/python/config.json`,
which the harness target already defaults to), and scope the run to the change with `PROVIDER`
and `FEATURE`, or `SMOKE=1` for a cross-cutting change. Never run the unscoped sweep, and never
widen the scope beyond the change, without a separate explicit yes from the user that names
the scope.

Expand All @@ -787,8 +788,11 @@ Once all approved changes are applied:
combined with PROVIDER, FEATURE or FOLDER, apply the same filters to both smoke commands.
The preflight is an estimate: the main pass forks one newman per provider and a producer
shared by several forks runs once per fork, so the live total can exceed the preflight sum
(observed: 102 preflight, 122 live for SMOKE=1). The enforced bound is
`HARNESS_MAX_REQUESTS`: always pass it with the ceiling the user approved. The recipe checks
(observed: 102 preflight, 122 live for SMOKE=1). `HARNESS_MAX_REQUESTS` is the optional
enforced bound: add it with the ceiling the user approved when a run is broad enough that the
cost is worth capping; a `PROVIDER=` + `FEATURE=` scoped run is usually small enough not to
need it. Left unset, the recipe skips the budget check and the run proceeds (Makefile:2177).
When it is set, the recipe checks
every newman launch against its exact filtered count before it starts (main shards, 429
replays, the cache-parity pass, sequential mode); a launch that would cross the cap is
refused and the run exits 3, so the live total can never exceed the approved number. The
Expand All @@ -808,9 +812,9 @@ Once all approved changes are applied:
make dev APP_DIR=$(pwd)/tests/integrations/python # in the background; wait for /health = 200
```
```bash
make run-provider-harness-test APP_DIR=$(pwd)/tests/integrations/python CI=1 HARNESS_MAX_REQUESTS=<approved ceiling> PROVIDER=<provider> FEATURE="<keyword>"
make run-provider-harness-test PROVIDER=<provider> FEATURE="<keyword>"
# cross-cutting change: the curated smoke set instead
make run-provider-harness-test APP_DIR=$(pwd)/tests/integrations/python CI=1 HARNESS_MAX_REQUESTS=<approved ceiling> SMOKE=1
make run-provider-harness-test SMOKE=1
Comment thread
coderabbitai[bot] marked this conversation as resolved.
```
Report the provider status table and `tmp/harness-failures.md` findings, and state exactly
which scope ran. See AGENTS.md "Every fix ends with a provider-harness run".
Expand Down
10 changes: 5 additions & 5 deletions .claude/skills/resolve-pr-comments/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ gh api repos/OWNER/REPO/pulls/PR_NUMBER/comments/COMMENT_ID/replies -X POST -f b
- `COMMENT_ID` is the numeric comment id (same as GraphQL `databaseId` from the thread's first comment).
- Request body: only `body` (string). No `in_reply_to`, `commit_id`, or path params.

## Step 5a: Run unit tests, then hand over the harness command
## Step 5a: Run the tests, then hand over the harness command

After the local edits for a comment (or for the batch), run the tests yourself and report the result. Do **not** run the provider harness - that run is the user's to trigger.

Expand Down Expand Up @@ -256,12 +256,12 @@ lsof -nP -iTCP:8080 -sTCP:LISTEN
make dev APP_DIR=$(pwd)/tests/integrations/python

# 3. run the harness against that server
make run-provider-harness-test APP_DIR=$(pwd)/tests/integrations/python CI=1 HARNESS_MAX_REQUESTS=<ceiling> PROVIDER=<provider> FEATURE="<keyword>"
make run-provider-harness-test PROVIDER=<provider> FEATURE="<keyword>"
```

`APP_DIR` must be an **absolute** path. `run-provider-harness-test` uses it verbatim (Makefile:2255), unlike `make dev` which wraps it in `$(abspath)`; a relative value resolves against the wrong cwd in `logs-db-url.js` and the dbverify reporter.
Do not pass `APP_DIR` or `CI=1` to `run-provider-harness-test`. `APP_DIR` already defaults to `tests/integrations/python` (Makefile:2255), the same profile `make dev` is pointed at, and `CI=1` suppresses the interactive HTML viewer that makes a live run readable. `make dev` is the one that needs `APP_DIR` spelled out, because it is what decides which code and config the server runs.

**Never print the block with a placeholder still in it.** `<ceiling>`, `<provider>` and `<keyword>` are for this template only. Substitute the real values for the change before showing it, so every line can be pasted straight into a shell, and say in one line why that scope covers the change. If the change is not wire-visible (comments, renames, test-only edits), skip the block and say it is exempt instead.
**Never print the block with a placeholder still in it.** `<provider>` and `<keyword>` are for this template only. Substitute the real values for the change before showing it, so every line can be pasted straight into a shell, and say in one line why that scope covers the change. If the change is not wire-visible (comments, renames, test-only edits), skip the block and say it is exempt instead.

## Step 5b: Push and Reply to FIX comments

Expand Down Expand Up @@ -331,7 +331,7 @@ If count is 0 (across all pages), report success. If comments remain:
6. **Update tracking file** after each action
7. **Some bots are slow** - CodeRabbit may take minutes to auto-resolve after push
8. **User pushes manually** - This skill never commits or pushes; the user must push code changes before expecting auto-resolution of FIX actions
9. **Never run the provider harness** - run the unit tests, then print the Step 5a block with both final commands filled in so the user runs the harness themselves
9. **Never run the provider harness** - run the tests via `make test-core`, then print the Step 5a block with both final commands filled in so the user runs the harness themselves

## Error Handling

Expand Down
14 changes: 7 additions & 7 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -596,26 +596,26 @@ lsof -nP -iTCP:8080 -sTCP:LISTEN
make dev APP_DIR=$(pwd)/tests/integrations/python

# 3. run the harness against that server
make run-provider-harness-test APP_DIR=$(pwd)/tests/integrations/python CI=1 HARNESS_MAX_REQUESTS=<ceiling> PROVIDER=<provider> FEATURE="<keyword>"
make run-provider-harness-test PROVIDER=<provider> FEATURE="<keyword>"
```

`APP_DIR` must be an **absolute** path. `run-provider-harness-test` uses it verbatim (Makefile:2255), unlike `make dev` which wraps it in `$(abspath)`; a relative value resolves against the wrong cwd in `logs-db-url.js` and the dbverify reporter.
Do not pass `APP_DIR` or `CI=1` to `run-provider-harness-test`. `APP_DIR` already defaults to `tests/integrations/python` (Makefile:2255), the same profile `make dev` is pointed at, and `CI=1` suppresses the interactive HTML viewer that makes a live run readable. `make dev` is the one that needs `APP_DIR` spelled out, because it is what decides which code and config the server runs.

Never print that block with a placeholder still in it. `<ceiling>`, `<provider>` and `<keyword>` belong to the template; substitute the real values for the change so every line pastes straight into a shell.
Never print that block with a placeholder still in it. `<provider>` and `<keyword>` belong to the template; substitute the real values for the change so every line pastes straight into a shell.

The exemptions are the ones in the previous section: a change with no wire-visible effect (comments, internal renames, log lines, test-only or guidance-only edits) or behaviour no HTTP request can reach is exempt. For an exempt change, say so explicitly instead of printing the block.

```bash
# Scoped to the change (preferred): the provider and a keyword from the affected cases
make run-provider-harness-test APP_DIR=$(pwd)/tests/integrations/python CI=1 HARNESS_MAX_REQUESTS=<approved ceiling> PROVIDER=<provider> FEATURE="<keyword>"
make run-provider-harness-test PROVIDER=<provider> FEATURE="<keyword>"

# Curated ~100-request smoke set across all providers, when the change is cross-cutting
make run-provider-harness-test APP_DIR=$(pwd)/tests/integrations/python CI=1 HARNESS_MAX_REQUESTS=<approved ceiling> SMOKE=1
make run-provider-harness-test SMOKE=1
```

The profile to pass is `APP_DIR=$(pwd)/tests/integrations/python`, the shared provider config at `tests/integrations/python/config.json` that every live check uses. Always pass it explicitly, so a stale server or another config never answers for the code under test.
The profile is the shared provider config at `tests/integrations/python/config.json` that every live check uses. Pass it to `make dev` as `APP_DIR=$(pwd)/tests/integrations/python` so a stale server or another config never answers for the code under test; the harness target already defaults to it and does not need it repeated.

`HARNESS_MAX_REQUESTS` is the enforced spend bound: the recipe checks every newman launch against its exact filtered request count before it starts and refuses any launch that would cross the cap (exit 3), so the live total never exceeds the approved number. Always pass it; the preflight count from `filter-collection.mjs` is only an estimate because shared producers repeat per provider fork. Stream-cancellation probes are never sent under a cap.
`HARNESS_MAX_REQUESTS=<n>` is an optional enforced spend bound: the recipe checks every newman launch against its exact filtered request count before it starts and refuses any launch that would cross the cap (exit 3), so the live total never exceeds the approved number. Add it when a run is broad enough that the cost is worth capping; a `PROVIDER=` + `FEATURE=` scoped run is usually small enough not to need it. The preflight count from `filter-collection.mjs` is only an estimate because shared producers repeat per provider fork. Stream-cancellation probes are never sent under a cap.

Port 8080 is a blocking precondition worth restating in the block: the recipe reuses any server whose `/health` answers and never starts the `APP_DIR` one, so a stale listener silently tests old code. `lsof -nP -iTCP:8080 -sTCP:LISTEN` must come back empty, or show only a Bifrost started from the code under test. Starting it first with `make dev APP_DIR=$(pwd)/tests/integrations/python` and waiting for `/health` is the reliable pattern, since a cold start can outlast the recipe's 60s health wait.

Expand Down
Loading
Loading