feat(agent-core-v2): add the experimental flow feature - #3038
Conversation
…ding agent turnId is an Agent-scope counter starting at 0 per agent, but cancelPendingForTurn matched pending interactions by turnId alone, so any subagent ending its own turn N cancelled approvals/questions parked by other agents on the same turn number. Match on (turnId, agentId) instead, treating an origin without agentId as main.
Multi-stage flow runs behind the flow experimental flag (default off): flow definitions under .kimi-code/flows/ declare stages (objective, completion, gate); the main agent supervises the run via the /flow builtin skill, dispatching workers per stage and passing gates on per-criterion evidence. FlowStart/FlowAdvance/FlowAbort drive the run; human gates reuse the plan_review approval round-trip; a flow-reviewer profile provides independent read-only gate review. Wire vocabulary (flow_run.* events, flow/flow.gates replayable keys) and the owner service stay on the static channels so sessions recorded with the flag on keep replaying after it is turned off.
🦋 Changeset detectedLatest commit: 862268e The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
…first, and review loop criteria
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a575533bcc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Reject a human-gated pass whose review was skipped because the run state changed between preparing and executing the call (batched tool calls prepare every call before executing any, so a same-batch FlowStart/FlowAdvance sequence could slip past the user gate). - Reject a pass verdict that carries unmet criteria. - Make the flow definition frontmatter strict so a misspelled gate key fails validation instead of silently downgrading a human checkpoint. - Resolve the definition path through the bound runtime's workspace view and require the fs capability on FlowStart. - Add the flow tools to the default-approve set so a human gate is not double-prompted by the fallback permission ask. - Reinject the concrete run task after compaction, confirm start took effect before reporting it, and reject whitespace-only tasks.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 16d05bf54a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… criterion text - FlowStart rejects a definition whose declared id differs from the requested flow name, so a copied or renamed file cannot start a run under a misleading identity. - Stage objectives/completions and per-criterion text/evidence are trimmed and must be nonempty, closing the remaining blank-string holes that could pass acceptance without verifiable content.
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
The flow folds now emit an AgentStatusUpdated flowRun summary (stage id,
index, total, gate; null when the run ends) so clients get a live badge
signal over the existing agent.status.updated WS event, and a new
GET /sessions/{id}/flow route serves the full run snapshot and gate
audit records for the flow panel. REST is the authority, the WS summary
is the refresh signal.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 257ee6f778
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The flow run state survives replay by design (static-channel wire vocabulary), so a session that ran a flow while the experimental flag was on would still expose its leftover active run over GET /flow after the flag was turned off — and the web client's badge refill would then show a zombie flow panel with no tools left to advance or abort it. Gate the route's response on the flag: the outward claim now follows the runtime capability, while the engine state stays replayable.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5426be0c4a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…sh command A new workspace skill source projects every definition under .kimi-code/flows/ into a user-activatable skill of type 'flow' (empty while the flow flag is off; fs-watched; definitions whose id does not match their file name, or that fail to parse, are skipped with a reason). Its activation prompt binds the shared supervisor contract — now extracted to contract.md and reused by the generic /flow entry — to that one flow, so the model no longer parses the flow id out of free text. The TUI renders flow-typed skills as flow:<name> instead of skill:<name>, giving flows their own namespace beside skills.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f71ac4fd47
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ills route listWorkspaceSkillsForRoot rebuilds the catalog by hand instead of going through the workspace handler's catalog service, so the flows source never fed it and the web slash menu (which lists skills per workspace, not per session) missed every /flow:<id> entry. Extract the flow discovery into a shared discoverFlowSkills function and add it as a sixth arm of that rebuild, gated on the flow flag like the source.
…n itself Dogfooding showed the supervisor skipping FlowStart on its first real run — it read the definition, mirrored the stages into TodoList, and dispatched workers with no engine run, no gates, and no panel. The user's intent is already fully determined by the slash command, so starting the run is now a mechanism, not a model choice: the flow service subscribes to the observable SkillActivated event (which now carries skillType) and starts the run from the activation's skill path and args before the prompt lands. The per-flow skill text carries the statically rendered blueprint, forbids re-reading the definition and TodoList mirroring, and keeps FlowStart only as the recovery path; a TodoList veto enforces the no-mirroring rule while a run is active (tower precedent).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 152cdeb4bf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The agent profile's static tool whitelist gates tool activation, so the FlowStart/FlowAdvance/FlowAbort contributions never reached the main agent — dogfooding showed the model reasoning 'FlowAdvance is not in my listed tools' and falling back to verbal stage transitions while the run stayed at stage one. Add the three names to AGENT_TOOLS (TowerInit precedent); while the flow flag is off the contributions do not exist, so the whitelist entries are inert. Token-count assertions and tool snapshots shift with the longer Agent tool description.
… assertions The whitelist change itself plus the fullCompaction/loop test updates that belong with the previous commit (an earlier git add swallowed a bad path and silently staged only the tool snapshot).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f532824c87
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… the flow run The flow-run start signal rode a volatile agent.status.updated frame (never journaled, never replayed), and a run auto-started within the first second of a fresh session predictably beat the client's WS subscription; the REST refill fires on session select (too early) and on main turn end (too late — a human gate holds the first turn open through the whole stage), so the flow UI first rendered only at stage two. Make the status a level signal instead of an edge: readLegacyStatus now carries flowRun (main agent only — a null from a worker frame would wrongly clear the session badge downstream; null while the flow flag is off, matching the REST route's zombie-run gate), so every merged main status frame refreshes the badge, and subscribe() sends one synthesized volatile snapshot frame to the just-subscribed target so the current stage renders within the subscribe round-trip.
…play The human gate borrowed the plan_review display, so every gate approval rendered as 'Ready to build with this plan?' with plan-mode buttons — wrong semantics at the flow's core UX moment, and the per-criterion verdicts (already structured in FlowAdvance args) were flattened into a markdown blob. Add a flow_gate_review variant to the display vocabulary (protocol, agent-core-v2, kap-server wire schema) carrying the stage position, gate, objective/completion, per-criterion verdicts, and the next stage. FlowAdvance now attaches the structured payload (the markdown renderer is gone), and the gate hook matches the new kind. The TUI approval panel renders it natively: a 'Pass this stage gate?' header, a flow/stage/progress context line, a ✓/✗ criteria checklist with dimmed evidence, the supervisor note, and gate-specific choices (pass / reject with feedback / reject). The vscode adapter's exhaustive display switches learn the new kind; acp-adapter degrades to its default approval options by design. Old web bundles fall back to the generic approval card and can still decide the gate.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0fd28dc43f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…ce, protect feature profiles Review round 39: - A queued flow prompt that is aborted (or fails) before reaching a step head now releases its reservation: the prompt handle's completion discards the pending activation and its stored definition, so an orphaned entry can no longer block every later flow activation. The single-activation busy path does the same. - The active/pending admission check moved after the awaited metadata update, directly adjacent to the synchronous SkillActivated dispatch, so two concurrent promptWithSkills requests can no longer both pass the check before either publishes its activation. - Bundle preparation rolls back stored activation data when a later skill in the bundle fails to prepare. - The profile catalog's builtin protection now covers every builtin-priority contribution (features included), so a workspace file profile named flow-reviewer cannot silently replace the flow reviewer without override: true.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a35b1b7ce7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (flowActivations.length > 0) { | ||
| const flowIds = flowActivations.map((activation) => activation.origin.activationId); | ||
| void handle.completion.then((completion) => { | ||
| if (completion.state !== 'cancelled' && completion.state !== 'failed') return; |
There was a problem hiding this comment.
When an onBeforeSubmitPrompt hook blocks a bundled flow prompt, the handle settles as blocked without ever reaching the step-head injector that consumes its pending activation. Fresh evidence beyond the cancellation/failure cleanup is the explicit blocked terminal state omitted here: the reservation remains in pendingActivations, so every later flow activation is rejected as already queued even though no run can be aborted; clean up blocked completions as well.
AGENTS.md reference: packages/agent-core-v2/AGENTS.md:L74-L76
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed — the completion cleanup now also treats blocked as a terminal release: a hook-blocked prompt never reaches a step head, so its reservation and stored definition are discarded (a later retry is covered by the blueprint's FlowStart recovery path).
| flagWas = flagNow; | ||
| if (this.scopeContext.agentId !== 'main') return; | ||
| void this.dispatcher.dispatch( | ||
| new AgentStatusUpdated({ flowRun: flagNow ? this.summary() : null }), | ||
| ); |
There was a problem hiding this comment.
If the flow flag is turned off after a flow prompt is queued but before it reaches a step head, retracting FlowInjection prevents reconciliation while this listener only clears the published status. The prompt can then finish with its activation still pending; after re-enabling the flag, hasPendingActivation() rejects every new flow and an ordinary prompt cannot consume the orphan because its activation ID is no longer the latest user origin. Cancel or discard pending flow activations when the effective flag turns off.
AGENTS.md reference: AGENTS.md:L66-L68
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed — disabling the flag clears queued reservations (the retracted injector can no longer consume them), so re-enabling starts clean instead of inheriting a dead pending activation that rejects every new flow.
Review round 40: - The completion cleanup also treats the blocked terminal state as a release: a hook-blocked bundled flow prompt never reaches a step head, so its reservation must not outlive it (the blueprint's FlowStart recovery covers a later retry). - Turning the flow flag off clears queued reservations, since the retracted step-head injector can no longer consume them; re-enabling no longer inherits a dead pending activation that rejects every new flow.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 83a9d00912
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const flagNow = this.flags.enabled(FLOW_FLAG_ID); | ||
| if (flagNow === flagWas) return; | ||
| flagWas = flagNow; | ||
| if (!flagNow) this.pendingActivations.clear(); |
There was a problem hiding this comment.
When a bundled flow prompt is pending behind an active turn, clearing this map only releases its activation reservation; the PromptHandle and its message remain queued in AgentPromptService, so the supervisor blueprint still launches later. With the flag off, FlowFeature has retracted the injector and all three flow tools, leaving the accepted prompt unable to start, advance, or abort its requested flow. Fresh evidence after the earlier reservation fix is that this configuration handler never cancels the handle created by promptWithSkills; cancel queued flow prompts or otherwise prevent their blueprints from being delivered after disable.
AGENTS.md reference: AGENTS.md:L64-L68
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed — the skill service now tracks queued flow prompts (bundled and single-activation) and aborts them when the flag turns off, so an accepted blueprint no longer launches into a session whose flow tools and injector were retracted; the abort flows through the existing completion cleanup, releasing the reservation and stored definition.
|
|
||
| private rejectWhileFlowRunActive(skillType: string | undefined): void { | ||
| if (skillType !== 'flow') return; | ||
| if (!this.flags.enabled(FLOW_FLAG_ID)) return; |
There was a problem hiding this comment.
When the flag is turned off while promptWithSkills is awaiting its metadata update, the request may already have prepared a flow from the old catalog; this early return then treats flag-off as permission to continue, publishes SkillActivated, and submits the flow blueprint after the feature's tools and injector have been retracted. Reject flow-typed activation when the flag is disabled rather than skipping the admission check, so an in-flight request cannot bypass the experimental gate.
AGENTS.md reference: AGENTS.md:L64-L68
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed — the activation guard now rejects flow-typed activations outright while the flag is off (REQUEST_INVALID), instead of treating flag-off as permission to continue, so a request that prepared from the old catalog during the awaited metadata update can no longer publish after the feature was retracted.
The supervisor can now move an active run to another stage: backward
to redo work whose conclusion was invalidated, forward to skip stages
that do not apply to the task. The definition's frontmatter governs
it via jumps: approval (default — every jump goes through a user
review), free (unsupervised), or disabled (strictly linear, the old
behavior).
- New FlowJump tool (to + reason, both required): validated like the
other flow tools, solo-response only, supervisor-only, flag-gated.
- A durable FlowJumped event moves the undoable stage pointer and
appends a kind: 'jump' record (from/to/reason/decidedBy) to the
retained audit trail; the run epoch bumps on every jump so verdicts
and reminders prepared before it are void, and a revisited stage
re-discloses its reminder.
- The approval flow reuses the gate-review machinery: a new
flow_jump_review display (protocol + kap-server + engine schemas),
epoch-bound reviews, observed-rejection provenance, and no
session-wide approval anywhere (TUI choices, ACP options, VS Code
downgrade all treat jumps like gates).
- GET /sessions/{id}/flow exposes run.jump_policy and the audit
records as a verdict/jump union.
- The supervisor contract's Order rule now teaches FlowJump instead
of mandating abort-and-restart.
|
@codex review |
Review round 41: - The activation guard now treats a disabled flow flag as a rejection (the tools and injector are gone), instead of as permission to continue — closing the window where a request prepared from the old catalog published its activation after the feature was retracted. - The skill service tracks queued flow prompts (bundled and single) and aborts them when the flag turns off, so an accepted supervisor blueprint no longer launches into a session whose flow machinery has been withdrawn; the abort flows through the existing completion cleanup, releasing the reservation and stored definition.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f785e127c7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| throw new Error2( | ||
| ErrorCodes.REQUEST_INVALID, | ||
| 'A flow run is already active or queued in this session. Finish or abort it (FlowAbort) before starting another flow.', |
There was a problem hiding this comment.
When the REST skill-activation endpoint invokes a flow while another run is active or queued, this new branch throws REQUEST_INVALID, but packages/kap-server/src/routes/skills.ts's sendMappedError() does not handle that code and rethrows it to the global handler as 50001 INTERNAL_ERROR. This is a normal client conflict rather than a server failure, so map it to VALIDATION_FAILED and declare that response on the route.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed — sendMappedError now maps REQUEST_INVALID to the VALIDATION_FAILED envelope, so activation conflicts (active/queued run, disabled feature, non-main agent) surface as client errors instead of a 500.
| if (this.loop.status().state === 'running') { | ||
| return this.prompt.inject(message); | ||
| } |
There was a problem hiding this comment.
When a single flow activation arrives during an active turn, this path uses prompt.inject() and therefore creates no PromptHandle and never enters queuedFlowPrompts; if the flag is disabled before that queued step materializes, the disable listener cannot abort it, so its supervisor blueprint is still appended after the flow injector and tools have been retracted. Fresh evidence beyond the prior queued-prompt fix is that only handle-backed prompts are tracked, while this assigned SteerStepRequest remains uncancellable by the skill service; retain its abort receipt or route flow activations through a tracked handle.
AGENTS.md reference: AGENTS.md:L64-L68
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not fixing in this PR (framework follow-up): the inject path enqueues a steer step on the loop with no cancellable handle — cancelling it needs a loop-level API that nothing else has today. The remaining window is narrow (guard-checked immediately before inject) and its damage is inert: the pending reservation and stored definition are already cleared by the disable path, the injector and all flow tools are retracted, so the stray blueprint text can only lead the model to a FlowStart call that fails with a clear unknown-tool error. A steer-cancellation API is the right fix and belongs to a loop-layer change, not this feature.
| export * from '#/features/flow/tools/start/start'; | ||
| export * from '#/features/flow/tools/advance/advance'; | ||
| export * from '#/features/flow/tools/abort/abort'; | ||
| export * from '#/features/flow/skill/skill'; |
There was a problem hiding this comment.
Consumers importing the new flow contracts from @moonshot-ai/agent-core-v2 can access FlowStartInputSchema, FlowAdvanceInputSchema, and FlowAbortInputSchema, but the adjacent FlowJumpInputSchema and IFlowJumpTool module is not re-exported here. The runtime advertises and executes FlowJump, yet root-entry consumers cannot use its typed contract without a package-internal deep import; export #/features/flow/tools/jump/jump alongside the other three flow tools.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed — the package entry point now re-exports the FlowJump module (input schema and tool contract) alongside the other flow tools.
…FlowJump contract Review round 42: - The skills route maps REQUEST_INVALID activation failures (active or queued run, disabled feature, non-main agent) to the VALIDATION_FAILED envelope instead of rethrowing them as a 500 — these are client conflicts, not server failures. - The package entry point re-exports the FlowJump input schema and tool contract alongside the other flow tools.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9bafcf8b53
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| skillArgs: input.args, | ||
| }; | ||
| this.activationData.put(origin.activationId, skill.data); | ||
| const turn = await this.recordActivation(origin, content); |
There was a problem hiding this comment.
When a /flow:* activation is injected into a running turn and prompt.inject() rejects (for example, if loop admission fails), recordActivation() has already published SkillActivated, so the flow service has inserted a pending activation. This await propagates the error without reaching the turn === undefined cleanup below, leaving hasPendingActivation() permanently true and causing every later flow activation in the session to be rejected. Clean up the activation data and pending reservation when recordActivation() throws as well as when it returns undefined.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed — a rejected prompt delivery (inject or enqueue) during a flow activation now releases the pending reservation and the stored definition on the error path before rethrowing.
| 'FlowStart', | ||
| 'FlowAdvance', | ||
| 'FlowAbort', | ||
| 'FlowJump', |
There was a problem hiding this comment.
When the flow flag is off, or when a later user/plugin registration shadows one of these tools, the name-only tool registry can resolve an unrelated executable as FlowStart, FlowAdvance, FlowAbort, or FlowJump. This policy checks only toolCall.name, so that unrelated tool now bypasses the fallback approval prompt even though the experimental flow contribution is absent. Scope this approval to the builtin flow contribution/source, or prevent these names from being shadowed, so disabling the feature cannot leave its permission effects enabled.
AGENTS.md reference: AGENTS.md:L66-L68
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed — the policy now resolves the flow tools' source from the tool registry and auto-approves them only when they are the builtin registrations, so a shadowing user/MCP tool (possible while the flag is off) no longer inherits the bypass. Test covers builtin, shadowed, and absent registrations.
| readonly permission?: PermissionMode; | ||
| readonly usage?: UsageStatus; | ||
| readonly phase?: AgentPhase; | ||
| readonly flowRun?: AgentFlowRunStatus | null; |
There was a problem hiding this comment.
The server and SDK now expose flowRun, but no shipped client consumes it: the TUI's handleStatusUpdate() ignores the field, and a repo-wide search finds no flowRun token in the committed apps/kimi-code/dist-web bundle. Consequently the supervisor prompt's claim that the UI shows flow progress is false—outside approval gates, users receive no persistent indication of the active stage. Add the TUI handling and/or sync the web implementation and committed bundle with this protocol change.
AGENTS.md reference: AGENTS.md:L18-L18
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Partly fixed, remainder is settled scope: the projected blueprint wording no longer claims every UI shows flow progress (UIs with flow support do). The web/desktop FlowPane consuming flowRun ships in the companion PR and enters dist-web via the established post-merge sync:web step; the TUI segment was explicitly deferred earlier in this cycle. The protocol field and subscription snapshot exist precisely for those consumers.
| }; | ||
| } | ||
| const args = parsed.data; | ||
| if (this.flow.run().active) { |
There was a problem hiding this comment.
When a flow activation is queued behind an active turn, it reserves capacity in pendingActivations, but an explicit FlowStart issued by the current model checks only run().active and can start another run before that queued prompt reaches a step head. The accepted activation prompt is then still delivered with its supervisor blueprint, while reconciliation discards its reservation because the explicit run is active; its recovery FlowStart consequently fails. Fresh evidence beyond the pending-admission fix is that this explicit start path never checks hasPendingActivation(); reject it while a queued activation owns the reservation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed — FlowStart now reports a queued activation with a dedicated wait-for-it message instead of racing it, so the reserved prompt keeps its automatic start.
… inject failure cleanup Review round 43: - FlowStart reports a queued activation reservation with a dedicated message instead of racing it: the queued prompt keeps its start. - The default-approve policy resolves the flow tools' registry source and auto-approves them only when they are the builtin registrations, so a user/MCP tool shadowing a flow tool name (possible while the flag is off) no longer inherits the approval bypass. - A failed prompt delivery during a flow activation (inject/enqueue rejection) now releases the pending reservation and stored definition on the error path. - The projected blueprint no longer promises that every UI shows flow progress (UIs with flow support do).
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 016c28d1a2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| import { FLOW_REVIEWER_PROFILE } from './flow'; | ||
|
|
||
| const FLOW_REVIEWER_TOOLS = [ | ||
| 'Bash', |
There was a problem hiding this comment.
When flow-reviewer runs under auto approval or a user approves its command, this profile can execute arbitrary shell writes even though the feature promises an independent read-only review; BashTool passes the model-provided command directly to the workspace shell, so prompt text saying “change nothing” is not an enforced capability boundary and a reviewer can modify the artifacts it then approves. Fresh evidence after the profile-shadowing fix is that the protected builtin profile itself still grants Bash; remove it or provide a technically read-only verification surface.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not removing Bash (settled design), wording fixed: the reviewer running verification commands (tests, greps) is a core part of its value and an explicit early design decision of the supervisor model. The real capability boundary is the approval chain — a subagent's Bash commands go through the same per-command approval as anyone else's, and auto mode is the user explicitly waiving per-command review session-wide, which grants the reviewer nothing the main agent doesn't have. The profile description no longer claims 'read-only tools'; it now states shell is verification-only with each command subject to the approval policy. A statically-enforced read-only Bash is a general capability that belongs to its own feature, not this PR.
| ); | ||
| } | ||
| try { | ||
| if (flowActivations.length > 0) this.rejectWhileFlowRunActive('flow'); |
There was a problem hiding this comment.
When another flow is already active or queued, this guard is guaranteed to reject the bundled activation, but it runs only after applyPromptMetadataUpdate() has persisted the rejected request as lastPrompt and may have assigned it as an untitled session's title. Fresh evidence after moving the final admission check next to SkillActivated is this now-observable side effect on a request that returns REQUEST_INVALID; retain the post-await atomic check, but add a preflight conflict check before mutating metadata.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed — a deterministically-known conflict (active or queued run, disabled feature) is now rejected before the metadata update can persist the doomed request as lastPrompt or an untitled session's title; the post-metadata check remains as the race-closing final admission adjacent to the SkillActivated dispatch.
Conflicts were all generated or baseline-tracking artifacts: the wire manifest is regenerated (now including flow_run.jumped), the tool snapshot tests are re-recorded, and the compaction token baselines are re-shifted on top of main's new values (+11 for the FlowJump tool).
…l wording Review round 44: - promptWithSkills rejects a deterministically-known flow conflict (active or queued run, disabled feature) before the prompt metadata update persists the doomed request as lastPrompt or a session title; the post-metadata check stays as the race-closing final admission next to the SkillActivated dispatch. - The flow-reviewer profile description no longer claims read-only tools: shell access is verification-only by contract and every command still goes through the approval policy.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Release bundled flow reservations when submission rejects
When reservation.submit() rejects before returning a handle—for example, if durable PromptAccepted dispatch fails—the preceding recordActivation() calls have already installed the flow in pendingActivations, but the only cleanup is attached after a handle is obtained. The request therefore fails while every later flow activation is rejected as already queued. Fresh evidence beyond the earlier delivery cleanup is that the bundled submission path still has no catch that calls discardPendingActivation() for the prepared flow IDs.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| promptMetadataTextFromContentParts(input.input), | ||
| ); |
There was a problem hiding this comment.
If applyPromptMetadataUpdate() rejects after bundle preparation, every prepared skill payload has already been inserted into the session-scoped, unbounded activation-data map, but execution exits before either recordActivation() consumes it or discardPrepared() runs. A transient or persistent metadata-store failure followed by retries therefore retains complete flow definitions for the session lifetime. Fresh evidence beyond the preparation rollback is that this awaited metadata operation remains outside every cleanup block.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed — the metadata update (and the final admission check after it) is wrapped so a persistence failure discards the stored activation payloads before rethrowing; retries no longer accumulate entries in the activation-data map.
| private rejectWhileFlowRunActive(skillType: string | undefined): void { | ||
| if (skillType !== 'flow') return; | ||
| if (!this.flags.enabled(FLOW_FLAG_ID)) { |
There was a problem hiding this comment.
When an ordinary SKILL.md declares the already-supported type: flow, it remains discoverable independently of .kimi-code/flows, but this type-only check now rejects it whenever the experimental flag is off; with the flag on, prepareActivationStart() still cannot start it because its path and payload are not from the canonical flow source. Such a listed command is therefore either rejected or delivered without a run. Identify projected flow skills by their reserved source/path/data, or exclude arbitrary file skills from the flow type, rather than applying the experimental behavior to metadata alone.
AGENTS.md reference: AGENTS.md:L64-L68
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed — the admission guards now key on the projected flow namespace (a flow: catalog name plus type: flow) instead of the type alone, so an ordinary SKILL.md declaring type: flow is not engine-managed and activates like any other skill regardless of the flag; only projected definitions get the flag/active-run/main-agent gating.
The only conflict was the generated state manifest; regenerated on top of main (now also carrying the flow jump policy and audit records).
Review round 45: - A metadata persistence failure between bundle preparation and the activation dispatch now discards the stored activation payloads before rethrowing, so retries cannot accumulate entries in the unbounded activation-data map. - The flow admission guards (flag, active/queued run, main-agent) apply only to projected flow skills (the flow: catalog namespace): an ordinary SKILL.md that declares type: flow is not engine-managed and activates like any other skill again.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Release bundled flow reservations when submission throws
When reservation.submit() rejects—for example because the PromptAccepted dispatch or prompt enqueue fails—the preceding recordActivation() calls have already inserted the flow into pendingActivations, but the completion cleanup is installed only after this await succeeds and the finally block merely disposes the reservation. Fresh evidence beyond the single-activation delivery cleanup is that this bundled submission path remains outside any catch, so the orphaned reservation makes every later flow activation fail as already queued; discard the prepared data and pending activation before rethrowing.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!FLOW_TOOL_NAMES.has(event.toolCall.name)) return; | ||
| const firstFlowCall = event.toolCalls.find((call) => FLOW_TOOL_NAMES.has(call.name)); |
There was a problem hiding this comment.
When a user or MCP tool shadows FlowAdvance, FlowJump, or another flow-tool name while the flag is enabled, this name-only guard treats that unrelated executable as a builtin flow operation and applies the flow batching vetoes; for example, a shadowing FlowJump issued with any sibling is rejected as a stale stage jump. Although the approval policy now verifies the registry source, these execution hooks do not, so apply the same builtin-source check before enforcing flow-specific behavior.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed — the execution hook (batching vetoes and the gate/jump approval path) now confirms the call resolves to the builtin registration in the tool registry before treating it as a flow operation, mirroring the approval-policy fix; a shadowing registration passes through untouched. Test added.
Review round 46: the batching vetoes and gate/jump approval hook resolved flow tools by name only, so a user/MCP tool shadowing a flow-tool name inherited flow semantics (solo-response vetoes, gate review). The hook now confirms the builtin registration in the tool registry, mirroring the approval-policy fix.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 862268e5aa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| return (await this.prompt.enqueue({ message })).launched; | ||
| const handle = await this.prompt.enqueue({ message }); | ||
| if (origin.skillType === 'flow') this.trackQueuedFlowPrompt(handle); |
There was a problem hiding this comment.
When an ordinary SKILL.md uses the already-supported type: flow but does not belong to the reserved flow: namespace, admission correctly treats it as a normal skill, yet this line still tracks its handle as an experimental flow prompt based solely on its type. If the flow flag is subsequently disabled before that turn completes, the configuration listener calls prompt.abort() and cancels this unrelated skill invocation; use the same projected-flow predicate used during admission.
AGENTS.md reference: AGENTS.md:L64-L68
Useful? React with 👍 / 👎.
| const commands = sortedSkills.filter(isUserActivatableSkill).map((skill) => { | ||
| const commandName = | ||
| skill.source === 'builtin' || skill.isSubSkill === true | ||
| skill.type === 'flow' || skill.source === 'builtin' || skill.isSubSkill === true |
There was a problem hiding this comment.
For a nonprojected file skill that declares type: flow, this changes its command from /skill:<name> to the bare /<name> even while the experimental feature is off. Because resolveSlashCommandInput() resolves built-ins first, a skill named compact, review, or another built-in becomes unreachable; the same type-only condition in packages/acp-server/src/slash.ts drops reserved-name collisions entirely. Apply the bare-name treatment only to projected skills whose names use the reserved flow: prefix.
AGENTS.md reference: AGENTS.md:L64-L68
Useful? React with 👍 / 👎.
| if (origin?.kind === 'user') { | ||
| return ( | ||
| count + | ||
| (origin.skillActivations ?? []).filter((entry) => entry.skillType === 'flow').length |
There was a problem hiding this comment.
When one queued promptWithSkills request contains two ordinary file skills whose metadata uses type: flow, the skill service accepts the bundle because neither name is in the projected flow: namespace, but this type-only count reaches two and steer() rejects even selecting that single prompt. This prevents Ctrl-S steering for otherwise normal skills and applies even with the experimental flag off; count only projected flow: activations here as admission does.
AGENTS.md reference: AGENTS.md:L64-L68
Useful? React with 👍 / 👎.
1 participant
Problem
Long multi-stage tasks today rely on a single agent both doing the work and deciding the work is done — nothing separates execution from acceptance, so runs drift, self-approve, and leave no auditable record of why a stage was allowed to proceed. There is also no way for a user to declare a repeatable staged workflow with explicit human checkpoints and have the engine drive it.
What changed
Adds an experimental
flowfeature to agent-core-v2 (flagflow, envKIMI_CODE_EXPERIMENTAL_FLOW, default off)..kimi-code/flows/<id>.md: frontmatter declares stages as objective / completion / gate (ai|human|ai-then-human); optional prose sections carry per-stage boundaries. Intent only — no step lists, no wiring./flowbuiltin skill: it dispatches each stage to worker subagents, verifies every completion criterion against objective evidence, and submits verdicts throughFlowAdvance, whose schema requires per-criterion evidence.FlowStartparses and snapshots the definition;FlowAbortends a run. Stages advance strictly in order — the service rejects out-of-order verdicts.plan_reviewapproval round-trip (zero UI changes): approve passes the gate, reject-with-feedback flows back as the next instruction, plain reject stops the turn. Inautopermission mode gates follow the same skip convention as plan/goal.flow-reviewerprofile provides independent read-only gate review (no write tools, no AskUserQuestion) for high-stakes gates.flow, undoable — follows conversation undo like plan mode) and the gate audit trail (flow.gates, world-time, kept across undo). Wire vocabulary and the owner service stay on the static channels, so sessions recorded with the flag on keep replaying after it is turned off, and the feature contributes nothing while the flag is off (tower pattern).Also fixes a pre-existing interaction bug this feature would have tripped on:
cancelPendingForTurnmatched pending approvals byturnIdalone, so any subagent ending its own turn N cancelled approvals parked by other agents on the same turn number; it now matches on(turnId, agentId).Covered by feature-assembly, service, ops/undo, and definition-parsing tests; manifests regenerated.