Repository navigation
Conversation
PR #1203: Stage workflow reporting and bound same-session completion53 files, +976 / -99 Scope🔴 PR has 1075 lines changed. Split into focused PRs. 🟡 1075 lines changed. PRs under 400 receive more thorough review. 🟡 53 files changed. Are all changes related? StructuralOxlint structural signals:
Slop
Static AnalysisOxlint: 55 diagnostics across 2 files (9 rules) consistent-function-scoping (20): packages/cli/tests/support/fake-acp.ts, packages/acp/src/provider.ts CorrectnessFILE: packages/cli/tests/support/fake-acp.ts, PATTERN: signal cluster, CONCERN: Multiple Oxlint violations in same file, QUESTION: Are these generated or unreviewed code fragments? |
606845d to
2cc94e2
Compare
taras
changed the title
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| const parsedSessionOptions = {}; | ||
| if (typeof sessionOptions.model === "string") parsedSessionOptions.model = sessionOptions.model; | ||
| if (isStringArray(sessionOptions.allowed_tools)) parsedSessionOptions.allowed_tools = [...sessionOptions.allowed_tools]; | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| const sessionOptions = {}; | ||
| if (!assignSessionModel(sessionOptions, record.model)) return null; | ||
| if (!assignSessionAllowedTools(sessionOptions, record.allowedTools)) return null; | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| if (!assignSessionAllowedTools(sessionOptions, record.allowedTools)) return null; | ||
| // XMD workflow role controls: preserve the explicit tool deny list. | ||
| if (record.disallowedTools != null) { | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| if (record.disallowedTools != null) { | ||
| // XMD workflow role controls: preserve the explicit tool deny list. | ||
| if (!isStringArray(record.disallowedTools)) return null; | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| if (typeof options.model === "string" && options.model.trim().length > 0) target.model = options.model; | ||
| if (Array.isArray(options.allowedTools)) target.allowedTools = [...options.allowedTools]; | ||
| // XMD role tool denials reach the supported Claude SDK option. | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| const sessionOptions = {}; | ||
| assignDefinedOption(sessionOptions, "model", nonEmptyString(stored.model)); | ||
| assignDefinedOption(sessionOptions, "allowedTools", storedAllowedTools(stored.allowed_tools)); | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| const next = { | ||
| model: nonEmptyString(options.model), | ||
| allowed_tools: Array.isArray(options.allowedTools) ? [...options.allowedTools] : void 0, | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| sessionOptions?: { | ||
| model?: string; | ||
| allowedTools?: string[]; | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| session_options?: { | ||
| model?: string; | ||
| allowed_tools?: string[]; | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| type SessionAgentOptions = { | ||
| model?: string; | ||
| allowedTools?: string[]; | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| const parsedSessionOptions = {}; | ||
| if (typeof sessionOptions.model === "string") parsedSessionOptions.model = sessionOptions.model; | ||
| if (isStringArray(sessionOptions.allowed_tools)) parsedSessionOptions.allowed_tools = [...sessionOptions.allowed_tools]; | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| const sessionOptions = {}; | ||
| if (!assignSessionModel(sessionOptions, record.model)) return null; | ||
| if (!assignSessionAllowedTools(sessionOptions, record.allowedTools)) return null; | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| if (!assignSessionAllowedTools(sessionOptions, record.allowedTools)) return null; | ||
| // XMD workflow role controls: preserve the explicit tool deny list. | ||
| if (record.disallowedTools != null) { | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| if (record.disallowedTools != null) { | ||
| // XMD workflow role controls: preserve the explicit tool deny list. | ||
| if (!isStringArray(record.disallowedTools)) return null; | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| if (typeof options.model === "string" && options.model.trim().length > 0) target.model = options.model; | ||
| if (Array.isArray(options.allowedTools)) target.allowedTools = [...options.allowedTools]; | ||
| // XMD role tool denials reach the supported Claude SDK option. | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| const sessionOptions = {}; | ||
| assignDefinedOption(sessionOptions, "model", nonEmptyString(stored.model)); | ||
| assignDefinedOption(sessionOptions, "allowedTools", storedAllowedTools(stored.allowed_tools)); | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| const next = { | ||
| model: nonEmptyString(options.model), | ||
| allowed_tools: Array.isArray(options.allowedTools) ? [...options.allowedTools] : void 0, | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| sessionOptions?: { | ||
| model?: string; | ||
| allowedTools?: string[]; | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| session_options?: { | ||
| model?: string; | ||
| allowed_tools?: string[]; | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| type SessionAgentOptions = { | ||
| model?: string; | ||
| allowedTools?: string[]; | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| const parsedSessionOptions = {}; | ||
| if (typeof sessionOptions.model === "string") parsedSessionOptions.model = sessionOptions.model; | ||
| if (isStringArray(sessionOptions.allowed_tools)) parsedSessionOptions.allowed_tools = [...sessionOptions.allowed_tools]; | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| const sessionOptions = {}; | ||
| if (!assignSessionModel(sessionOptions, record.model)) return null; | ||
| if (!assignSessionAllowedTools(sessionOptions, record.allowedTools)) return null; | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| if (!assignSessionAllowedTools(sessionOptions, record.allowedTools)) return null; | ||
| // XMD workflow role controls: preserve the explicit tool deny list. | ||
| if (record.disallowedTools != null) { | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| if (record.disallowedTools != null) { | ||
| // XMD workflow role controls: preserve the explicit tool deny list. | ||
| if (!isStringArray(record.disallowedTools)) return null; | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| if (typeof options.model === "string" && options.model.trim().length > 0) target.model = options.model; | ||
| if (Array.isArray(options.allowedTools)) target.allowedTools = [...options.allowedTools]; | ||
| // XMD role tool denials reach the supported Claude SDK option. | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| const sessionOptions = {}; | ||
| assignDefinedOption(sessionOptions, "model", nonEmptyString(stored.model)); | ||
| assignDefinedOption(sessionOptions, "allowedTools", storedAllowedTools(stored.allowed_tools)); | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| const next = { | ||
| model: nonEmptyString(options.model), | ||
| allowed_tools: Array.isArray(options.allowedTools) ? [...options.allowedTools] : void 0, | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| sessionOptions?: { | ||
| model?: string; | ||
| allowedTools?: string[]; | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| session_options?: { | ||
| model?: string; | ||
| allowed_tools?: string[]; | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| type SessionAgentOptions = { | ||
| model?: string; | ||
| allowedTools?: string[]; | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| const parsedSessionOptions = {}; | ||
| if (typeof sessionOptions.model === "string") parsedSessionOptions.model = sessionOptions.model; | ||
| if (isStringArray(sessionOptions.allowed_tools)) parsedSessionOptions.allowed_tools = [...sessionOptions.allowed_tools]; | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| const sessionOptions = {}; | ||
| if (!assignSessionModel(sessionOptions, record.model)) return null; | ||
| if (!assignSessionAllowedTools(sessionOptions, record.allowedTools)) return null; | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| if (!assignSessionAllowedTools(sessionOptions, record.allowedTools)) return null; | ||
| // XMD workflow role controls: preserve the explicit tool deny list. | ||
| if (record.disallowedTools != null) { | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| if (record.disallowedTools != null) { | ||
| // XMD workflow role controls: preserve the explicit tool deny list. | ||
| if (!isStringArray(record.disallowedTools)) return null; | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| if (typeof options.model === "string" && options.model.trim().length > 0) target.model = options.model; | ||
| if (Array.isArray(options.allowedTools)) target.allowedTools = [...options.allowedTools]; | ||
| // XMD role tool denials reach the supported Claude SDK option. | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| const next = { | ||
| model: nonEmptyString(options.model), | ||
| allowed_tools: Array.isArray(options.allowedTools) ? [...options.allowedTools] : void 0, | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| } | ||
| function hasPersistedSessionOptions(options) { | ||
| return options.model !== void 0 || options.allowed_tools !== void 0 || options.max_turns !== void 0 || options.system_prompt !== void 0 || options.env !== void 0; | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| sessionOptions?: { | ||
| model?: string; | ||
| allowedTools?: string[]; | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| session_options?: { | ||
| model?: string; | ||
| allowed_tools?: string[]; | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
| type SessionAgentOptions = { | ||
| model?: string; | ||
| allowedTools?: string[]; | ||
| // XMD workflow role controls: preserve the explicit tool deny list. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // XMD workflow role controls: preserve the explicit tool deny list. |
Continue a progress-only independent review once
Keep the workflow role specification aligned with the deny list forwarded by ACPX.
Preserve the original pilot tree and document where the example selects the scoped provider.
Why
Workflow roles receive result-format instructions before they need them. An independent reviewer can also finish a turn before returning its verdict.
What changes
claude-workflow-roleprovider from the runtime base. The workflow tells roles to join foreground work before returning.How it works
Phase components freeze and verify the submission guide around dispatch and acceptance. Delivery reserves the completion turn and each summary correction before prompting. It retains both replies and the source checks for acceptance, hold and reconciliation.
Review guide
Start with
Phase/Instructions.mdandPhase/SubmissionGuide.ts. Follow the provider selection in the role components, then readDelivery/Finish.md,Delivery/Response.mdandDelivery/Reconcile.mdtogether. The ACP spec addition here identifies where the example selects the provider.How to verify it
The workflow suites exercise submission timing, role-guide deduplication, scoped Claude provider selection, cold reentry, progress continuation, summary repair and reconciliation. The focused run passed 9 tests and 24 steps. Session continuity passed 4 tests and 14 steps.
Fable independently passed signed workflow head
945b35d18367b6da75f57c2ae6d47d2145c956aa. It previously passed the original combined revision in a separate review.The new head preserves the original combined head as its first parent and merges the runtime branch as its second parent. The complete tree is byte-for-byte identical to the pilots' pinned revision
ce89bf1ae9b9ef03195c336ba745fdc10ca1c69con main88fa816edd01b13ee25dfc82ae836476e6fcd9ae.GitHub CI passed on the new workflow head. The run was queued against pinned main before this PR was retargeted because CI only triggers for PRs targeting main. The workflow CI merge and the final stacked merge both have the pilot tree. The first Node 17/18 shard hit the 120-second Implementation fixture timeout tracked in Implementation Markdown journey intermittently exceeds 120s in CI #1143. The retry passed on the same main-target merge commit. First run · Passing retry
Scope
This PR depends on the standalone Claude workflow-role runtime. Runtime PR #1242 It contains the example composition, submission and review behavior, and example end-to-end tests. The pilots keep their existing combined pin.
Risks and limitations
Simulated tests cover continuation and hold behavior. Live reviewer continuation timing remains unmeasured.
Scope confirmation
Refs #1199.