Skip to content

feat(benchmark): run benchmark suites as a default workflow - #1379

Open
SaulMoro wants to merge 253 commits into
Tencent:mainfrom
SaulMoro:feat/1180-benchmark-suites
Open

SaulMoro wants to merge 253 commits into
Tencent:mainfrom
SaulMoro:feat/1180-benchmark-suites

Conversation

@SaulMoro

@SaulMoro SaulMoro commented Oct 25, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

A team could not tell whether a tool, a model, its own assets or one of its workflows fixes its real tasks better. Now teamai benchmark run <suite> runs a suite from the team repo: every arm works the same tasks in fresh clones and is scored by a check it never sees. It is teamai's second default workflow, workflows/teamai-benchmark.yaml, which a team can replace, and the engine and drivers know nothing about benchmarks.

$ teamai benchmark run fix-auth --sandbox                       # local driver; done envelope on stdout, results on stderr
$ teamai benchmark run fix-auth --sandbox --member-aliases      # arm aliases from ~/.teamai/models/aliases.yaml over the team's
$ teamai benchmark run fix-auth --sandbox --fail-on-regression  # CI: also exit 1 when an arm dropped against teamai-reports
$ teamai benchmark try ../team/benchmarks/fix-auth/tasks/expired-token.yaml   # no agent, no run, no sandbox

An arm changes the tool, the model, the team assets or the workflow, and a matrix crosses them:

# benchmarks/fix-auth/suite.yaml
schemaVersion: v1alpha1
name: fix-auth
repo: https://github.com/acme/auth-service.git
defaults: { tool: claude, model: strong }
arms:
  - name: main                                    # the revision you pulled
  - name: next
    team: next                                    # a branch, tag or commit of the team repo
  - name: no-auth-tokens
    team: { without: [skills/auth-tokens] }       # the pulled revision without these team repo paths
  - name: plan-first
    workflow: plan-then-fix                       # a team workflow with inputs.task
repetitions: 3
timeout: 45m
parallel: 2                                       # 1 to 32 lanes; also bounded by workflows.max_parallel
trial: { timeout: 8m, budget: { tokens: 300000 } }
# benchmarks/onboarding-docs/suite.yaml: arms claude-pulled, claude-none, codex-pulled, codex-none
defaults: { tool: claude }
matrix:
  tool: [claude, codex]
  team: [pulled, none]
# benchmarks/fix-auth/tasks/expired-token.yaml
schemaVersion: v1alpha1
title: Reject expired refresh tokens
prompt: Refresh tokens past their expiry are still accepted. Fix it.
startingCommit: 3f2c9a7d1e0b4c6a8f5d2e1b0c9a8f7e6d5c4b3a
check:
  run: npx vitest run test/auth/refresh.test.ts
  protect: ['test/**', 'package.json', 'package-lock.json']
  add:
    - test/auth/refresh.test.ts                                     # from source.fix; never in the agent's clone
    - { from: check-files/token.test.js, to: test/token.test.js }   # from the suite directory
source: { platform: claude-code, sessionId: 7c1e2b9a, fix: 9a1d4e7c2b8f0a3d6e5c1b9f8a7d2e4c6b0a1f3e }

The default workflow, which a team's workflows/teamai-benchmark.yaml (or workflows/<ns>/teamai-benchmark.yaml) replaces, for example without strict:

schemaVersion: v1alpha1
name: teamai-benchmark
description: Run a benchmark suite. Every arm works the same tasks in fresh clones, judged by checks it never sees.
invoke: deny
inputs: { suite: string, aliases: string }
steps:
  - id: plan
    command: teamai benchmark plan "$SUITE" --aliases "$ALIASES"
    env: { SUITE: "{{ inputs.suite }}", ALIASES: "{{ inputs.aliases }}" }
    output: { definition: workflow, plan: json }
  - id: trials
    needs: [plan]
    workflow: { from: plan }
    requires: { isolation: sandbox, strict: true }
finally:
  - id: report
    command: teamai benchmark report
    output: { text: string, table: json }
outputs: { text: steps.report.outputs.text, table: steps.report.outputs.table }
teamai benchmark run fix-auth
├─ dry plan        the plan step's freeze, with no run: an invalid suite, an unresolvable ref or alias, a `without`
│                  path that changes nothing, a bad arm workflow, single-repo mode or user scope are refused, no run left
└─ workflow start teamai-benchmark --inputs '{"suite":"fix-auth","aliases":"team"}' --timeout 45m
   ├─ plan         teamai benchmark plan: freezes the team revision, aliases and scorers; fills the commit cache;
   │               prints one step per baseline and per trial
   ├─ trials       workflow: { from: plan }
   │  ├─ baselines.<task>                   clone → setup → teamai benchmark check --baseline
   │  └─ lane<k>.r<rep>.<task>.<arm>        clone + team install → setup → prompt or arm workflow → teamai benchmark check --trial
   └─ finally      teamai benchmark report: table, pairs, benchmark record (run directory, and teamai-reports with usage reporting on)
teamai benchmark run exits When
0 The run succeeded, whatever the trials scored; also when the record is still pending, and with --fail-on-regression when nothing comparable is published (with a notice)
1 The run failed, was cancelled or timed out (as workflow start); report failed or was cancelled; --fail-on-regression found an arm whose paired drop has its whole 95% interval below zero

The dashboard's Benchmarks page. These screenshots are from this PR's earlier revision; since then Compare also pairs arms that differ only in workflow, a without arm lists its paths, and the empty page suggests teamai benchmark run.

Compare: how much the team's assets add, trial by trial (onboarding-docs, a seeded workspace).

Compare tab: pulled team against no team assets

