Repository navigation
feat(workflow): enforce scoped requirements and execution limits - #1303
Conversation
- Parse workflow and step requirements, inherited defaults/setup, duration limits, token budgets, and max_turns; negotiate member ceilings and driver capabilities before both new leases and restart handbacks. - Resolve declared env through TeamAI, give commands and tools only their required team keys, reject input/env collisions, and mask every report string plus local result artifacts before persistence. - Run setup once per new workspace after the base overlay, with separate credentials; reject tracked edits or retained secrets, and stop setup processes on timeout or interruption. - Persist deadlines, account for current and archived token passes, cancel outstanding leases on exhaustion, reject late reports, and enforce max_turns only for supported headless Claude launches. - Add negotiation, masking, setup, archive-budget, concurrency, protocol conformance, and real-CLI safety coverage; update bilingual workflow/env docs and generated help. Typecheck, lint, build, targeted units, and CLI safety verification pass.
The PR description documents sufficient testing, including a representative real-CLI run after build. No testing-record finding applies. I did not run, build, or install PR code. |
- Enforce the minimum of enclosing step and nested workflow timeouts, and negotiate/deliver member network ceilings for setup on new and returned leases. - Poll the engine while parallel workspace preparation and setup run, cancelling expired siblings and stopping cancelled preparation before any agent launch. - Mask parsed command output before writing result artifacts. Sanitize all raw tool artifacts before schema validation and error returns, removing malformed artifacts when they cannot be safely parsed and rewritten. - Add regressions reproducing nested deadline precedence, setup network policy, a timed sibling's delayed write during long setup, multiline JSON secrets and invalid result schemas; retain explicit before/after failure evidence. - Verify 146 relevant unit tests, a final 50-test requirements/adapter run, typecheck, lint and the build-first real CLI safety round trip.
All five earlier findings are resolved in the current diff. The PR description documents sufficient testing, including a representative real-CLI run after build; no testing-record finding applies. I did not run, build, or install PR code. |
SaulMoro
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
@nateEc Thanks. This branch conflicts with main in src/workflow/types.ts and src/__tests__/workflow-records.test.ts. Main caused it: #1234 (2401d047) added workflow_namespace_conflict and prompt_namespace_conflict to the WorkflowDiagnostic['code'] union and to the diagnostics list in the records test, on the same lines this PR extends. Keeping both sets of codes should resolve it. We'll review the rebased head. #1303, #1304 and #1306 also share most of their workflow files (types.ts, parse.ts, local-driver.ts, src/index.ts, the workflow guides), so whichever lands first will need the other two rebased again.
@nateEc an update to the request for changes: the conflict with |
# Conflicts: # docs/dev/workflow-engine.md # skill-data/core/references/commands.md # src/__tests__/workflow-records.test.ts # src/index.ts # src/workflow/conformance.ts # src/workflow/engine.ts # src/workflow/local-driver.ts # src/workflow/local-tool-adapter.ts # src/workflow/parse.ts # src/workflow/step-state.ts # src/workflow/types.ts
… and tool guard Classify the new step startedAt and lease startedAt/deadline fields for run handover, use isLaunchTool after its move to tool-launch, and name the two environment failure codes the records test requires.
Restore the files the first attempt touched to their main content so the requirements work lands as if it had been there from the start.
Move the scenario interpreter into conformance.ts so the in-process and CLI runners run the same operations. Operations can speak as a local, session or external driver profile, and an advance operation moves the clock.
driver.can is parsed by one schema, and its unknown keys are derived from it. Both places that hand out a lease ask one negotiate function, so an interactive step without can.hitl waits for a driver that has a person instead of failing hitl_required, and a hand-back to such a driver gives the lease up. wait.held lists each held step with what it lacks.
Steps take requires: and a workflow file takes defaults.requires, which acts as if written on every step; nearer keys win. network and isolation are best effort: a driver that cannot enforce one still gets the step, and the lease, the settled step, the report reply and the run record say what was not enforced. workflows.sandbox makes the local driver declare sandbox isolation.
requires.env names the keys a step needs. Only a driver that declares can.env takes such a step; the lease carries envScope, the keys to provide and the ones to withhold, by name. A required key with no value fails missing_env before the step runs, a step env: key that shadows a team or required key fails env_conflict, and workflows.allow refuses a step that needs more than the member allows. workflows.max_parallel, when set, caps the open leases of any driver. The local driver gives each command and tool only the keys its lease provides.
Every run-file write, queued record, and printed workflow envelope masks the values teamai treats as secret: environment values that look secret and the team's declared secrets. The pinned definitions stay as pulled. The secret set is resolved once per operation, before the run lock. start refuses inputs or a pinned workflow or scorer that holds a known secret value with invalid_inputs, naming the input and the secret, never the value. An input may share a name with an env key.
A step's timeout bounds its lease, or the workflow or items it runs, and a workflow file's timeout bounds its run. Deadlines are stamped when a lease is issued or a workflow step starts, and apply on the first operation after they pass, outer first. A timed-out step fails with code timeout, which expressions and records read as outcome timeout, and cancels what it runs. A run past its timeout cancels every open lease and fails. An expired lease of another driver no longer holds the run, and its late report is lease_cancelled.
budget: { tokens: N } on a workflow file or a workflow step bounds the input
and output tokens its steps spend, over every pass, archived ones, and
attempts a driver released. A workflow that spent its budget cancels what it
still runs and fails with budget_exceeded: the run, or the workflow step, so a
step around it that reads its outcome can recover. A prompt under a budget
waits for a driver that declares can.tokens, and a success report without
tokens fails budget_unmeasured. Judge work stays outside every budget.
max_turns on a prompt step reaches the lease as maxTurns only for a driver that declares can.max_turns; any other driver gets the step with max_turns listed as unenforced. A report may add max_turns to what its lease left unenforced, when the tool it ran cannot cap turns, and the run records the union. The local driver passes --max-turns to headless Claude and reports max_turns as unenforced for other tools and for interactive prompts.
A for_each step's timeout bounds the whole fan-out, so its items no longer copy it onto their own leases. An item still takes the timeout of the workflow file it runs, and the for_each step's deadline reaches its items' leases through the step they run under.
A workflow file can declare `setup: { command, requires: { env } }`. A
lease that names a workspace carries the setup in effect, and a driver
without `can.setup` waits for such a step. Setup's env keys negotiate like
a step's, reach only setup, and are withheld from every step that does not
require them.
The local driver runs setup last in `stageLease`, only for a worktree it
just created, and fails the step before it runs on a non-zero exit
(`setup_failed`), a change git would commit (`setup_dirty`), or a setup key
value left in a file setup created or changed, ignored files included
(`setup_secret`). Any failure removes the worktree. A marker in the
worktree's git dir makes a worktree whose setup never finished be made
again.
The local driver awaited a lease's staging inline, so while setup ran it did not ask the run again, did not see a cancel, and could start the step after the run had ended. Staging and setup now run inside the in-flight attempt for every max_parallel: a command enters the attempts at once and a prompt's attempt begins with staging, so the driver keeps polling and a cancel aborts whichever phase is running. Setup runs in a process group of its own, which an abort stops. A lease cancelled or past its deadline once staged never starts its command or tool, and reports nothing.
The run deadline counted from the run's start, so after a run ended by its workflow timeout, `teamai workflow retry` reopened it and the next call timed it out again: the run could not be recovered. A retry that reopens a run now records `timeoutFrom`, which the run deadline counts from, the way a retried workflow step's timeout already counts from the retry. The field is published, so a claim keeps it.
The local driver kept a step's raw output, secret values included, in the attempt's result.json after the report. Mask that file before reporting, like the run file, records and CLI output.
The other six earlier findings are resolved: both nested timeout bounds, fan-out timeout ancestry, parallel and single-worker setup polling, parsed-output masking, and invalid-artifact sanitization. The PR description documents sufficient testing, including a representative real-CLI run after build. No testing-record finding applies. I did not run, build, or install PR code. |
…quired value Masking the value of every key a run requires took the environment's value of a required key out of the mask set when a team value shadowed it, made start refuse a workflow or input holding an ordinary required value such as a deploy target, and gave resume --claim a different set from the run file. A key declared in env/secrets.yaml is secret on every machine, and CI resolves it from the environment, so masking follows those declarations again: declared secrets, wherever this machine resolves them, and environment values whose name looks secret.
…r decide A staging failure such as workspace_base or base_config marked the copy used, so the retry its message asks for got a lease without setup and ran the step in a fresh worktree that setup never prepared. Every lease with a workspace in a workflow that declares setup now carries it and needs can.setup. The driver runs it when it creates the worktree or finds its setup unfinished. A copy is used only once a report shows the step ran there: a success, an exit code, changes or a read.
…e in it A held hand-back dropped the owner of an unused copy even when the same driver still held another lease there. Once that lease reported, the copy was used with no owner, so another driver could take the next step in it and work on a branch whose worktree lives on the first driver's machine. The owner is now dropped only when no other step holds a lease in the copy, and a granted hand-back records its driver as the copy's owner.
- 基于维护者更新的 008f00e 继续修复,保留完整的需求协商、预算与密钥处理重构。 - shell 退出后持续复查进程组,已退出时立即结束等待;仍存活的后代继续保留五秒宽限期和 SIGKILL 升级。 - 新回归在修复前复现整段等待;7 个进程停止单测、10 个 setup 用例和 6 个真实 CLI 安全用例通过。 - 补充中英文网络策略边界说明;构建、类型检查和 lint 通过,Linux 运行结果由本次推送后的 CI 验证。
|
Addressed the review follow-up at
The PR description contains the updated verification record. Ready for another review. |
SaulMoro
left a comment
There was a problem hiding this comment.
@nateEc Thanks for this work. We are taking over #1303, #1304 and #1306 to speed up delivery. We will push the remaining changes to your branches, bring them up to date with main, and keep you as co-author in the squash commit. #1303 and #1306 go first, and #1304 follows once both have merged.
Please hold off on pushing to these branches in the meantime. All three stay blocked until we finish.
The six earlier timeout, cancellation, and artifact-value findings are resolved. The earlier setup-network finding no longer applies to the explicitly narrowed v1 contract documented in this diff. The PR description documents sufficient testing, including representative real-CLI verification after build. No testing-record finding applies; the older tested commit and additional coverage deferred to CI are not blocking. I did not run, build, or install PR code. |
…pts ledger The two hand-back sites that cancel an attempt pushed it to cancelledAttempts directly, so a late report from its driver was answered lease_cancelled but its tokens were dropped. Both now go through stopAttempt, like every other place the engine stops an attempt.
SaulMoro
left a comment
There was a problem hiding this comment.
@nateEc Thanks for this work. We are taking over these three PRs to speed up delivery:
We will push the remaining changes to your branches, bring them up to date with main, and keep you as co-author in the squash commit. #1303 and #1306 go first, and #1304 follows once both have merged.
Please hold off on pushing to these branches in the meantime. All three stay blocked until we finish.
- 递归遮蔽 JSON 对象键名中的已知秘密,避免计算属性名经输出、尝试产物或运行记录泄露原值。 - 在应用团队环境覆盖之前按秘密声明收集个人环境值,使普通名称 VALUE 的个人值与团队值都参与输入拒绝和脱敏。 - 两项新增回归修改前失败;292 项相关单测、含秘密键名的 6 项 CLI 安全 E2E、构建、类型检查和 lint 通过。
|
Addressed the review follow-up at
The PR description contains the updated verification record. Ready for another review. |
All earlier findings are resolved in the current diff, including the setup-network finding under the documented narrowed v1 contract. The PR description includes sufficient built-CLI verification. The older tested commit and coverage deferred to CI are not blocking. I did not run, build, or install PR code. |
|
Acknowledged the maintainer takeover; I will make no further pushes to this PR branch. The secret-key and shadowed-personal-secret fixes were pushed at c474193 before I read the takeover request. The latest review confirms those earlier findings are resolved, but now identifies the PWD/live-lease identity masking collision in maskedRun. That newer finding remains for the takeover implementation; I am not marking this PR ready or overriding your changes. My local validation and the prior fixes remain recorded in the description. |
…wip/1303-r3 # Conflicts: # docs/guide/workflow-advanced.md # docs/guide/zh-CN/workflow-advanced.md # src/__tests__/workflow-engine.test.ts # src/env-resolution.ts
|
No new findings in the specified diff. All earlier findings are resolved: nested and fan-out timeout handling, setup cancellation polling, artifact sanitization, JSON-key masking, personal-secret masking, and live lease identity preservation. The setup-network finding no longer applies under the documented narrowed v1 contract. The PR description includes sufficient built-CLI verification. The older tested commit and additional provider/agent coverage deferred to CI are not blocking. I did not run, build, or install PR code. |
All earlier findings are resolved; the setup-network finding no longer applies under the documented narrowed v1 contract. The PR description documents sufficient built-CLI verification. The older tested commit and coverage deferred to CI are not blocking. I did not run, build, or install PR code. |
Each item took the step's budget as its own, so items that together spent past it all succeeded. The step now checks what its items spent together and stops them once they pass it; an item keeps only the budget of the workflow file it runs.
Setup runs in its own process group, which the terminal's Ctrl-C does not reach. With max_parallel 1 the driver left SIGINT to an attached command, or ignored it for an interactive tool, already while setup ran, so Ctrl-C ended teamai and left setup running, or was lost. The driver now handles every stop signal until the command or tool starts.
|
No new findings in the specified diff. All earlier findings are resolved, including aggregate fan-out budget enforcement and terminal cancellation during setup. The setup-network finding no longer applies under the documented narrowed v1 contract. The PR description includes sufficient built-CLI verification. The older tested commit and additional provider/agent coverage deferred to CI are not blocking. I only read the specified diff; I did not run, build, or install PR code. |
|
@nateEc Thanks for this work, and for the stop-polling and masking fixes during the takeover. To speed up delivery we closed the remaining review points on your branch and merged it. Your authorship and co-authorship are kept in the squash commit. |
…1306 Conflicts, resolved so both features hold: - step-state.ts: RunningSchema takes Tencent#1303's expiresAt and limitsFrom; DeliveringSchema and the shared variant list stay. CancelledSchema keeps settledFields, which now carry Tencent#1303's unenforced. cancel() keeps a delivering step's settlement and uses Tencent#1303's stoppedLease for a leased one. stopWhere, which Tencent#1303 moved here, cancels a delivering step as before. - engine.ts: this branch's stopWhere is dropped for Tencent#1303's. The drain, the gate and the report vetting take the run's secrets and reconcile with a time, as updateRun, queueRunRecord and reconcileRun now require. leaseNext resolves the env before the drain, so its writes mask the same secrets. parseReport reads unenforced and still names unread changes fields. A report marks its copy used and may start a delivery. - types.ts, snapshot.ts: copies hold from and used; deliveries, creating, releasedTokens, stoppedAttempts, limitsFrom, unenforced and expiresAt each keep a field rule. - conformance.ts, conformance-interpreter.ts and both runners: the runner moved into Tencent#1303's interpreter. Scenarios send base instead of workspace: true, since every profile can take workspaces, and the interpreter fills a report's changes from its saved lease. - parse.ts, records.ts, run-controls.ts, workspace.ts, the handoff and engine tests and docs/dev/workflow-engine.md keep both sides. - Tests call startRun, cancelRun, retryStep and updateRun with secrets, and pass workspace modes in can.
Summary
A workflow can now say what a step needs (
requires,defaults.requires,setup) and how far a run may go (timeout,budget,max_turns). Before handing out a lease, the engine checks what the step needs against what the driver says it can do (driver.can) and what the member's config allows. The rules live in the engine and the CLI protocol, so the local driver, a session driver, and an external driver that only speaksteamai workflow next/report --jsonall get the same behaviour.requires.env,interactive,setup, tokens of a headless budgeted promptrequires.network,requires.isolation,max_turns, withholding other env keys, tokens of an interactive promptSecret values never enter a run: leases carry key names only (
envScope), and every known secret value is masked where data leaves teamai (run file, run record, everyteamai workflowoutput, the local driver's attemptresult.json).startrefuses inputs that hold one. A known secret is the value of a key declared inenv/secrets.yaml(also when this machine takes it from the environment, as CI does) or of an environment variable whose name looks secret.Type of Change
interactivestep leased to a driver without HITL now waits for one instead of failinghitl_requiredDesign notes
unenforcedrecords the gap (decision of Oct 15). Strict requirements hold the step until a driver that declares them asks; the run never fails midway because one driver lacks a capability.workflow runwithout a terminal exitsbusynaming the missinghitl. Gates (approve,choose) keep their existingauto_approvebehaviour.negotiate(requirements.ts) is pure and is the only place that decides grant, hold or fail;offerTermsbuilds its input for both the ready loop and a hand-back. A hand-back to a driver that lost a capability cancels the attempt and gives the step up the way a takeover does, so another driver can take it.teamai env execcannot hide a value from the agent that runs it (an agent can print or transform the variable). A driver whose executor is the agent leavesenvout ofcan, andrequires.envsteps wait for one that keeps values from the agent (the local driver, CI). No key-list flag was added toenv exec.env/secrets.yamlis secret on every machine, and an undeclared key keeps the existing name heuristic. Masking every required value instead would rewrite engine fields that happen to hold one (a checkout path in a lease's driver identity) and makestartrefuse common values such asproduction, so a team masks a value likeDATABASE_URLby declaring it. A personal environment value and a different team value under the same name are both masked, and JSON keys are masked as well as values. Team secrets shorter than the existing minimum length are not masked (unchanged); the run's pinned definitions are kept as pulled, sostartrefuses a secret value in them.timeoutis a stopped state. A timed-out step is a failure with codetimeout, shown as outcometimeout. Expiry is applied byreconcileRun(run, now)whenever an operation runs; the engine has no timers. Deadlines inherit through ancestry, fan-out items included, and an expired lease held by another driver no longer blocks the run.limitsFrom, set when the run starts and whenever a retry reopens it; a workflow step's count from when its current pass starts. So a run stopped by its timeout or its budget can be retried.for_eachstep's budget covers all its items together; each item keeps only the budget of the workflow file it runs. A headless budgeted prompt needscan.tokens; a success without tokens from such a lease failsbudget_unmeasured. An interactive prompt's tokens are best effort: no driver can meter a session a person drives. Scoring never writes a run, so scorers stay outside the budget and keep their own lease timeout. The run record's token fields still describe passes only; the run file and its snapshot carry the full sum the budget uses, so the record schema is unchanged.setupcarries it and needscan.setup; the driver runs it when it creates the worktree or finds its setup marker unfinished, with setup's own keys only, bounded by the step's deadline for every caller. So a retry after a staging or setup failure runs setup again. Setup runs in its own process group, so the local driver handles every stop signal until the step's command or tool starts: Ctrl-C during setup stops it, even where the attached command would otherwise get the terminal's signal itself. Setup and staging run in the driver's processes, soallow.networkfilters only declared step requests (see the guide). It failssetup_failed,setup_dirty(a change the step would commit) orsetup_secret(a setup key's value in a file setup created or changed, ignored files included), and the workspace is removed.max_turns. The lease carriesmaxTurnsonly to a driver withcan.max_turns. The local driver passes--max-turnsto headless Claude and reportsmax_turnsas not enforced for other tools and interactive runs.SIGKILLafter the grace period) instead of sleeping for the full grace, so cancelling setup returns promptly on Linux.workflows.allow { network, env }is a ceiling for every driver on the machine;workflows.sandboxlets the local driver declaresandboxisolation;workflows.max_parallel, when set, caps open leases per run for any driver.supportshook (external drivers cannot pass a callback),can.networkas a list,requires.human/requires.resume,setup.timeout, and rejecting inputs by env key name (inputs are not env).Test Plan
npx tsc --noEmitpassesnpm run lintpassesnpx vitest runpasses: 541 files, 15568 passed, 21 skippednpx vitest run commands-reference -u:commands.mdregenerated (thenext --jsonhelp now lists everydriver.cankey)teamai workflow next/report/respond --jsononly) under three driver profiles: local, session, and external withouttokensorenv.npm run build:Not run here: the GitLab and GitHub providers, and the Codex, CodeBuddy and OpenCode agents (left to CI).
Related Issues
Closes #1082
Notes for Reviewers
caninstead ofhitl/workspaceModes, the secrets argument,stageLease(config),now); both pass their own tests and E2E in a local integration checkout. A session driver that follows feat(workflow): add session discovery and managed public leases #1304's skill (noenv, nosetup) waits onrequires.envsteps and on workspace steps of a workflow withsetup; a local driver or CI picks those up.