Skip misconfigured agents across all phases of a run#45
Open
DAreRodz wants to merge 91 commits into
Open
Conversation
Run the classifier up front and act on each consequence: stop the run for a misconfigured judge before any hook or report, exclude misconfigured test agents from the tracker and the agent loop via a run-scoped allowlist without mutating config, halt after the current iteration when the improver is misconfigured and suppress the final-pass sweep, put the skipped set on the run context, and pass it through to the run report.
Add a deterministically-misconfigurable `gpt` (openai-api) test agent to the
testing-project config and make the e2e hook skip it when its credential is
absent. `afterAllScenarios` now reads `ctx.skipped`/`ctx.config`, derives the
runnable test-agent ids, and forwards them to `runE2eVerification`, which
appends one `--project <id>` per runnable id (defensive intersection with the
configured project names) so no Playwright project runs for a skipped agent and
the merged report attributes no e2e failure to it. The pure `projectArgs`
helper is split into its own module and unit-tested.
The planned `playwright.config.ts:23` change was intentionally not applied: in
that file `config` is the statically imported, unnormalized config, so
`roles.test.agents` is a `string[]` of ids (not `AgentDefinition[]`). The
original `.map((agentId) => ({ name: agentId }))` already yields string project
names equal to the ids — exactly what `--project` binding needs. Mapping over
`agent.id` there would read `.id` off a string (undefined) and fail typecheck;
the normalized `AgentDefinition[]` shape only exists on the running harness's
`ctx.config`, which is where this change reads it. A regression guard locks the
string-name contract.
Manual end-to-end acceptance (gated on wp-env + Playwright + a built package,
so not in the unit suite): with OPENAI_API_KEY unset, a testing-project run
skips `gpt` — no Playwright project/spec runs for it, the runnable `haiku` e2e
runs, and the run exits non-zero with `gpt` surfaced; with the key set, `gpt`'s
project is created and included.
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
1 participant
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.
Summary
Implements skip-misconfigured-agents for
@automattic/skillsmith(issue #37): an agent whose required provider credential is missing is removed from the run, announced once with its id and reason, and excluded from every phase — testers don't generate, judges don't review, and it stays out of later iterations.API / behavior
2, distinct from1(evaluation failure) and0(clean pass). A skip never lets the run exit0.report.json: new top-levelskippedarray recording each skipped agent (id + reason).Provider.requiredEnv(optional): names the credential env var a provider needs; a missing one triggers the skip.RunContext.skipped(additive): exposes the skipped set to hooks.Changeset:
minor(.changeset/skip-misconfigured-agents.md).Circular-import fix (latest review run)
The most recent commits break an import cycle between
testing-project/skillsmith.config.tsandeval/utils/verify-e2e.tsthat crashed fixture-config loading (the CLI passed typecheck/lint/unit-tests but couldn't actually load the config).runE2eVerificationnow receivesconfiguredProjectNamesas a parameter instead of importingconfigback — removing the back-edge so the graph is acyclic — and a root-suite regression test (src/__tests__/config-loads.test.ts) plus theconfig-smokegate (added in #43) guard against recurrence. Project-selector forwarding is unchanged.Verification
All declared guardrails green:
typecheck,lint,tests(185, 0 fail),config-smoke(0 — was 1 before the fix),changeset-format,changeset-status.Built via Radical Pipelines (pipeline v3); artifacts under
.pipelines/37-skip-misconfigured-agents-v3/. Tracked in Linear BILLOW-45.Closes #37.