Team changes: one tool and model across the team revisions it ran on, with the files each changed.

Team changes tab: claude on three team revisions

Arms: every arm over time, as before.

Arms tab, fix-auth

Question Tab, panel What it showed
How much do our assets help on onboarding docs? Compare, by Team assets claude +41 pp · 95% +16 pp to +59 pp over 27 paired trials, −60% cost per passed trial
Do they help codex as much? Compare, Difference per pair of arms +15 pp, interval crosses zero: not separated
Did the auth-tokens skill help claude? Team changes 67%, 62%, 70% over three revisions, none told apart; 67ee2ce changed 3 files, session-fixation 1/12 to 3/12
Switch our auth fixes to claude? Arms, Leading arm claude 67% · 95% 57%–75% · not separated from codex
Which model is worth paying for on migrate-db? Arms, Pass rate against cost sonnet 73% at 0.31 USD a trial, opus 78% at 0.89 USD; the frontier joins haiku, sonnet, opus
Light theme, Cost axis, zh-CN, tablet and phone

Compare, light

Team changes, light

migrate-db, pass rate against cost per trial

Arms, zh-CN

Compare, tablet Team changes, tablet Compare, phone Team changes, phone Arms, phone
Generic workflow keys this adds, usable by any workflow
Key Contract
workflow: { from: <step> } Runs the definition a needed step printed; admitted when that step reports. A bundle may not hold from or prompt_file steps; a refused admission fails the producer invalid_output. A run with a from step is isolated
output types workflow, json A workflow bundle to admit; opaque data passed on whole
finally: Top-level read-only command steps that run on any end but a crash; the outcome is kept; workflow cancel ends them
done.outputs The run's top-level outputs, resolved after finally
workflow start --timeout The shorter of the definition's timeout and the flag; the skill driver supports it too
workspace: { clone: { commit, repo?, team } } A fresh clone teamai prepares at lease, offline, with the team installed at a revision, none, a ref or { without: [paths] }
tool: Leased only to a driver that lists it in can.tools; tool_unavailable when no driver of an alone run can take it
permissions: unattended Prompt steps only, under a strict sandbox floor; leased only to can.unattended; tools without a supported mode fail tool_unavailable
model: { name, effort? } Exact, never alias-resolved
on_limit: end_turn timeout, max_turns, budget end the turn from tool launch and keep the work; on a workflow step, an allowance its prompt turns spend; drivers opt in with can.end_turn
requires: { …, strict: true } Strict floor for isolation and network; withholds the credential list under a strict sandbox; a local-only run that cannot meet it is refused at start
scorers: { sampling: all } Every pass judged; replaces the generated-only required
Isolated run A run that opens a clone: never published, claimed or retried, no session publication, snapshots or member stats; record isolated: true
A step's teamai A command step that runs teamai … gets the CLI that runs the step first on its PATH
Benchmark record

workflows/runs/<run id>/benchmark.json in the scope's data directory, and benchmarks/runs/<run id>.json on teamai-reports when usage reporting is on. The released BenchmarkRecord shape, field for field (limits, scoringCost, baselines, arm alias and source), plus schemaVersion: 1, run, outcome, teamaiVersion, startedAt, finishedAt, username?, workflow: { name, origin, sourceHash }, arms[].team (pulled | ref | without (paths) | none, with its revision), arms[].workflow?, trials[].agent?, trials[].used? (loaded | unknown per skill), complete on tokens and cost, and skillChanges. Pairs are derived from trials[] when read. No prompts, commands, credential URLs, local paths or hidden check bytes. The workflow run record drops benchmark and gains isolated.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature causing existing behavior to change)
  • Documentation only
  • Refactor / internal cleanup

Test Plan

  • npx tsc --noEmit passes
  • npm run lint passes
  • npx vitest run passes, except the GitLab request-deadline tests, which fail the same way on main on this machine
  • Added/updated tests for the change
npm run build                                     ok
npx tsc --noEmit                                  ok
npm run lint                                      ok
npx vitest run --shard=1/2                        10954 passed, 3 failed, 4 skipped
npx vitest run --shard=2/2                        8993 passed, 1 failed, 20 skipped
  the 4 failures: gitlab-mr-timeout, gitlab-org-timeout, gitlab-mr-create-timeout;
  gitlab-mr-timeout and gitlab-org-timeout fail the same way on main (254fb92db) here
npm run test:e2e -- src/__tests__/e2e/benchmark- src/__tests__/e2e/workflow-from src/__tests__/e2e/dashboard-workflow-runs
                                                  21 files, 159 passed
npm run test:e2e -- src/__tests__/e2e/workflow-conformance.test.ts -t "through the CLI as <profile> "
                                                  local 63, skill 53, skill-bare 53 passed

The two commits after that add tests only. On them: npm run test:e2e -- src/__tests__/e2e/benchmark- 19 files, 166 passed; npx vitest run src/__tests__/benchmark-history.test.ts 20 passed; tsc and lint ok.

Real-CLI e2e record

The built dist/index.js in a throwaway HOME, with a real Claude login and default Claude settings (no permission allow list). A team repo with one skill, answer; a project whose node test.js fails; a suite with one task and two arms, pulled and team: { without: [skills/answer] }.

$ teamai --version
0.22.0
$ claude --version
2.1.312 (Claude Code)
$ date -u
2026-10-30T12:51:41Z
$ teamai init https://github.com/acme/team.git --provider git --agent claude --scope workspace && teamai pull
$ teamai benchmark run fix --sandbox; echo "exit: $?"
| Arm             | Tool   | Model (source)      | Team                          | Planned | Passed | Error | Pass rate   |
| pulled          | claude | tool default (team) | pulled 930a93c                | 1       | 1      | 0     | 100% (1/1)  |
| no-answer-skill | claude | tool default (team) | 930a93c without skills/answer | 1       | 1      | 0     | 100% (1/1)  |

