Repository navigation
fix(workflow): a session with no tool is asked for it, and a resume's --tools replaces the run's - #1481
Conversation
|
No findings I can confidently identify in the requested diff. The PR description satisfies the testing requirement: it records a build and representative real-CLI verification at Review was read-only; I did not run, build, or install anything from the PR. |
f1e1a5b to
b793133
Compare
|
No findings I can confidently identify in the requested diff. The PR description satisfies the testing requirement: it records The earlier review reported no findings, so none require a resolution update. Review was read-only and limited to the specified diff; I did not run, build, or install anything from the PR. |
…tool} slot
Describe told a session with no tool, in prose, to describe again with
--tools, yet still offered starts without it. An agent that ran one
anyway started a run whose modeled steps fail and whose scored steps go
unjudged. Putting the slot on those starts would not do: describe chose
its question, judging included, before it knew the tool.
When the session names no tool and the workflow has a prompt or scored
step, describe now answers ask.kind tool (R27): the reason, a describe
literal with --tools default='{tool}', and the ids of the tools teamai
knows. It offers no start, so the agent fills the slot and describes
again, and that reply asks the person with judging known. An unfilled
slot is refused as a tool teamai does not know. The workflow skill
lists the new kind beside the others.
Tested: R27 answers tool in attended and unattended runs, the filled
describe offers starts that keep the tool, and a run of commands alone
still starts directly (seen failing first answering start); the filled
literal parses back with its tool.
A run started in Oh My Pi with --tools default=omp and resumed from Qoder with --tools default=qoder, neither host setting a session variable teamai reads, kept omp: S1 filled the tool only when the run had none. Its children's models and done.score still named Oh My Pi. A tool the call declares now wins over the stored one. Tested: a run started with --tools default=pi and resumed with --tools default=claude dispatches its step with the model resolved for claude (seen failing first with no dispatch).
… refused A start without --tools from a session whose host names no tool still ran a workflow with prompt or scored steps. Its judging was decided with no tool, so it judged nothing, and naming the tool on a later resume could not bring judging back. The tool is now one of the answers such a start must carry: it is refused with confirm_needed, whose next is the describe that asks for the tool. Tested: the start of a scored prompt workflow without a tool is refused and points at describe, while one of commands alone starts (seen failing first starting the run). Two tests that started a run with no tool now reach that state the only way left, a takeover by a session that names none.
…eds is refused
A resume or takeover from a session with no tool still drove a run with
prompt or scored steps here, deciding its judging with no tool, as an
unnamed start did. The resume now answers tool_needed (R28) before it
takes the run, and its next is the resume with --tools default='{tool}',
a slot the agent fills with its tool, as describe's ask.kind tool has.
R11's next takes the same slot for a session with no tool, in place of
the sentence that told it to add the flag. The workflow skill says to
fill a {tool} slot in next before running it.
Tested: a resume and a takeover of a scored run from a session with no
tool answer tool_needed with the slotted resume, and the resume with
the tool drives it (seen failing first driving the run).
…t prompt steps The host question, whose h4 turns the session's judging off, was asked only of runs with a prompt step. A run whose workflow step scores a sub-workflow of commands alone, judged by a Pi session, skipped it, so bare Pi got a done.score that needs a fresh subagent it cannot start. R10 and R19 now ask it when the session judges the run, too. Tested: such a run under a Pi session asks the host question, and h4 turns judging off with no_child_agent (seen failing first with no question).
…needs it Requiring the tool for every prompt step refused a start, and asked describe for a tool, where it changes nothing: a prompt step that pins no model and is not scored ran before without one. Describe now records the prompt steps whose child runs a model teamai resolves for the tool (the step's model, or defaults.model for a step with no agent, scoped as the engine scopes it), and needsTool asks only for those and for scored steps. Tested: describe in a session with no tool asks for one for a workflow whose defaults pin a model, and answers start for a prompt step with no model and no scorer (seen failing first asking for a tool). The user-scope skill run test, which starts such a workflow with no tool, passes again.
…r its judge Asking the host question of a run its session judges let the answer pick the host profile of a run with no prompt step. On h4 that declares a read-only workspace and one child at a time, so a command step with `workspace: new` was held for good. The pending host now records whether the run has a prompt step. When it has none, the answer only decides judging (h4 still turns it off), and the host stays `none`, so its commands run with h1's profile as before. A state kept before this change always had a prompt step. Tested: the command-only judged run now reaches done under h4 in a new worktree, where it was held before.
…ent to the person When the only answer a start lacked was the session's tool, the refusal still said to ask the person. The describe it points at answers `ask.kind: tool`, which the agent answers itself, so the message now stops at running the describe literal. Tested: the refused start test asserts the new message.
…s scored steps A session that names no tool was refused a start with `--no-judge` of a scored workflow whose steps pin no model, though nothing in that run needs the tool. Scripted `--driver skill` use and hosts teamai has no tool id for could run such a workflow before. A scored step now counts toward needing a tool only when the run is judged. Describe still asks for the tool, since it offers judged starts. Tested: a no-judge start of the scored review workflow from a session with no tool now starts; before, it was refused with `confirm_needed`.
…ns of the call
The next of `tool_needed` was a bare `resume <run> --driver skill
--tools default='{tool}'`. An agent that filled the slot and ran it lost the
person's `--parallel`, HITL mode and `--no-judge`, and without `--confirmed` an
`invoke: ask` workflow went back to the person for a second yes.
The refusal now carries the resume's options and its `--confirmed`, and the
next repeats them with the tool slot. It leaves `--claim` out: the claim the
refused call made stands.
Tested: a resume with `--parallel 2 --auto-approve --confirmed` is refused
with a next that keeps all three; before, it dropped them.
…ne caller The default for the tools argument of askFor had no caller that used it.
…s the session's An agent whose tool is not among ask.tools had no step for ask.kind tool, and guessing an id would resolve models for another tool. It tells the person ask.reason instead.
…kill.json to drive the run R28 refuses after the resume has claimed the run. When this machine kept a skill.json from driving the run before another install took it, the claim installed the published run over it, and the refusal left that skill.json in place. The retry its next names then found the run already owned, so nothing was installed, and the old skill.json drove it: no host question, a stop for an attempt from before the claim, and the resume's options ignored. R28 now removes the skill.json kept here, which R19 would have replaced, so the retry readies the run anew. This also covers an unattended resume, which claims before R28 answers. Tested: CI drives a run, a member takes it over and publishes it, and a CI session that names no tool claims it back. It is refused with tool_needed, and the resume its next names, with the tool filled, asks the host question. Before the change, that resume answered with a stop for the stale attempt.
b793133 to
d778105
Compare
|
No findings I can confidently identify in the specified diff. The PR description satisfies the testing requirement: it records Earlier reviews reported no findings, so none require a resolution update. Review was read-only. I did not run, build, or install anything from the PR. |
SaulMoro
left a comment
SaulMoro
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Approving at d778105. All three findings from #1480's CI review and both nits from our approval are fixed, each checked with the built CLI on main and at f1e1a5b. The rebases onto main keep every commit as it was (range-diff identical). The three commits since then are small: when a claim back is refused only because the session names no tool (R28), the stale skill.json is removed so the retry readies the run again, with a transition test for the takeover-and-claim-back path; the skill tells the agent to relay ask.reason when none of ask.tools is its tool; and askFor loses a default no caller used.
| Case | main | this PR |
|---|---|---|
Resume with --tools default=claude on a run started in omp |
keeps omp (model anthropic/claude-opus-4-5, judge omp) |
switches to claude: model opus, done.score and the lease judge claude (qoder the same) |
| Start without a tool the run needs | starts, the step fails with model_unavailable, judging is lost |
confirm_needed; its next is the describe, which returns ask.kind: tool with a {tool} slot and the 24 tool ids |
| Resume or takeover without a tool | runs with no tool | tool_needed; its next keeps the call's options (--parallel 1 --auto-approve, --no-judge); filling {tool} works |
Commands-only scored workflow, host answers h4 |
never asks; still offers done.score |
asks; h4 turns judging off and the command runs to done |
| Mixed builds | runs and skill.json written by main continue here, and the reverse |
npx tsc --noEmit and npm run lint exit 0; the PR's 5 test files (217 tests) pass, also with a symlinked TMPDIR, and 12 of the new tests fail on main. The three E2E shards pass; the types.test.ts failure in Lint & Test is main's (fixed by #1483).
[P3 nit] A host outside teamai's tool list gets stuck on a scored workflow that pins no model: describe asks for a tool and offers no start without judging, though start <wf> --no-judge works. Unlikely, since teamai installs the workflow skill only into hosts on that list.
Covered by unit tests only: a refused --claim resume removing the local skill.json (needs two installs), --confirmed kept in the tool_needed command, unattended start and resume.
… --tools replaces the run's (Tencent#1481) Follow-up to Tencent#1480. Describe asks a session that names no tool for it, with a {tool} slot; a start or resume that needs a tool the session does not name is refused with tool_needed and a next that keeps the call's options; a resume's --tools replaces the tool the run had; a run of commands alone without judging needs no tool. Refs Tencent#1479
Summary
#1480 merged with three findings from the CI review still open. A resume that names another tool kept the old one. A run started without a tool could not get its judging back. A run whose only scored step is a
workflow:step of commands never asked the host question, so bare Pi got adone.scoreit cannot act on. This follow-up fixes all three, and the two nits from the approving review. It also makes describe ask a session that names no tool for one, with a slot the agent fills, rather than offering starts that drop it.--no-judge, so a later resume cannot tell an opt-out from "no tool at start". Instead, a run that needs a tool cannot be created without one.model, ordefaults.modelfor a step with noagent). A prompt workflow with neither, or a scored one started with--no-judge, still runs without--tools, as it did before feat(workflow): sessions in hosts teamai cannot detect name their tool with --tools (#1479) #1480.nexttakes the same{tool}slot for a session with no tool. It replaces the sentence feat(workflow): sessions in hosts teamai cannot detect name their tool with --tools (#1479) #1480 added.skill-data/workflow/SKILL.mdsays to fill the slot.Type of Change
Test Plan
npx tsc --noEmitpassesnpm run lintpassesnpx vitest run: 17753 passed. Oneworkflow-reconciliation.test.tscase timed out at 15 s under local load; main at 632d180 times out on the same file on this machine, and this PR does not touch it.Added/updated tests for the change. Each new test was seen failing first: R27 (
ask.kind tool), R28 (tool_neededon resume and takeover, its options, and the skill.json a claim back leaves), the refused start and the--no-judgestart, the declared resume tool, and the host question of a session-judged run of commands that writes in a new worktree.npm run test:e2e -- workflow-skill-claim score-external-driver workflow-session-1086 workflow-prompt workflow-conformance(the files that drive the skill driver and session judging): 5 files, 174 passed, at f1e1a5b, before the rebase over fix(config): treat sessions as a member setting so a workspace sees the machine value #1483, which changes onlysrc/types.ts.npx vitest run src/__tests__/workflow-skill-transitions.test.ts: the transition table matchesdocs/dev/workflow-engine.mdcell by cell.Rebased onto main 2faf53d, whose two new commits share no file with this PR:
tsc, lint, and the skill driver, protocol, records and generated-doc suites (220 tests) pass at d778105.Real CLI after
npm run buildat b793133, in a shell that sets no agent session variable,omp18.12.6 on PATH. Base is main 632d180 built the same way. The script inits a team with a modeled, scoredreviewworkflow and agatedworkflow whose only step is a scoredworkflow:step of oneworkspace: newcommand, then runs each printed literal as the agent would.Record
Before, a start without
--toolsdrove a run whose step model and judging need a tool. After, describe asks for the tool, the start that skips it is refused, and the filled describe drives the run: the runner resolvesstrongforomp,done.scoreleases judging toomp, and the stored label hasjudge.tool: omp. The run of commands alone, judged by apisession that answersh4, turns judging off and still runs its step todone.Related Issues
Follow-up to #1480. Refs #1479.
Notes for Reviewers
docs/dev/workflow-engine.md. R3, R4, R10, R11, R13, R18, R19 and S1 changed. The transition table test checks them cell by cell.next --takeovertakes no--tools, and keeping the old tool would resolve child models for a tool the new session may not run. The judging decided at start stays. The model failure and R11 say how to name the tool.workflow.factsgainsmodeled.WorkflowClient.resumehas installed the published run. When this machine kept a skill.json from driving the run before, R28 removes it, as R19 would have replaced it, so the resume its next names readies the run anew instead of driving the old state.none, soh4does not hold its commands in a worktree. skill.json's pending host recordshasPromptStep, and a state kept before reads as having one, which it did.