Repository navigation
feat(workflow): add session discovery and managed public leases - #1304
Conversation
The PR description documents sufficient testing, including a representative build-first real-CLI round trip. No testing-record finding. I only read the diff; I did not run, build, or install PR code. |
|
No new findings in the specified diff.
I only read the diff and trusted base review criteria; I did not run, build, or install PR code. |
SaulMoro
left a comment
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 |
- Add invocable workflow metadata, unambiguous aliases, bounded SessionStart discovery, a CLI-served workflow skill, and input-schema inspection without creating a run. - Resolve run ids across configured local scopes independently of cwd, reject duplicate ids, and bind configuration and workspace preparation to the run's retained original checkout. - Reuse local base-overlay, worktree, commit and push helpers through next --prepare and report; persist only preparation paths/fingerprints, retain edits on handback, and reject cancelled or mismatched reports before git delivery. - Keep graph scheduling in the engine and required approvals on the existing direct-person channel; update bilingual docs and generated help, with workflow/hook/skill tests, lint/typecheck/build and a real-CLI discovery-to-push round trip.
- Retain interactive and explicit read-only flags in durable preparation data, then apply the local driver's interactive write exemption during reporting. Explicit read-only sessions continue to reject repository edits. - Add prepared-session regressions for both interactive modes and extend the real CLI session round trip to preserve edits made in the active checkout. - Update skill-serving catalog and recall-gate expectations for the new workflow skill, fixing the failed fork-safe CI shard without weakening checks. - Verify all five session-driver tests, nine build-first real CLI/session/skill serving cases, typecheck and lint.
- 将会话驱动迁移到主分支共享 DriverSchema，保留发布与恢复的运行锁、命名空间诊断码及会话续接状态。 - 本机尚无运行文件时允许 next claim 和 resume 继续进入远程认领流程，避免本地发现逻辑提前拒绝它们。 - 新 next claim 回归在修复前失败、修复后通过；较大范围 1405 项 workflow 测试和最终命令文件 17 项通过。 - 6 个真实 CLI 会话、跨机器发布与恢复用例及构建、类型检查和 lint 通过；重新生成命令参考。
1c5c63a to
73b8835
Compare
|
Addressed the review follow-up at
The PR description contains the updated verification record. Ready for another review. |
I only read the specified diff and trusted base review criteria; I did not run, build, or install PR code. |
SaulMoro
left a comment
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.
- 按工作区完成接口的失败标志处理会话报告，推送被拒绝时保留分支、提交和原始执行错误。 - 恢复跨检出目录 resume 的当前目录回退，确保缺失基础提交和合并提交仍执行远端校验。 - 新增成功与失败报告的拒绝推送回归；24 项相关单测、7 项 Git CLI E2E、构建、类型检查和 lint 通过。
|
Addressed the review follow-up at
The PR description contains the updated verification record. Ready for another review. |
SaulMoro
left a comment
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.
|
No new findings in the specified diff.
I only read the specified diff and trusted base review criteria; I did not run, build, or install PR code. |
…ce helpers The script git-mvs the local driver and tool adapter into src/workflow/drivers/local/ and setup and worktree-snapshot into src/workflow/place/, rewrites every relative specifier that names a moved file (static, type, export-from, dynamic import, vi.mock), and fails if any relative specifier then resolves to no file. A second run changes nothing.
Output of node scripts/codemods/1304-move-drivers.mjs: renames plus specifier rewrites, no code change.
A workflow now says whether an agent session may start it with `invoke: allow | ask | deny`; a file that writes none is `deny`. `bad_invoke` replaces `bad_invocable`, and an alias that is not a token, or repeats the workflow's own name, is `bad_alias`. Pull reports `alias_conflict` on each workflow whose alias another workflow also uses or takes as its name. Those workflows stay cached and start by name; only the alias starts nothing.
…eckout places The run lock moves to store.ts as withRunLock, with a wait bound and a RunLock value that proves a caller holds it. The local driver takes it without waiting, as before. A step without a workspace contract is now staged by place/place.ts: preparePlace records the place and the checkout snapshot under the attempt, and finishPlace runs the read-only check from whichever process reports the attempt. The local driver goes through it for those steps. protocol.ts holds WorkflowClient, moved from the conformance interpreter, and inProcessClient, which the in-process conformance suite and plain workflow start now use. Its report finishes a prepared place first. protocolDescribe reads a workflow and the ones it runs into inputs, invoke, gates, prompt steps, steps outside the checkout and run width. StepModel is defined in step-model.ts, and the driver identity helper in driver-identity.ts, so neither flat module imports the local driver.
- `workflow start|next|report --driver skill`: describe says how to start, start prints the first turn, and `next --wait` polls until the run is done. A detached runner per attempt runs its command in the checkout, masks the output and reports it. - `pull` writes a `workflow-<name>` Claude skill for each `invoke: allow` workflow, signed with a marker so a later pull rewrites only an unedited copy and never a skill of the same name it did not write. - The teamai stub pre-approves the skill driver calls that cannot start a run. - Workflows with prompt steps or steps outside the checkout answer `needs_local_driver`; the person runs them with `--driver local`. - The transition rows are documented in docs/dev/workflow-engine.md and a test checks them against the code both ways; a boundary test keeps the skill driver off the local driver. - Both vitest configs seed an agent session id and a workflow attempt, which the setup file removes, so the scrub always has something to remove.
A driver can now declare can.env: 'commands'. negotiate grants command steps and setup their envScope and holds a prompt step that requires env with missing: ['env']. can.env true and false keep their meaning.
The skill driver declares can.env: 'commands'. Its runner builds the command's environment from the lease's envScope, so a required key is given and the run's other keys are kept from the step.
… and keep them out of git
…ne unsupported_driver message A resume without --driver skill refused --parallel, --hitl and --no-judge with one combined message from a hand-written check, while start refused each flag on its own through NonSkillStartOptionsSchema. That schema, now NonSkillRunOptionsSchema, checks both, and both commands build the unsupported_driver refusal from one helper.
An editor shows only the second of two adjacent JSDoc blocks, so the older one-line block above runSkillCall was dead text.
…rals' option words, once The skill state a start (R10) and a resume or takeover (R19) write was built by the same newSkillState call in two places, and the resume literal repeated the start literal's --parallel, HITL and --no-judge words. plannedState and runOptionWords now hold each once. The start literal keeps --confirmed between --parallel and the HITL word, so no literal changes.
…he workflow has prompt steps The guide said the first turn of a resume asks the host question when prompt steps are left. It asks whenever the run's workflow has a prompt step, settled or not, as R19 records.
…sumeRefusal A --json next whose payload claims the run makes the resume's invoke check too, and answers confirm_needed with the resume --claim --confirmed literal, since a payload cannot carry the person's yes. The dev doc's invoke rule named only resume and the takeover.
…t held it A stopped attempt held its place until a report abandoned it, but a driver that is gone never sends that report. A local driver killed mid-step whose lease then expired or was cancelled left its place busy for good, where the expiry had freed it before, and a skill session continuing the run queued behind that attempt forever. Only the stopped attempts of the preparing lease's own driver hold their place now. That driver tracks and reports them, as the skill driver does through its runner or the tick, and a --json driver can report the attempt busy names. Another driver's stopped attempt holds nothing, as before.
…tempt the run already stopped, when a session resumes
… end, and count its pull request receipts A retry that landed while the engine answered done gave the old end the retried run's version, so every later tick skipped the engine and the retried step never ran. A receipt or an unresolved create changes no revision, so a done run also stopped recovering a pull request opened before its delivery process stopped.
…o reach teamai-reports A done reply that carries notPublished is not the kept end, so the next tick's engine next publishes it again, as the guide says. R19's row now names the stopped attempts' groups a resume stops, and refuseOptions gets its JSDoc back.
…e claiming the run An unattended resume or takeover with the skill driver counted every required approval, interactive step and open choice of the definition, so a run whose approval already ran was still refused. It now counts only the steps the run has not settled; invoke: ask still counts. The refusal also came after the claim, so an unattended resume --claim of a run that needs a person or the local driver left this install owning it. Both are now read from the run as it stands, before the claim.
A dashboard answer is the person's, so it records agent: false.
Resolved: Both earlier findings remain fixed: interactive write permissions are preserved, and rejected pushes fail successful reports while retaining warnings for already-failed reports. Testing: The description documents sufficient build-first real-CLI verification. No testing-record finding. I only read the specified diff and trusted base criteria; I did not run, build, or install PR code. |
…fuse an unattended start that needs a person A sub-workflow skipped whole, or a for_each step done, settles the steps inside it, whose paths the run never lists; an unattended resume counted their approvals still. A start --driver skill in an unattended run now answers unattended_denied as describe does, before any run is created.
Earlier findings: Interactive write permissions, rejected-push handling, and unattended actual starts are fixed. Skipped/completed containers are handled; the per-item case remains above. Testing: The description includes sufficient build-first real-CLI verification. No testing-record finding; the older recorded head is not blocking. I only read the diff and trusted base criteria; I did not run, build, or install PR code. |
…ed run a claim would install An unattended next --takeover of a run a session drives skipped the check and made the caller orchestrator of a run that waits for a person. An unattended resume --claim judged this machine's older copy instead of the published run the claim installs. An approval every for_each item already answered still counted, since the run names it per item.
Resolved: The earlier concrete findings are fixed: interactive write permissions, rejected-push handling, unattended starts and existing-state preflight, published-state claim checks, skipped/completed containers, and direct per-item approval aggregation. Testing: The description documents sufficient build-first real-CLI verification. The older recorded head and provider/host coverage deferred to CI are not blocking findings. I only read the specified diff and trusted base review criteria; I did not run, build, or install PR code. |
…ume, and count a settled step a loop can rerun An unattended resume or takeover of a run this machine keeps skill state for now switches that state to unattended options (row R26), so its optional gates no longer wait for a person nobody is. A run whose definitions loop settles nothing for the unattended check, and a step inside a for_each item counts as settled when the item settled a step that holds it.
Resolved: The earlier concrete findings are fixed, including retained-state unattended options and per-item settled ancestors. Looped approvals are now guarded, but that guard introduces the first finding. Testing: The description documents sufficient build-first real-CLI verification. No testing-record finding. I only read the specified diff and trusted base criteria; I did not run, build, or install PR code. |
…p that can still run reaches A loop that ended, whose owner is final or whose workflow's step settled, no longer keeps an answered approval counting as one a person must answer. A loop that can still run, or one whose owner a loop can run again, still does.
Resolved: The overly broad loop exclusion is fixed; only loops that can reopen a step prevent it from being considered settled. Earlier interactive-permission, push-failure, and resume/takeover fixes remain intact. Testing: The description documents sufficient build-first real-CLI verification. The older recorded head and coverage deferred to CI are not blocking. I only read the specified diff and trusted base review criteria; I did not run, build, or install PR code. |
The test setup now drops CI, so the pull a new step workspace runs draws its spinner in the terminal these tests open. That terminal had no columns, and the spinner then clears lines without end, so the Ctrl-C tests timed out.
Resolved: Earlier interactive-permission, push-failure, unattended-start/takeover, published-snapshot, settled-container/fan-out, and loop-scope findings remain fixed. Testing: The description includes sufficient build-first real-CLI verification. The older recorded head and coverage deferred to CI are not blocking findings. I only read the specified diff and trusted base review criteria; I did not run, build, or install PR code. |
…the harness A session's sleep sets the fake clock from the harness's own, so a vi.setSystemTime between it and the next call could be undone: the lease then lapsed without the timeout, the step was leased again, and the runner reported twice. The harness's advance moves both.
…e that holds it A run of the user scope started in a project with its own scope lives in the user scope, as do the judge leases score --next takes for it. workflow publish now finds the run's scope as the other run commands do, and score --lease falls back to the user scope when this directory's scope does not hold the lease.
Resolved: The earlier user-scope publish and judge-follow-up findings are fixed. Earlier interactive-permission, push-failure, unattended-start/takeover, published-snapshot, settled-container/fan-out, and loop-scope fixes remain intact. Testing: The description documents sufficient build-first real-CLI verification. The older recorded head and coverage deferred to CI are not blocking findings. I only read the specified diff and trusted base review criteria; I did not run, build, or install PR code. |
|
Merged as fc8a229. Thank you, @nateEc: this is a large piece of work, and the state machine held up well under review. On top of your branch I added the unattended rules from #1086 (steps left, refusal before any claim, kept skill state), the done-run cache fixes, run scope for publish and judge leases, and two test timing fixes; each is a separate commit in the branch history if you want to look. |
Summary
A coding session runs team workflows with
--driver skill. The agent orchestrates. A file-backed state machine in the CLI drives the public lease loop and answers literal commands. The engine keeps the graph.Behaviour changes a user can see:
invocablebecomesinvoke: allow | ask | deny, defaultdeny, plusalias.invokelimits agents only:invoke--driver skillalways counts)allowask--confirmed). Unattended:denied, also with--confirmeddeny, the defaultresumeandnext --takeoverfollow the same rule, except for a run the calling session started and drives.pullgenerates oneworkflow-<name>skill perallow/askworkflow;pushrefuses generated and CLI-owned skills.workflow startfails withnested_workflowinside an attempt or a workflow worktree; both drivers export the marker to every process they start.Run modes:
TEAMAI_MODE(attended/unattended) >CI>attended. An unattended describe answers the recommended start with--auto-approve, ordenied; an unattended resume or takeover decides the same way from the steps the run has left, before it claims anything. Every process TeamAI starts for a step getsTEAMAI_MODE. An unattended local run under a terminal asks nothing.A run published elsewhere continues from a session with
teamai workflow resume <run> --driver skill [--claim].The local driver honours
--auto-approvein a terminal.respondfrom an agent session counts as the agent's answer, so required gates refuse it.score --next {run}for an unknown run answersrun_not_found.workflow reportornextgiven a judge lease answersjudge_leaseand namesteamai score.can.env: 'commands'declares scoped env for command steps and setup only.Known limits, recorded in the docs:
teamai workflow nextprefix in the generated skill also covers--takeoverand--takeover --confirmed.report --jsoninstead of its brief's command still settles the step, but the session's journal shows that step released.Type of Change
invokenow refuses a start, resume or takeover from an agent sessionTest Plan
npx tsc --noEmitpassesnpm run lintpassesnpx vitest runpassesHead
b2f115c02, built withnpm run build, macOS, providergit.npx tsc --noEmit,npm run lintnpx vitest runnpm run test:e2eon the workflow files (session-1086, skill-claim, publish-resume, nested-start, conformance, prompt, workspaces, inline, command)npm run test:e2eonworkflow-signals,env-exec-signals,score-external-driverworkflow-skill-model(property test), 10 runsReal CLI, built
dist. Members alice and bob use separate homes, sharing local bare team and business origins. Agent-session commands set onlyCLAUDE_CODE_SESSION_ID.A session claims a run another install stopped mid-step and drives it to done
invoke: deny on resume: an agent session is refused, the person's terminal claims
Unattended loop (TEAMAI_MODE=unattended): describe recommends --auto-approve, the loop reaches done without asking
Unattended resume --claim with the skill driver: refused before the claim when a person is needed, counted from the steps left (this head)
Done runs: a retry during the end's publication, an end that failed to publish, a late PR receipt (at d373ea5; the done path has not changed since)
A user-scope run started in a project that joined another team: judge it from there (this head)
workflow publishof such a run, from the same project, is covered by a unit test that publishes it to a bare origin (it answeredrun_not_foundbefore).Live agents, three workflows each (
hm-flow: parallel prompt steps, a command step with a secret, a required and an optional approval, judging;hm-ask:invoke: ask;hm-cancel: cancelled by the person mid-step):channel=tty agent=false); no secret read, no git in a place, no step work by the orchestratorteamaiin a worktree resolved to an older install, reported throughreport --jsonby hand instead of its brief's commandNot covered here: GitLab and GitHub providers, CodeBuddy and OpenCode hosts (left to CI); Windows.
Related Issues
Fixes #1086
Notes for Reviewers
Merge danger, door: two-way
invokeandaliasare new workflow keys: teamai before 0.28.0 drops a workflow that sets either, andreview --branchwarns. Nothing migrates stored state;skill.jsonis new.Blast radius: workflows
Every workflow without
invokenow refuses a start from an agent session. Local-driver runs from a terminal, CI and sub-workflows are unchanged. The shared place module now also routes the local driver's prepare, so a regression there reaches local runs.Review suggestions left as they are, on purpose:
--hitl personkeeps it: it is the caller's explicit choice, and refusing it would change the optionsstartaccepts.score --leaseresolves the scope from the directory it runs in, then the user scope. A lease names the cwd to submit from, its run's checkout, as--next's help and thelease_unknownmessage say.