| Arm             | Skill  | Change  | Loaded in this arm | Loaded in the defaults arm |
| no-answer-skill | answer | removed | 0/1                | 1/1                        |
exit: 0

(Columns that were all 0 are left out above.)

  • Both trial transcripts show permissionMode: bypassPermissions, from permissions: unattended under the strict sandbox.
  • The record is published on teamai-reports as benchmarks/runs/<run id>.json, and no clone is left in TMPDIR.
  • Not run here: Codex, CodeBuddy and OpenCode with real models, the gitlab and github providers, and Windows (the .cmd wrapper of a step's teamai). The e2e suite covers them with fake tools; real runs are left to CI.
Issue criteria and the tests that cover them
Issue # Test
#1180 1 e2e/benchmark-pull, benchmark-suite
#1180 2 e2e/benchmark-local (eight trials)
#1180 3 e2e/benchmark-team-assets, e2e/benchmark-local
#1180 4 e2e/benchmark-env
#1180 5 e2e/benchmark-checks
#1180 6 e2e/benchmark-checks
#1180 7 e2e/benchmark-models
#1180 8 benchmark-table
#1180 9 e2e/benchmark-judges
#1180 10 e2e/benchmark-local, benchmark-table
#1180 11 e2e/benchmark-lifecycle, e2e/benchmark-judges, e2e/benchmark-skill-loads
#1180 12 e2e/benchmark-driver-contract, workflow-engine
#1180 13 e2e/benchmark-pull (review --branch), review-branch
#1180 14 e2e/benchmark-local, e2e/benchmark-run, benchmark-table
#1180 15 e2e/benchmark-run
#1180 16 e2e/benchmark-lifecycle, e2e/benchmark-local, workflow-records, e2e/benchmark-ci-job
#1180 17 e2e/benchmark-parallel
#1180 18 e2e/benchmark-fail-on-regression
#1180 19 e2e/benchmark-try
#1180 20 e2e/benchmark-team-arms, e2e/benchmark-local
#1180 21 e2e/benchmark-models
#1181 1 e2e/benchmark-team-arms
#1181 2 e2e/benchmark-team-arms (team branch, frozen models and scorers)
#1181 3 e2e/benchmark-team-arms (team remote out of reach)
#1181 4 e2e/benchmark-local, e2e/benchmark-team-arms
#1181 5 e2e/benchmark-team-assets (run ends, run cancelled halfway)
#1181 6 benchmark-table, e2e/benchmark-team-arms
#1181 7 e2e/benchmark-team-arms (team branch, frozen models and scorers)
#1181 8 e2e/benchmark-driver-contract (team branch)
#1181 9 benchmark-history (run pairs and history)
#1181 10 benchmark-history
#1181 11 e2e/benchmark-skill-loads
#1181 12 benchmark-suite, benchmark-history
#1181 13 e2e/benchmark-workflow-arms
#1181 14 e2e/benchmark-workflow-arms
#1181 15 e2e/benchmark-workflow-arms
#1181 16 benchmark-team-without, e2e/benchmark-team-arms, e2e/benchmark-pull (review --branch)
#1181 17 e2e/benchmark-workflow-arms
#1400 1-4, 6 e2e/benchmark-limits, benchmark-table
#1400 5 review-branch, e2e/benchmark-pull
#1400 7 benchmark-suite, e2e/benchmark-local
#1401 1-4, 6 e2e/benchmark-checks, benchmark-suite
#1401 5 e2e/benchmark-pull (review --branch and run), review-branch, benchmark-suite
#1401 7 benchmark-suite, e2e/benchmark-local

Paths are under src/__tests__/.

Related Issues

Closes #1180, closes #1181, closes #1400, closes #1401. #1402 builds on teamai benchmark try, source.fix and check.add; #1469 on the generated scorer pins. Other features as default workflows: #1589.

Notes for Reviewers

  • Door: two-way for runs and the generic keys, which are additive. One-way for the suite and task schema, which is strict, so source.fix, check.add, trial, team (with without paths), matrix, arm workflow and parallel ship in this first release. workflow start --benchmark, --member-aliases and --try leave workflow start for teamai benchmark run|try|plan|check|report; they were never released.
  • Blast radius: the generic keys are opt-in, except what ordinary runs see:
    • prompts in different places now run in parallel up to workflows.max_parallel, as the guide already says;
    • a top-level run's outputs now reach done.outputs, including a null a step gave;
    • a command step that runs teamai … gets the running CLI first on its PATH.
    • teamai uninstall in a workspace whose hooks live in HOME now keeps HOME's built-in hooks while the user scope or another workspace on the machine still runs them, and removes only this workspace's team hooks.
  • Boundary: src/benchmark/ imports only src/workflow/'s public interface; the engine, protocol and drivers never import src/benchmark/. workflow-module-boundaries.test.ts fails closed on a new edge, with an empty allowlist. The suites cache moved to src/benchmark/.
  • Changes since this PR's earlier revision:
    • A local benchmark without a sandbox is refused at start (requirement_unmet), where it used to print a notice, run every trial to not_run and exit 0.
    • A baseline's check.add files are written after setup, where they used to be written before. A run and teamai benchmark try share one order: clone, setup, restore check.protect (trial only), write check.add, check.
    • The error code benchmark_local is now isolated_run.
    • Where user scope also reaches the workspace, plan refuses every suite with team_ref_blocked and leaves no run (Q19), where trials used to settle not_run.
    • Benchmarks are refused in single-repo mode for every arm, with a benchmark-worded message.
    • --member-aliases stays on teamai benchmark run; the default workflow takes it as input aliases (team | member), and a team override without that input is refused under the flag.
    • Arm team: { without } takes team repo paths for any asset, namespaces included, in place of skill names. plan uses pull's resolvers and refuses a path that changes nothing: an inactive namespace, a shared path an active namespace replaces ("list X too"), a path tags leave out, a skill the member excludes, or nested paths. It is exact under skills/, rules/, agents/, claudemd/ and docs/; elsewhere a path only has to exist. An entry inside a shared file still needs a branch.
    • --fail-on-regression compares with the newest comparable record published on teamai-reports, leaving out this machine's pending queue, and exits 0 with a notice when there is none. parallel is 1 to 32, serial by default.
    • Arm workflows and a teamai-benchmark override resolve by namespace like any team workflow. Two active namespaces defining one name refuse at start with workflow_namespace_conflict.
    • Strict-sandbox trials run with permissions: unattended, including arm workflows. A team override without that floor keeps the tool's permission settings and reports it.
    • An arm workflow is refused at start when it takes an input besides task, has finally steps, runs workflow: { from }, or runs an agent while the arm sets no model (set model in the suite's defaults).
    • Every step in a clone gets back the team assets installed for it before it runs, so a continue step after another one sees them. A benchmark installs them for the arm's tool, through the new workspace.clone.team.selection.enabledAgents, even when the arm's workflow starts with a command.
    • A workflow arm's trial is classified by the steps its workflow ran: not_run when the tool could not launch, even after a first command ran, and error once a prompt was leased or the trial was cancelled after a lease, including an earlier loop pass.
    • A failed isolated run prints the command that starts a new run, with its --inputs.
  • Hand-back: a lease handed back to the same driver gets its grants from that driver's current can; its deadline, and an end_turn timeoutMs, count from the first lease, so a restart never extends a step.
  • Accepted risk: a turn lost to a driver restart is not charged to a workflow step's allowance. Each prompt's own lease is still capped from leasedAt, so the overrun is at most one prompt timeout per prompt.
  • External drivers: a benchmark needs TEAMAI_WORKFLOW_ATTEMPT from the lease env for its command steps, clone in can.workspace, can.env: true and can.isolation: sandbox for the strict sandbox, can.setup, can.unattended for unattended prompts, can.end_turn for trial limits, the arm's tool in can.tools, and the lease workspace as cwd.
  • Known limit: the skill driver's suggested start commands do not carry --timeout.
  • Departs from the issues: [feat] Run benchmark suites as workflows #1180 and [feat] Compare team asset sets in benchmarks #1181 are rewritten to this surface (original [feat] Run benchmark suites as workflows #1180 text kept in a comment). In [feat] Compare team asset sets in benchmarks #1181, pairs are any two arms that differ in exactly one of team, tool, model and workflow, not each arm against the defaults arm; the Compare tab and outputs.table.pairs carry the interval, and no arm is recommended. Skill loads are shown against the suite's defaults arm.
  • Start reading at workflows/teamai-benchmark.yaml, then src/benchmark/ (start, plan, trial, report, team-without), then the generic keys in src/workflow/.

@SaulMoro
SaulMoro force-pushed the feat/1180-benchmark-suites branch from 5959410 to 824495b Compare October 25, 2026 10:27
@SaulMoro SaulMoro assigned jeff-r2026 and unassigned jeff-r2026 Oct 25, 2026
@github-actions

Copy link
Copy Markdown
  • [P1 blocking] Protected submodules can supply modified checks — src/workflow/benchmark/trial.ts:732. If setup initializes a tests submodule and the task uses protect: [tests], an agent can replace tests/check.js with an unconditional success. The superproject’s clean/checkout-index operations do not restore the submodule’s contents, and holds() accepts any directory for mode 160000. The modified check can therefore mark an unchanged, broken implementation passed. Reject protected gitlinks or restore and verify their contents recursively.

The PR description documents sufficient testing: it includes a built real-CLI verification record and representative E2E results. Deferred provider/agent coverage is not blocking.

Reviewed only the specified diff; ran no PR code.

@github-actions

Copy link
Copy Markdown
  • [P1 blocking] Normalize protected paths before checking submodules — src/workflow/benchmark/trial.ts:713. protect: ["./tests/check.js"] is accepted, but reachesInto() compares the initial . with tests and returns false. If setup initializes a tests submodule, the superproject’s listing and cleanup do not restore that nested file. An agent can replace it with unconditional success and receive passed without fixing the implementation. Normalize or reject dot components before matching.

  • [P1 blocking] Enforce the suite deadline during local tool execution — src/workflow/local-driver.ts:1054. The adapter receives only a signal triggered by terminal signals; lease.expiresAt is unused. With timeout: 30s and a hung tool, the driver remains awaiting adapter.run() and never calls next to reconcile the timeout, leaving the run and clone active indefinitely. Abort the launch at the lease deadline and reconcile afterward.

The earlier exact protect: [tests] submodule case is fixed; the dot-prefixed nested-path bypass above remains.

The PR description documents sufficient testing, including a built real-CLI run. Deferred provider/agent coverage is not blocking.

Reviewed only the specified diff; ran no PR code, builds, or installs.

@SaulMoro SaulMoro assigned jeff-r2026 and unassigned jeff-r2026 Oct 25, 2026
@github-actions

Copy link
Copy Markdown
  • [P2 non-blocking] Update the timeout guidance — docs/guide/benchmarks.md:83 and docs/guide/zh-CN/benchmarks.md:83 still say running tools finish first and hung trials block until CI kills them. The local driver now aborts tools at the suite deadline and records the interrupted trial as error. Update both timeout bullets to match.

  • [P3 nit] Escaped globs bypass the submodule guard — src/workflow/benchmark/trial.ts:717. The accepted glob protect: ['te\sts/**'] matches tests/** in Git, but reachesInto() treats the backslash literally and misses a tests submodule. With that unusual spelling and an initialized submodule, modified checks remain unrestored and can yield a false passed. Reject backslashes or handle Git’s glob escaping.

Earlier findings: The exact-submodule case, dot-prefixed path bypass, and local-tool deadline omission are resolved in the current diff.

Testing: The PR description documents sufficient testing, including a built real-CLI run. Deferred provider/agent coverage is not blocking.

Reviewed only the specified diff; ran no PR code, builds, or installs.

@SaulMoro SaulMoro assigned jeff-r2026 and unassigned jeff-r2026 Oct 25, 2026
@github-actions

Copy link
Copy Markdown
  • [P2 non-blocking] Apply the suite deadline to trial installation — src/workflow/benchmark/trial.ts:547. The installation subprocess uses a fixed ten-minute timeout rather than lease.expiresAt. If a 30s suite expires during a slow installation, next --prepare keeps awaiting it, delaying reconciliation, clone cleanup and the results table. Bound installation by the remaining suite time and propagate cancellation.

Earlier findings: All are resolved: protected-submodule handling, dot-prefixed paths, escaped globs, local-tool deadlines, and both languages’ timeout guidance.

Testing: The PR description includes sufficient built real-CLI verification. Deferred provider/agent coverage is not blocking.

Reviewed only the specified diff; ran no PR code, builds or installs.

@SaulMoro
SaulMoro force-pushed the feat/1180-benchmark-suites branch from 621a620 to 79b2e12 Compare October 25, 2026 22:22
@SaulMoro
SaulMoro force-pushed the feat/1180-benchmark-suites branch from e3d7f4a to f7a7737 Compare October 26, 2026 05:04
@SaulMoro
SaulMoro marked this pull request as ready for review October 26, 2026 06:29
@github-actions

Copy link
Copy Markdown
  • [P2 non-blocking] Apply the suite deadline to judging — src/workflow/scoring.ts:1165. With a 30s suite and a slow judge, the local driver awaits judgeLease() without passing the suite deadline, so it cannot call next to reconcile the timeout or print final results until judging finishes. Bound judging by the remaining suite time.

  • [P2 non-blocking] Handle malformed glob classes during validation — src/workflow/benchmark/suite.ts:112. A task with protect: ['test/['], add: ['test/check.js'] and a valid source.fix throws from new RegExp() inside the Zod transform. This interrupts suite validation instead of producing a per-file diagnostic. Validate the pattern or handle the regex error.

Earlier findings: All are resolved in the current diff, including the protected-submodule bypasses, local-tool deadline, bilingual timeout guidance and installation deadline.

Testing: The PR description documents sufficient built real-CLI verification. Deferred provider/agent coverage is not blocking.

Reviewed only the specified diff; ran no PR code, builds or installs.

@github-actions

Copy link
Copy Markdown

No new confident findings in the current diff.

Earlier findings: All are resolved, including the protected-submodule bypasses, local-tool deadline, bilingual timeout guidance, installation deadline, judging deadline, and malformed-glob validation crash.

Testing: The PR description documents sufficient testing, including built real-CLI verification. Coverage deferred to CI is not blocking.

Reviewed only the specified diff against the trusted base review criteria. Ran no PR code, builds, tests, or installs.

@STiFLeR7 STiFLeR7 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary. Adds --benchmark to teamai workflow start: runs YAML-defined benchmark suites (tasks with a starting commit, an optional known-good fix, and a check or success-criteria judge) across tool/model "arms" and repetitions, each trial in an isolated clone with secrets stripped and git hooks/filters/fsmonitor neutered, scored and tabulated. Adds --try to validate one task file's check against its starting commit and fix before it joins a suite. Rebased on #1304, closes #1180.

Verified.

  • Secret stripping genuinely happens before any tool/driver launch and can't be reintroduced by a step's requires.env: negotiate() (src/workflow/requirements.ts:272-276) filters a step's requires.env to exclude every name in the trial's withhold list (stripped) before building the granted lease's envScope.provide — so provide and withhold are disjoint by construction, not just by call order in stepEnvironment (src/env-exec.ts:347-360). TRIAL_WITHHELD_ENV (terms.ts:24-26) is threaded into the plan, prompt/setup scope, and --try. Confirmed by reading e2e/benchmark-env.test.ts in full: none of the withheld names appear in setup/tool/check env even when the team env declares them, and no git hook/fsmonitor/clean filter in the clone sees them either.
  • Isolation is real: freshClone (trial.ts:482-504) git inits a fresh repo, fetches only startingCommit by SHA (never a branch ref) from the run's checkout or a commit cache, and detaches HEAD there — no git remote add is ever called, and a SHA-only fetch cannot pull descendant commits, so the fix commit is never reachable in the trial's clone during the prompt phase. check.add/restoreProtected only write fix-derived bytes in after the prompt's report is already committed, i.e. after the agent's tool process has exited.
  • check.add/source.fix verification is real, not a naive copy: writeAdded (trial.ts:799-823) and restoreProtected (trial.ts:699-751) read the source commit into a private index via read-tree, write with checkout-index --stdin, and verify every written path byte-for-byte against the commit's blob hash via holds() (trial.ts:840-853), including the executable bit and symlink-vs-file kind. Submodule paths are explicitly refused.
  • Path traversal is blocked at the schema level: RepoPathSchema (suite.ts:30-33) rejects empty/./.. segments, backslashes, and a leading :. check.add paths must additionally fall under a check.protect glob.
  • The per-copy lock genuinely serializes prepare/report/check/sweep on the same trial copy without serializing across different trials — confirmed via e2e/benchmark-trial-copy.test.ts (a second concurrent next --prepare gets busy rather than a corrupted/double clone; a killed report can be retried cleanly).
  • Judge-at-timeout doesn't double-score (e2e/benchmark-judges.test.ts: exactly one judge launch, run ends timeout, trial unscored); "Not logged in" doesn't corrupt subsequent trials (verified both by the PR's real-CLI evidence and the general error path in takePrompt/settle).
  • Spot-checked 3 of the Evidence table's test-file references against their actual bodies (benchmark-env.test.ts, benchmark-checks.test.ts, benchmark-judges.test.ts) — all three test exactly what their row claims, including several of the listed adversarial cases.
  • npx tsc --noEmit and npx oxlint --type-aware --deny-warnings --report-unused-disable-directives both ran clean on this branch.

Findings. None I can back with a concrete failure scenario or a cited rule violation. I specifically looked for races in the per-copy lock, double-scoring at judge timeout, "Not logged in" corrupting later trials, and path traversal via check.protect/check.add — all are handled and tested.

Not covered / unverified.

  • Could not execute npm run build or vitest myself in my environment (a sandbox policy against running an unreviewed PR's own code) — static tsc/oxlint are clean, but I don't have independent confirmation of the claimed 578 files, 16565 passed / 383 passed e2e numbers beyond reading the relevant test sources directly.
  • Didn't re-audit the pre-existing, unmodified src/update.ts file-lock internals (acquireLock/releaseLock) — treated as trusted existing infrastructure since this PR doesn't touch it.
  • Didn't review the other ~110 changed files outside src/workflow/benchmark/*.ts (pull/review/doctor wiring, dashboard, docs) at the same depth — focused the time on the new module and its safety-critical claims.
  • TRIAL_WITHHELD_ENV only covers git/CI credential-shaped names, not generic cloud credentials (AWS_*, NPM_TOKEN, etc.). Looks like a deliberate scope choice per terms.ts's own comment ("credentials a trial could push, publish or authenticate with") rather than an oversight — flagging it as a question rather than a finding, in case it's worth widening in a follow-up.

Verdict. Comment (approve-leaning, no blocking findings). The two most safety-critical claims — the secret-strip-wins-over-requires.env guarantee and the no-remote/ancestors-only clone isolation — both check out at the actual git-plumbing and negotiation-logic level, not just by the PR description, and the adversarial test cases listed genuinely exist and test what they claim.

@SaulMoro

Copy link
Copy Markdown
Collaborator Author

@STiFLeR7 thanks for the thorough review. You found nothing blocking, so could you approve it?

@STiFLeR7 STiFLeR7 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed — nothing blocking. Approving.

@SaulMoro SaulMoro left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was generated by AI during triage.

Re-review at dde0913. The README benchmarks column, the CHANGELOG entry, the workflows.sandbox docs (EN and zh-CN), the isolated-run hint and the finally case are all resolved. I checked the hint, the refusals and the finally case with the built CLI.

[P1 blocking] A workflow arm that starts with a command gets the wrong tool's team assets. This is the CI review's clone.ts finding, reproduced. Setup: a member with enabledAgents: [claude], a suite with defaults: { tool: codex }, and an arm workflow: codex-flow whose first step is a workspace: continue command, followed by a prompt. The plain Codex arm's clone has .codex/skills/answer/SKILL.md. The workflow arm's clone has no Codex team skill, so its Codex prompt runs without the team's assets, and the table shows that trial as Not verified. Cause: src/workflow/place/clone.ts:362 falls back to member.enabledAgents when the lease has no tool, and a command-first lease never has one. Carry the arm's tool on the lease regardless of the first step, and add a case where enabledAgents lacks the arm's tool.

[P2 non-blocking] The model-less refusal says "Set model on the arm", but the suite rejects model next to workflow on an arm ("an arm changes at most one of the defaults keys"). defaults.model is what works; I ran it and the arm runs on that model. The message should point there.

[P3 nit]

  • The isolated-run hint teamai workflow start agent-fix drops the run's inputs, so it fails with inputs.task is missing. The same restartCommand text is used by the not_retryable refusal.
  • The CHANGELOG says --fail-on-regression fails a job whose pass rate dropped. Per the body, it exits 1 only when an arm's paired drop has its whole 95% interval below zero.
  • The body still does not mention the uninstall change (src/uninstall.ts, src/hooks.ts), or the new refusal and hint.

CI at this head: no failures so far. macOS Node 22/24 and the E2E shards were still running when I checked.

…am assets

Two causes made a workflow arm run without the team assets it compares:

- The clone installed for the tool of the step that opened it, else the
  member's enabled agents. A workflow arm that opens its clone with a
  command got the member's tools, not the arm's. The plan now writes the
  arm's tool into the clone's team selection (enabledAgents), which the
  install delivers to.
- Each step's commit puts the clone's agent files back as the commit has
  them, which takes the delivery out, and a later step that continued
  the clone was never given it again. The install now keeps a copy of
  what it delivered, and a later step gets it back, recorded as overlay,
  as a worktree step gets its overlay.

Proved by a benchmark-workflow-arms e2e: a codex arm whose workflow
starts with a command, on a member who installs only claude, ran its
prompt with no team recorded and no team skill in its clone; it now
runs on the pulled revision with the skill there, as the prompt arm
does. The clone-install e2e files pass (61 tests).
The refusal of a workflow arm with no model whose workflow runs an agent
said to set model on the arm, which the suite rejects: an arm with
workflow changes no other defaults key. It now says to set model in the
suite's defaults, which the arm inherits, and the guide says so too.
Start a new run: teamai workflow start <name> dropped the run's inputs,
so for a workflow that takes any it failed with invalid_inputs. Both the
not_retryable refusal and the failed-run hint now pass them with
--inputs, quoted for a POSIX shell.

Proved by the workflow-from case of a workflow with inputs, which
expected the command without them and now expects it with them.
On Windows, removing a table test's team directory failed with EBUSY in
three CI runs out of about seven, each time in this file. Each test ends
its run, and record publication then runs git in the team directory,
which these tests never read; no other process was seen there. They now
run with usage reporting off, so no publication git runs there.

The holder is not confirmed: git calls there are awaited and fail fast,
as the directory is not a repository. If Windows still fails, the
removal helper should name what holds the directory.
@github-actions

Copy link
Copy Markdown
  • [P2 non-blocking] Classify workflow trials using their child attempts — src/benchmark/result.ts:279. A workflow step has no attempt, so cancelling the suite while its child prompt runs records the trial as not_run instead of error. Conversely, an unavailable tool fails the child without propagating tool_unavailable, so the workflow trial becomes error instead of not_run. Inspect the child attempts when classifying workflow arms.

  • [P2 non-blocking] Keep workflow fixed in Team changes — src/benchmark/history.ts:453. configKey() includes only tool/model/effort. Arms running different workflows on different team revisions therefore become one chart series, presenting their workflow-dependent results as a team-revision change. Include workflow identity in the grouping and labels.

Earlier findings: Resolved, including tool installation for command-first workflow arms.

Testing: The description documents sufficient built real-CLI verification. Coverage deferred to CI is not blocking.

Reviewed only 64b031ed...283cbc1 against the trusted base criteria. Ran no PR code, builds, tests, or installs.

…flow ran

A workflow step has no attempt of its own; its prompts do. So a workflow
arm cancelled while its prompt held a lease read as not_run, and one
whose tool the driver could not launch read as error, with no line
naming the arm. A trial's result, and that line, now read the workflow
step and every step under it: not run until one of them was leased.

Proved by two benchmark-workflow-arms e2e cases, red before (error and
not_run swapped) and green after; benchmark-lifecycle and
benchmark-local, which pin the same rules for prompt arms, pass.
Team changes grouped arms by tool, model and effort only, so a prompt
arm on one team revision and a workflow arm on the next drew one line,
showing a workflow change as a team change. A line now also keeps its
workflow, which its chip names.

Proved by a benchmark-history case, red before (one line across both
revisions) and green after.
@github-actions

Copy link
Copy Markdown
  • [P2 non-blocking] Bound pre-launch observation by the suite deadline — src/workflow/place/clone.ts:250. cloneHead() explicitly uses no deadline, and promptReport() awaits it before launching the tool. On a large checkout, a slow git status can exceed the remaining suite time; cancelling the prompt does not interrupt this read. Pass the lease deadline through this path.

  • [P2 non-blocking] Count historical child attempts when classifying cancellation — src/benchmark/result.ts:269. After a workflow child completes a loop pass, repending clears its current attempt. Cancelling before its next lease makes unleased() return true because it ignores passes and archive, recording an already-run trial as not_run instead of error. Include historical child attempts.

Earlier findings: The previously reported concrete cases are resolved; the loop-history case is distinct.

Testing: The description includes sufficient built real-CLI verification. Coverage deferred to CI is not blocking.

Reviewed only 64b031ed...193cb453 against the trusted base criteria. Ran no PR code, builds, tests, or installs.

…l is classified

A trial cancelled between two loop passes has no current attempt, though an earlier
pass of its work was leased and kept in passes or the run's archive. It read as
not_run, which drops a trial that ran from the error count.

Tested: benchmark-table cases for a pass kept on the step and one archived.
… deadline

cloneHead ran git with no deadline, so a filter or config a step left in its clone
could hold the local driver before launch past the suite's time budget.

Tested: workflow-clone-place case with a git that never returns on status.
@github-actions

Copy link
Copy Markdown
  • [P2 non-blocking] Preserve null JSON outputs — src/workflow/engine.ts:510. A step declaring output: { value: json } can successfully report { value: null }, but exposing it through outputs: { value: steps.emit.outputs.value } silently drops value from done.outputs. This treats valid JSON null as a missing path. Distinguish missing outputs from explicit null values.

Earlier findings: Resolved in the current diff, including pre-launch observation deadlines and historical child-attempt classification.

Testing: The PR description includes sufficient built real-CLI verification. Coverage deferred to CI is not blocking.

Reviewed only cd9f3274...ce7d81bf against the trusted base criteria. Ran no PR code, builds, tests, or installs.

runOutputs dropped every output that read null, because resolvePath reads a missing
path as null too. A step declaring a json output can report null, and the run's
done.outputs then lost it. pathValue reads a missing path as undefined, so only an
output no step gave is left out.

Tested: workflow-protocol case with a null output and one from a skipped step.
@github-actions

Copy link
Copy Markdown
  • [P2 non-blocking] Reject dynamic arm workflows during preflight — src/benchmark/arm-workflow.ts:147. A valid arm workflow with a workspace: continue producer followed by workflow: { from: producer } passes this guard and is copied unchanged. Bundle admission later rejects its from step, so benchmark run creates a run that fails with invalid_output. Reject this unsupported workflow during preflight, before creating the run.

  • [P2 non-blocking] Classify unavailable tools independently of command attempts — src/benchmark/result.ts:279. A Codex workflow arm that starts with a command leases that command before discovering Codex is unavailable. Consequently, unleased(work) is false: the trial becomes error rather than the documented not_run, and its unavailable-tool notice disappears. For tool availability, distinguish command attempts from prompt attempts.

Earlier findings: The previously reported concrete cases are resolved, including null JSON outputs. The command-first classification case above is distinct.

Testing: The description documents sufficient built real-CLI verification. Coverage deferred to CI is not blocking.

Reviewed only the specified three-dot diff against the trusted base criteria. Ran no PR code, builds, tests, or installs.

…} step

A trial runs the arm's workflow from the benchmark plan's output, and admission
refuses a from step there. The run started anyway and failed with invalid_output;
the preflight now refuses the arm before any run, naming the step.

Tested: benchmark-workflow-arms e2e case, which created a failed run before.
…un when it starts with a command

The arm's first step, a command, is leased before its prompt finds the tool missing,
so the trial counted as leased and read error, and its unavailable-tool notice was
gone. Only a prompt's lease now shows the tool launched.

Tested: benchmark-workflow-arms e2e case with a command-first workflow on codex,
which read error before.
@github-actions

Copy link
Copy Markdown

No new confident findings in the specified diff.

  • Earlier findings: Resolved, including dynamic arm-workflow preflight rejection and command-first unavailable-tool classification.
  • Testing: The PR description documents sufficient testing, including built real-CLI verification and representative E2E results. Provider, agent, and Windows coverage deferred to CI is not blocking.

Reviewed only cd9f3274...a235078c against the checked-out base review criteria. Ran no PR code, builds, tests, or installs.

@github-actions

Copy link
Copy Markdown

No new confident findings in the specified diff.

  • Earlier findings: Resolved, including dynamic arm-workflow preflight rejection and command-first unavailable-tool classification.
  • Testing: The PR description documents sufficient testing: built real-CLI verification and representative E2E results. Provider, agent, and Windows coverage deferred to CI is not blocking.

Reviewed only ca2c5838...b17c09b6 against the checked-out base review criteria. Ran no PR code, builds, tests, or installs.

… its envelope

A refused report surfaced only as a ZodError on data, which does not say which step
was refused or why. The trial-timeout case failed that way on CI and does not
reproduce locally, so the next failure needs to name its cause.

Tested: benchmark-driver-contract e2e, 7 cases.
enabledAgents is new in workspace.clone.team.selection, and no key of selection
was listed. Name them all, with the default enabledAgents takes.
… the run deadline

Both cases ran a second arm the driver cannot run, so the run ended only at its
15 s timeout, and the claude trial had to report before it. With the trial
timeout's 3 s hang, clone and check, that margin was thin on a loaded runner:
the trial timeout case failed in 2 of 4 CI runs with a refused report. The
codex arm proved nothing these cases name, and the first case still covers the
suite timeout settling a trial not_run. One arm and a 60 s run deadline end the
run with its trial.

Tested: benchmark-driver-contract e2e, 7 cases; the two cases now take 8 s and
5.6 s, without waiting on the run deadline.
@github-actions

Copy link
Copy Markdown

No new confident findings in the specified diff.

  • Earlier findings: Resolved, including dynamic arm-workflow preflight rejection and command-first unavailable-tool classification.
  • Testing: The PR description documents sufficient testing: built real-CLI verification and representative E2E results. Provider, agent, and Windows coverage deferred to CI is not blocking.

Reviewed only de832803...b067ae94 against the checked-out base review criteria. Ran no PR code, builds, tests, or installs.

benchmarkRunConfig names a real GitHub URL as the team remote, so a run that
ended published its record with git ls-remote against github.com from a unit
test. On Windows the team directory then stayed busy when the test removed it
(EBUSY), first in benchmark-table, now in benchmark-trials. The config now
publishes nothing; the one test of the record queue opts in, and publishes
nothing either.

Tested: every user of benchmarkRunConfig (5 unit files, workflow-conformance unit
and e2e) with a logging git on PATH: no ls-remote, fetch or push left.
@github-actions

Copy link
Copy Markdown

No new confident findings in the specified diff.

  • Earlier findings: Resolved, including dynamic arm-workflow preflight rejection, command-first unavailable-tool classification, and the previously reported protection, validation, and deadline issues.
  • Testing: The PR description documents sufficient testing: built real-CLI verification and representative E2E results. Provider, agent, and Windows coverage deferred to CI is not blocking.

Reviewed only de832803...677af108 against the checked-out base review criteria. Ran no PR code, builds, tests, or installs.

@SaulMoro SaulMoro left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was generated by AI during triage.

Re-review at 677af10. Everything from the earlier rounds is resolved, and the description now covers the clone redelivery, the null output, enabledAgents, the arm refusals and the restart hint.

The commits since my last comment are test and docs changes:

  • The selection keys are documented in both languages, and they match src/workflow/place/clone.ts.
  • The outside-driver trial-limit cases now run one arm with a 60 s run timeout, so only the trial limit can fire. That was the intermittent workflow report refusal in E2E.
  • The in-process benchmark fixtures no longer publish, which removes the Windows EBUSY on teardown.

CI is green at this head: all 18 checks. macOS Node 24 needed one rerun for a workflow-workspace timeout; it passed on the rerun.

[P3 nit] The restart hint quotes --inputs POSIX-style, as the other printed commands do, so it may not paste into PowerShell as-is.

@jeff-r2026 this one is ready for your merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants