docs(feature-divide): durable-turn ownership row (H23) - #929
Conversation
Extend the Workflow run/step row to make the durable-turn ownership explicit and timeless: the Workflow run owns the turn; tools are steps (one 'use step' = one model round OR one tool execution OR one persist, never a mega-step); the worker is the Blob writer (persist/checkpoint segment) when the viewport is detached. Stop/cancel (detach != cancel, cancel = G22) and the /api/agent legacy truth (post-D17) were already present; no change needed there. Docs-only; no test gate. Fixes #817 Refs #794
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
btipling
left a comment
btipling
left a comment
There was a problem hiding this comment.
Adversarial review — PR #929
Verdict: PASS WITH NOTES
Repo: btipling/invincible
Scope: main ← plan/h23-feature-divide-durable-turn · 1 file · docs-only living-doc (H23 / #817)
Lenses run: L7 L8 (skip: L1–L6 L9 — no application/UI/CI code; L2 — no secrets/workflows/auth change; L4 — no deploy paths)
AGENTS.md read: yes (plus docs/architecture.md, lib/workflows/toolExecuteStep.ts, lib/agent/turnWorkerPersist.ts, Stop/cancel + /api/agent rows on head)
Findings
| Sev | Lens | Finding | Break scenario | Refutation attempt | Confidence |
|---|---|---|---|---|---|
| Minor | L8 | Workflow row says one 'use step' = “one tool execution”. Code + docs/architecture.md define the tool boundary as toolExecuteStep: one step for one model round’s toolCalls (waves/Promise.all inside), not one step per tool call. |
An implementer following the new sentence 1:1 wraps each tool in its own 'use step', breaks in-step wave parallelism / maxRetries = 0 batch semantics, and diverges from modelGenerateStep · toolExecuteStep · persistStep. |
Defender: plan #817 DoD uses the same shorthand; “tools are steps” still bans mega-runAgentStream. Fails as living-doc accuracy: architecture already has the precise antonym table; feature-divide should not reintroduce the looser “one tool execution” reading. |
high |
Residual risk
Stop/cancel (G22 await-ack) and /api/agent legacy (post-D17) rows are already correct on this branch — no gap there. Session-list row still says the DOM host drives client→Blob upload; that coexists with worker Blob turn-segment persist (turnWorkerPersist / persistStep) — complementary scopes, not a contradiction, but easy to conflate if someone skims only one cell.
Merge guidance
- PASS WITH NOTES: safe to merge from this attack; optional wording fix below.
- No Blockers/Majors. Docs-only; no test gate expected.
What was not attacked
Live Workflows Observability; full turnLoop runtime; browser harness.
Suggested fix punch list
| File / area | Expected behavior | Acceptance criteria |
|---|---|---|
docs/feature-divide.md Workflow run/step cell |
Replace “one tool execution” with the real step unit: one 'use step' = one model round or one toolExecuteStep (one model round’s toolCalls / waves) or one persist — still never a mega-step / never wrap runAgentStream. |
Sentence matches docs/architecture.md + toolExecuteStep.ts header; no “one tool call = one step” reading; timeless (no issue ids). |
Align the Workflow run/step cell with architecture.md and toolExecuteStep: one 'use step' is one model round, one toolExecuteStep (that round's toolCalls / waves), or one persist — never one tool execution. Co-authored-by: Bjorn <btipling@users.noreply.github.com>
btipling
left a comment
btipling
left a comment
There was a problem hiding this comment.
Adversarial re-review — PR #929 @ faa4146
Verdict: PASS WITH NOTES → clean enough (prior Minor closed)
Scope: Fixer round-1 docs wording only
Claim checklist
| Claim | Result |
|---|---|
Step grain is toolExecuteStep (one round’s toolCalls / waves), not one tool call |
PASS — cell now reads one 'use step' = one model round or one toolExecuteStep (one model round's toolCalls / waves) or one persist; mega-step / runAgentStream ban retained |
| Keep Workflow-owns-turn + worker Blob writer; timeless | PASS |
No new findings. Optional residual (non-blocking): DOM-host Blob upload vs worker turn-segment Blob remains two scopes across rows — already accurate if both cells are read.
Merge guidance
Safe to merge from this attack, subject to Bjorn’s call. Not merging from Reviewer.
1 participant
What
Living-doc update to
docs/feature-divide.mdonly (plan issue #817, backend-agents H23).Extends the Workflow run/step (backend-agents) ownership row to make the durable-turn ownership explicit and timeless:
'use step'= one model round or one tool execution or one persist; never a mega-step, never wraprunAgentStreamin a single step.DoD verification
POST /api/turns/:runId/cancel)/api/agentrestated as legacy test/JSON path; production usesPOST /api/turns(D17 shipped), not a pending cutoverThe Stop/cancel and
/api/agentrows already satisfied the DoD from prior work (G22 #816 / PR #927, D17 #811), so this PR only fills the genuine gap: the Workflow-row ownership/granularity/Blob-writer language.Gates
Docs-only change (
docs/,*.md) — no test gates per the repo scope table. No typecheck/vitest/build run (nothing to compile).Links