diff --git a/.planning/README.md b/.planning/README.md index 05908fb..0dccbfc 100644 --- a/.planning/README.md +++ b/.planning/README.md @@ -130,6 +130,8 @@ The plan aligns to: 73. [Phase 79 - Prompt Memory Lifecycle Hardening And Contributor Convergence](https://github.com/mikehostetler/jido_code/blob/main/.planning/phase-79-prompt-memory-lifecycle-hardening-and-contributor-convergence.md): harden provider behavior, retention and cleanup policy, verification defaults, and contributor guidance so prompt memory remains bounded, explainable, and clearly separate from provenance and durable repository memory. 74. [Phase 80 - Source Code Graph Save-Triggered Refresh Adoption](https://github.com/mikehostetler/jido_code/blob/main/.planning/phase-80-source-code-graph-save-triggered-refresh-adoption.md): add repository-scoped source-change observation and debounced refresh scheduling so the `source_code` graph updates after code saves from either a human editor or product-managed LLM write path. 75. [Phase 81 - CodingPod Refactorer API Exposure](https://github.com/mikehostetler/jido_code/blob/main/.planning/phase-81-coding-pod-refactorer-api-exposure.md): expose the existing lazy `Refactorer` specialist through a first-class `AgentWorkspace.refactor_work/3,4` API while preserving CodingPod isolation, task-board visibility, workflow provenance, and deterministic product-owned specialist routing. +76. [Phase 82 - Conversation Runtime Supervisor Stability](https://github.com/mikehostetler/jido_code/blob/main/.planning/phase-82-conversation-runtime-supervisor-stability.md): stabilize the conversation runtime child-work supervision contract so combined conversation suites are deterministic and queued work activation does not crash when the child supervisor is unavailable. +77. [Phase 83 - Refactorer Conversation Routing Adoption](https://github.com/mikehostetler/jido_code/blob/main/.planning/phase-83-refactorer-conversation-routing-adoption.md): adopt the exposed Refactorer into deterministic conversation workflow routing so explicit behavior-preserving refactor requests reach `AgentWorkspace.refactor_work/3,4` without changing full-workflow orchestration. Chronology note: Phase 55 now owns the previously landed `55.6.*` memory ontology and governed-reference verification so the planning sequence once @@ -170,6 +172,17 @@ the `CodingPod` topology and the public workspace API by specifying a first-class `refactor_work/3,4` route to the existing lazy `Refactorer` specialist. +Conversation-runtime stability note: Phase 82 addresses the historical +combined conversation-suite failure where queued child-work activation can +encounter an unavailable `Conversations.ChildSupervisor` even though the +involved test files pass individually. + +Refactorer conversation routing note: Phase 83 is the follow-on to Phase 81. +Phase 81 exposes the workspace API; Phase 83 routes explicit +behavior-preserving refactor intent through deterministic conversation routing +while keeping generic implementation requests on execute and +`full_workflow/3,4` unchanged. + ## Shared Conventions - Numbering: - Phases: `N` diff --git a/.planning/phase-82-conversation-runtime-supervisor-stability.md b/.planning/phase-82-conversation-runtime-supervisor-stability.md new file mode 100644 index 0000000..48dd39c --- /dev/null +++ b/.planning/phase-82-conversation-runtime-supervisor-stability.md @@ -0,0 +1,108 @@ +# Phase 82 - Conversation Runtime Supervisor Stability + + + + + +Back to index: [README](https://github.com/mikehostetler/jido_code/blob/main/.planning/README.md) + +## Relevant Shared APIs / Interfaces +- `lib/jido_code/conversations/coordinator.ex` +- `lib/jido_code/conversations/child_supervisor.ex` +- `lib/jido_code/conversations/child_worker.ex` +- `lib/jido_code/conversations/driver.ex` +- `lib/jido_code/conversations/runtime.ex` +- `lib/jido_code/conversations/persistence.ex` +- `test/jido_code/conversations_coordinator_test.exs` +- `test/jido_code/conversations_driver_test.exs` +- `test/jido_code/conversations_test.exs` +- `test/jido_code/conversations_pubsub_test.exs` +- `test/jido_code/conversations/context_memory_test.exs` + +## Relevant Assumptions / Defaults +- The conversation files currently pass when run individually. +- The combined conversation-runtime batch can fail around `test/jido_code/conversations_coordinator_test.exs:448` because queued child-work activation can call `JidoCode.Conversations.ChildSupervisor` after it is stopped, missing, or otherwise unavailable. +- The fix should preserve existing coordinator semantics for stop, steer, resume, queued child work, and turn supersession. +- The conversation runtime should not depend on test file ordering, global process leakage, or implicit supervisor lifetime assumptions. + +## Implementation Notes +- Reproduced the historical batch failure with: + `mix test test/jido_code/conversations_driver_test.exs test/jido_code/conversations_coordinator_test.exs test/jido_code/conversations_test.exs test/jido_code/conversations_pubsub_test.exs test/jido_code/conversations/context_memory_test.exs --seed 871949 --max-cases 1 --max-failures 1` +- The same combined batch can pass with other seeds, and `test/jido_code/conversations_coordinator_test.exs:448` passes in isolation, so this is an order-sensitive supervisor lifecycle issue. +- The crash occurs while `Coordinator.settle_child_work/5` settles a superseded active turn and immediately activates the queued replacement turn. That path calls `ChildWorker.start/1`, which directly calls the named `JidoCode.Conversations.ChildSupervisor`. +- The failing stack shows `DynamicSupervisor.start_child/2` exiting with `:shutdown`, which means the coordinator currently lets child-supervisor unavailability escape as a coordinator crash instead of preserving the settled state and leaving queued work recoverable. +- Phase 82.2 now routes child-work startup through `ChildWorker.start/1` and `Coordinator.start_child_worker/1`, normalizing supervisor unavailability and preserving coordinator state with a `turn.activation_failed` event if startup cannot begin. +- Phase 82.3 records the shared application-supervisor contract in `docs/developer/06-conversation-orchestration.md` and adds the historical seeded batch to `docs/developer/10-development-workflow-and-quality-gates.md`. +- Phase 82.4 adds focused coordinator coverage for supervisor-backed child-worker startup, normal queued child-work activation, and the steer/supersession activation path while the child-supervisor name is temporarily unavailable. +- Verified with `mix test test/jido_code/conversations_coordinator_test.exs --max-cases 1 --max-failures 1` and the historical seeded combined conversation batch. + +[x] 82 Phase 82 - Conversation Runtime Supervisor Stability + Stabilize the conversation runtime child-work supervision contract so combined conversation suites are deterministic, queued work activation is resilient, and supervisor lifecycle assumptions are explicit in both runtime code and tests. + + [x] 82.1 Section - Failure Reproduction And Lifecycle Diagnosis + Capture the existing combined-suite failure as a bounded runtime problem before changing coordinator behavior. + + [x] 82.1.1 Task - Reproduce the combined conversation batch failure + Lock down the failing command and identify the minimal cross-file ordering or shared process state that makes the coordinator test fail. + + [x] 82.1.1.1 Subtask - Record the failing combined conversation command, seed behavior, and exact failure shape for `Conversations.ChildSupervisor`. + [x] 82.1.1.2 Subtask - Confirm the same coordinator test passes in isolation so the regression target is suite-order stability rather than the test's core assertions. + [x] 82.1.1.3 Subtask - Identify whether the failure comes from application supervision, test setup teardown, named process shutdown, or queued child-work activation timing. + + [x] 82.1.2 Task - Trace child-work activation ownership + Make the runtime ownership model clear enough that the fix lands at the correct boundary. + + [x] 82.1.2.1 Subtask - Trace where `Coordinator` starts queued `ChildWork` and where it assumes `ChildSupervisor` is globally available. + [x] 82.1.2.2 Subtask - Trace how tests start, stop, or replace conversation supervisors across async and sync files. + [x] 82.1.2.3 Subtask - Document the intended supervisor ownership in the phase implementation notes or developer guidance before broadening behavior. + + [x] 82.2 Section - Supervisor Availability Contract + Make child-work startup fail closed or recover through a product-owned path instead of crashing the coordinator when the child supervisor is unavailable. + + [x] 82.2.1 Task - Harden coordinator child-work startup + Ensure queued work activation handles unavailable supervision in a typed, recoverable way. + + [x] 82.2.1.1 Subtask - Replace raw `GenServer.call/3` assumptions to `Conversations.ChildSupervisor` with a bounded helper that normalizes `:noproc`, `:shutdown`, timeout, and start-child errors. + [x] 82.2.1.2 Subtask - Keep coordinator state consistent when child-work activation cannot start, including active turn, active child work, lifecycle events, and operator-visible error state. + [x] 82.2.1.3 Subtask - Preserve successful activation semantics for normal queued tool calls and runtime child work. + + [x] 82.2.2 Task - Normalize test supervision setup + Remove hidden order dependence from the conversation tests so each file can run alone or as part of the batch. + + [x] 82.2.2.1 Subtask - Ensure conversation tests that require `ChildSupervisor` start from a known supervised application state. + [x] 82.2.2.2 Subtask - Remove or isolate test cleanup that shuts down shared conversation runtime processes needed by later files. + [x] 82.2.2.3 Subtask - Keep forced failure tests scoped to their fixture process rather than leaking global supervisor state. + + [x] 82.3 Section - Runtime State And Contributor Convergence + Preserve the conversation runtime mental model while making supervisor failure modes explicit for future work. + + [x] 82.3.1 Task - Keep turn supersession and queued work semantics stable + Verify the stability fix does not change the core stop, steer, and queued child-work behavior that Phase 40 introduced. + + [x] 82.3.1.1 Subtask - Preserve supersession links when steering overtakes queued work. + [x] 82.3.1.2 Subtask - Preserve cancellation and settlement behavior for active and queued child work. + [x] 82.3.1.3 Subtask - Preserve event publication and persistence side effects around child-work state changes. + + [x] 82.3.2 Task - Update contributor guidance for conversation supervisor lifecycle + Make the stable test and runtime contract discoverable for future conversation-runtime changes. + + [x] 82.3.2.1 Subtask - Document which supervisor processes are shared application infrastructure versus per-test fixtures. + [x] 82.3.2.2 Subtask - Document the expected behavior when child-work startup cannot begin. + [x] 82.3.2.3 Subtask - Update planning or developer notes with the combined-suite command that protects this boundary. + + [x] 82.4 Section - Integration Tests + End the phase with regression coverage that proves the full conversation-runtime batch is stable, not only the individual failing test. + + [x] 82.4.1 Task - Add focused regression coverage for child-supervisor availability + Exercise the unavailable-supervisor path and the normal queued-child-work path without relying on cross-file ordering. + + [x] 82.4.1.1 Subtask - Add coverage for typed coordinator behavior when `ChildSupervisor` is unavailable during queued child-work activation. + [x] 82.4.1.2 Subtask - Add coverage proving the steer-overtakes-queued-work scenario still preserves supersession links. + [x] 82.4.1.3 Subtask - Add coverage proving normal queued child work still starts under the expected supervisor. + + [x] 82.4.2 Task - Run the relevant conversation-runtime suites + Verify the historical failure is closed across individual and combined execution modes. + + [x] 82.4.2.1 Subtask - Run `mix test test/jido_code/conversations_coordinator_test.exs --max-cases 1 --max-failures 1`. + [x] 82.4.2.2 Subtask - Run the combined conversation batch covering coordinator, driver, persistence, PubSub, and context-memory tests. + [x] 82.4.2.3 Subtask - Run any affected broader verification command required by touched conversation, memory, or workflow-provenance boundaries. diff --git a/.planning/phase-83-refactorer-conversation-routing-adoption.md b/.planning/phase-83-refactorer-conversation-routing-adoption.md new file mode 100644 index 0000000..e59800a --- /dev/null +++ b/.planning/phase-83-refactorer-conversation-routing-adoption.md @@ -0,0 +1,106 @@ +# Phase 83 - Refactorer Conversation Routing Adoption + + + + + +Back to index: [README](https://github.com/mikehostetler/jido_code/blob/main/.planning/README.md) + +## Relevant Shared APIs / Interfaces +- `lib/jido_code/conversations/workflow_router.ex` +- `lib/jido_code/conversations/work_resolution.ex` +- `lib/jido_code/conversations/runtime.ex` +- `lib/jido_code/conversations/driver.ex` +- `lib/jido_code/agent_workspace.ex` +- `lib/jido_code/agents/refactorer.ex` +- `docs/developer/04-coding-pod-and-specialist-workflows.md` +- `docs/developer/05-specialist-prompts-context-and-tool-execution.md` +- `docs/developer/12-user-request-to-llm-message-path.md` +- `test/jido_code/conversations_driver_test.exs` +- `test/jido_code/conversations_coordinator_test.exs` +- `test/jido_code/agent_workspace_test.exs` + +## Relevant Assumptions / Defaults +- Phase 81 exposes `AgentWorkspace.refactor_work/3,4` as the product-owned API for the lazy `Refactorer` specialist. +- Conversation workflow routing currently treats the canonical productive workflows as `:plan`, `:execute`, `:review`, and `:explain`. +- `full_workflow/3,4` remains plan -> execute -> review; this phase adopts conversation routing only and should not silently add refactor to full workflow orchestration. +- Refactor routing should be deterministic and explicit enough that generic edit, fix, or patch requests still route to `:execute` unless the operator's intent is behavior-preserving refactoring. + +## Implementation Notes +- Phase 83.1 adds `:refactor` to `WorkflowRouter.workflows/0`, normalization, default scores, metadata, and deterministic cues for explicit behavior-preserving refactor intent. +- Execute routing keeps generic implementation, fix, edit, update, and patch cues; refactor-specific scoring is separated so broad implementation requests do not silently move to the Refactorer. +- Phase 83.2 routes refactor runtime requests through `AgentWorkspace.refactor_work/4`, projects `:refactoring` results, includes refactor in governed work resolution, and adds refactor-aware memory workflow policy. +- Phase 83.3 updates developer guidance and current-truth notes so conversation-level refactor routing is discoverable while `full_workflow/3,4` remains plan -> execute -> review. +- Phase 83.4 adds focused router, conversation runtime, work-item identity, and memory workflow coverage for refactor routing. +- Verified with the Phase 83 integration test, Phase 52 routing regression test, conversation-runtime batch, AgentWorkspace tests, memory workflow service tests, and `mix memory.verify`. + +[x] 83 Phase 83 - Refactorer Conversation Routing Adoption + Adopt the existing Refactorer specialist into deterministic conversation workflow routing so explicit behavior-preserving refactor requests reach `AgentWorkspace.refactor_work/3,4` without weakening execute, review, explain, or full-workflow semantics. + + [x] 83.1 Section - Routing Model And Intent Contract + Extend the canonical workflow routing model with refactor intent while keeping routing explainable and bounded. + + [x] 83.1.1 Task - Add `:refactor` to the canonical workflow router + Teach the product-owned routing boundary that refactor is a valid specialist workflow with its own scoring, metadata, and normalization path. + + [x] 83.1.1.1 Subtask - Extend workflow normalization, workflow lists, and routing metadata to include `:refactor`. + [x] 83.1.1.2 Subtask - Add deterministic refactor cues such as "refactor", "extract", "rename", "simplify", and "preserve behavior" without stealing generic implementation requests from `:execute`. + [x] 83.1.1.3 Subtask - Keep routing reasons inspectable so operators and tests can distinguish explicit refactor intent from implementation or review intent. + + [x] 83.1.2 Task - Preserve routing precedence and ambiguity behavior + Make refactor adoption follow the same explicit intent, continuity, and clarification rules established by deterministic workflow routing. + + [x] 83.1.2.1 Subtask - Let explicit product intent select `:refactor` ahead of free-text cues. + [x] 83.1.2.2 Subtask - Preserve active work-item workflow continuity for refactor turns unless the operator explicitly changes workflow. + [x] 83.1.2.3 Subtask - Clarify ambiguous requests when refactor and execute or review signals are tied or weak. + + [x] 83.2 Section - Runtime Dispatch And Result Projection + Route refactor decisions through the same conversation runtime, governed work, and workspace boundaries as other productive workflows. + + [x] 83.2.1 Task - Dispatch refactor workflow through `AgentWorkspace.refactor_work/4` + Make the conversation runtime call the public workspace refactor API rather than addressing the pod or specialist directly. + + [x] 83.2.1.1 Subtask - Add runtime dispatch for `:refactor` that passes managed repo id, work item id, bounded instruction, workspace path, LLM selection, semantic context, memory context, and provenance options. + [x] 83.2.1.2 Subtask - Preserve typed unavailable or degraded states when workspace path, CodingPod runtime, graph context, or specialist execution cannot run. + [x] 83.2.1.3 Subtask - Ensure refactor runtime results project the `:refactoring` payload into conversation output without disrupting existing plan, execute, review, and explain projections. + + [x] 83.2.2 Task - Align governed work resolution with refactor conversations + Keep work-item identity and admission rules coherent when a conversation turn requests behavior-preserving refactoring. + + [x] 83.2.2.1 Subtask - Include `:refactor` in productive work-resolution workflows that require governed work attachment. + [x] 83.2.2.2 Subtask - Preserve one active productive conversation per WorkItem and allow parallel refactor conversations across different work items. + [x] 83.2.2.3 Subtask - Keep refactor conversation provenance and prompt-memory behavior bounded by the same work-item scope as other specialist workflows. + + [x] 83.3 Section - Surface Guidance And Current-Truth Convergence + Make refactor routing understandable to operators and contributors while preserving the current product-owned boundaries. + + [x] 83.3.1 Task - Update developer guidance for conversation-level refactor routing + Align docs with the new workflow path so future contributors know when to use refactor instead of execute. + + [x] 83.3.1.1 Subtask - Update the user-request-to-LLM path guide to list `:refactor` as a conversation-routed workflow once implemented. + [x] 83.3.1.2 Subtask - Update CodingPod and specialist prompt guides to distinguish workspace API exposure from conversation runtime adoption. + [x] 83.3.1.3 Subtask - Document that `full_workflow/3,4` still remains plan -> execute -> review unless a later phase changes that orchestration contract. + + [x] 83.3.2 Task - Update product-facing routing metadata and degraded states + Keep refactor workflow status observable and typed at the same level as other conversation workflows. + + [x] 83.3.2.1 Subtask - Include refactor workflow names and labels wherever conversation workflow metadata is surfaced to operators. + [x] 83.3.2.2 Subtask - Preserve route-owned recovery copy when refactor routing cannot start because runtime, workspace, or graph context is unavailable. + [x] 83.3.2.3 Subtask - Keep product surfaces from exposing pod-local details such as node names, process ids, or specialist internals. + + [x] 83.4 Section - Integration Tests + End the phase by proving explicit refactor conversations route through the Refactorer while existing workflows keep their current behavior. + + [x] 83.4.1 Task - Add focused routing and runtime coverage + Verify `:refactor` is selected, dispatched, and projected only when routing inputs justify it. + + [x] 83.4.1.1 Subtask - Add coverage proving explicit refactor intent routes to `:refactor` and invokes `AgentWorkspace.refactor_work/4`. + [x] 83.4.1.2 Subtask - Add coverage proving generic fix, edit, and implementation requests still route to `:execute`. + [x] 83.4.1.3 Subtask - Add coverage proving ambiguous execute/refactor or review/refactor requests clarify rather than silently choosing the wrong specialist. + + [x] 83.4.2 Task - Add end-to-end conversation coverage + Verify refactor routing behaves correctly through governed work-item conversations and existing runtime context boundaries. + + [x] 83.4.2.1 Subtask - Add an end-to-end conversation test that creates or reuses governed work and records a refactor turn result. + [x] 83.4.2.2 Subtask - Add coverage proving refactor turns preserve work-item conversation identity and do not allow a second active conversation for the same WorkItem. + [x] 83.4.2.3 Subtask - Run the conversation-runtime suite, AgentWorkspace refactor coverage, and any memory or semantic verification commands required by touched boundaries. diff --git a/docs/developer/04-coding-pod-and-specialist-workflows.md b/docs/developer/04-coding-pod-and-specialist-workflows.md index b073ddc..8c108ff 100644 --- a/docs/developer/04-coding-pod-and-specialist-workflows.md +++ b/docs/developer/04-coding-pod-and-specialist-workflows.md @@ -146,11 +146,16 @@ Use the workspace entrypoint rather than direct pod or specialist calls. It: `full_workflow/3,4` still remains plan -> code -> review. Refactoring is an explicit stage until a later phase changes default workflow orchestration. -Conversation routing does not currently infer a dedicated refactor workflow. -If conversation or workflow surfaces adopt refactorer dispatch later, they -should map explicit refactor intent to `AgentWorkspace.refactor_work/3,4` and -return typed product-facing unavailable or degraded states when the refactorer -cannot run. +Conversation routing now treats explicit behavior-preserving refactor intent as +the `:refactor` workflow and dispatches it through +`AgentWorkspace.refactor_work/3,4`. Generic fix, edit, update, and patch +requests still route to `:execute`; use refactor when the requested change is +structural cleanup, extraction, rename, deduplication, or simplification that +should preserve behavior. + +Conversation and workflow surfaces should keep this as a product-owned +workspace route. They should not expose pod-local details such as node names, +process ids, or specialist internals when refactor startup degrades. ## Pod Teardown diff --git a/docs/developer/05-specialist-prompts-context-and-tool-execution.md b/docs/developer/05-specialist-prompts-context-and-tool-execution.md index a52ff7e..be949f0 100644 --- a/docs/developer/05-specialist-prompts-context-and-tool-execution.md +++ b/docs/developer/05-specialist-prompts-context-and-tool-execution.md @@ -246,6 +246,11 @@ sequentially, but prompt chaining is not the main contract. explicit workspace entrypoint, not a default stage in full workflow orchestration. +Conversation runtime may route explicit behavior-preserving refactor requests +to `refactor_work/3,4`. That is specialist selection, not prompt rewriting: +the original operator request is still wrapped as bounded conversation context +before it reaches the Refactorer. + ### Relatedness Is Not Semantic Today If you send two unrelated prompts to the same specialist on the same work item, diff --git a/docs/developer/06-conversation-orchestration.md b/docs/developer/06-conversation-orchestration.md index af5748f..e653375 100644 --- a/docs/developer/06-conversation-orchestration.md +++ b/docs/developer/06-conversation-orchestration.md @@ -31,6 +31,8 @@ It is not intended to be a free-floating page-local chat buffer. | `Conversations` | top-level product-facing API for reads and persistence lookups | | `Driver` | product-owned runtime boundary for opening conversations and handling commands | | `Coordinator` | active process that owns command admission, turn state, sequencing, and cancellation | +| `Conversations.ChildSupervisor` | application-owned dynamic supervisor for cancellable child-work processes | +| `ChildWorker` | runtime wrapper for one active unit of cancellable child work | | `WorkResolution` | promotes productive repo conversation turns into canonical governed `WorkItem` scope before durable specialist execution continues | | `Command` | typed control and work commands | | `Event` / `EventRecord` | append-only event stream and durable event persistence | @@ -85,6 +87,37 @@ Each active conversation has a coordinator process because someone has to own: That ownership makes interruption and reconnect behavior explainable. +## Child Work Supervision + +The coordinator owns conversation state. `Conversations.ChildSupervisor` owns +the normal OTP supervision path for cancellable child work. Tests and features +should treat it as shared application infrastructure, not as a per-test fixture +to stop or replace casually. + +When the coordinator starts queued child work, it goes through `ChildWorker` +rather than calling the dynamic supervisor directly. That boundary normalizes +short supervisor shutdown windows and missing-supervisor failures into typed +start results. If supervised startup is temporarily unavailable, the child +worker may use a bounded coordinator-owned fallback so existing queued turn +semantics continue without crashing the coordinator. If startup still cannot +begin, the coordinator preserves recoverable state, leaves the queued turn out +of the active slot, and appends a `turn.activation_failed` event with an +operator-facing remediation payload. + +Do not write tests that depend on conversation test files running in a specific +order. When changing child work, the coordinator, runtime startup, or +conversation supervisor ownership, include the seeded combined batch in the +verification set: + +```bash +mix test test/jido_code/conversations_driver_test.exs \ + test/jido_code/conversations_coordinator_test.exs \ + test/jido_code/conversations_test.exs \ + test/jido_code/conversations_pubsub_test.exs \ + test/jido_code/conversations/context_memory_test.exs \ + --seed 871949 --max-cases 1 --max-failures 1 +``` + ## Event-Driven Delivery Live updates are expected to flow through conversation events and PubSub. diff --git a/docs/developer/10-development-workflow-and-quality-gates.md b/docs/developer/10-development-workflow-and-quality-gates.md index 8ef2024..4f20fd7 100644 --- a/docs/developer/10-development-workflow-and-quality-gates.md +++ b/docs/developer/10-development-workflow-and-quality-gates.md @@ -45,6 +45,7 @@ before editing: | `mix source_graph.verify` | verify semantic source-code graph stack | | `mix memory.verify` | verify memory graph and capture-plane behavior | | `mix test test/jido_code/conversations/context_memory_test.exs test/jido_code/phase_seventy_eight_integration_test.exs` | verify prompt context memory adapter and runtime integration | +| `mix test test/jido_code/conversations_driver_test.exs test/jido_code/conversations_coordinator_test.exs test/jido_code/conversations_test.exs test/jido_code/conversations_pubsub_test.exs test/jido_code/conversations/context_memory_test.exs --seed 871949 --max-cases 1 --max-failures 1` | verify the historical conversation child-supervisor lifecycle regression | | `mix semantic.verify` | verify product-facing semantic behavior | | `mix docs` | build repo docs surface | @@ -57,6 +58,8 @@ Run the narrower verification commands when you touch those boundaries: - memory graph or capture boundaries -> `mix memory.verify` - prompt context memory adapter or conversation prompt recall changes -> run the focused prompt-memory tests listed above +- conversation coordinator, child-work supervision, or conversation runtime + startup changes -> run the historical seeded conversation batch listed above - product-facing semantic surfaces or services -> `mix semantic.verify` ## Branching And Collaboration diff --git a/docs/developer/12-user-request-to-llm-message-path.md b/docs/developer/12-user-request-to-llm-message-path.md index f5ed3e8..5ea402c 100644 --- a/docs/developer/12-user-request-to-llm-message-path.md +++ b/docs/developer/12-user-request-to-llm-message-path.md @@ -126,16 +126,20 @@ That value is still visible as the request later in - `:plan` - `:execute` +- `:refactor` - `:review` - `:explain` For words like `fix`, `change`, `edit`, or `patch`, it usually infers `:execute`. -The `refactorer` specialist is exposed through `AgentWorkspace.refactor_work/3,4`, -but conversation workflow inference does not currently choose a dedicated -`:refactor` workflow. Conversation adoption should be explicit if it is added -later. +For explicit behavior-preserving refactor requests, such as extracting shared +logic, renaming structure, deduplicating code, or simplifying code without +changing behavior, it infers `:refactor` and later dispatches through +`AgentWorkspace.refactor_work/3,4`. + +If refactor and execute or review signals are mixed without clear intent, the +runtime should ask for clarification instead of silently choosing a specialist. This matters because it decides: @@ -296,6 +300,14 @@ Memory context: Then the `coder` system prompt is added above it as the `system` message. +For a request like: + +`Refactor the parser helpers to extract duplication while preserving behavior` + +the same path preserves that text, infers workflow `:refactor`, attaches +governed work, and routes the bounded instruction to the Refactorer specialist +through `AgentWorkspace.refactor_work/3,4`. + ## What Actually Reaches The Model The effective LLM messages look conceptually like: diff --git a/lib/jido_code/conversations.ex b/lib/jido_code/conversations.ex index b151d94..6449d9c 100644 --- a/lib/jido_code/conversations.ex +++ b/lib/jido_code/conversations.ex @@ -833,6 +833,7 @@ defmodule JidoCode.Conversations do defp workflow_label("plan"), do: "planning" defp workflow_label("execute"), do: "implementation" + defp workflow_label("refactor"), do: "refactoring" defp workflow_label("review"), do: "review" defp workflow_label("explain"), do: "follow-up" defp workflow_label(_workflow_name), do: "governed" diff --git a/lib/jido_code/conversations/child_worker.ex b/lib/jido_code/conversations/child_worker.ex index 5294ac8..49cbba0 100644 --- a/lib/jido_code/conversations/child_worker.ex +++ b/lib/jido_code/conversations/child_worker.ex @@ -16,6 +16,7 @@ defmodule JidoCode.Conversations.ChildWorker do alias JidoCode.Control.Actor @supervisor JidoCode.Conversations.ChildSupervisor + @supervisor_start_attempts 2 @type runtime_spec :: map() @type state :: %{ @@ -34,7 +35,60 @@ defmodule JidoCode.Conversations.ChildWorker do @spec start(ChildWork.t()) :: {:ok, pid()} | {:error, term()} def start(%ChildWork{} = child_work) do - DynamicSupervisor.start_child(@supervisor, {__MODULE__, child_work}) + start(child_work, @supervisor_start_attempts) + end + + defp start(%ChildWork{} = child_work, attempts_remaining) do + case start_once(child_work) do + {:error, {:child_supervisor_unavailable, _reason}} when attempts_remaining > 0 -> + wait_for_supervisor_recovery() + start(child_work, attempts_remaining - 1) + + {:error, {:child_supervisor_unavailable, reason}} -> + start_without_supervisor(child_work, reason) + + result -> + result + end + end + + defp start_once(%ChildWork{} = child_work) do + case Process.whereis(@supervisor) do + supervisor_pid when is_pid(supervisor_pid) -> + try do + DynamicSupervisor.start_child(@supervisor, {__MODULE__, child_work}) + catch + :exit, reason -> {:error, {:child_supervisor_unavailable, reason}} + end + + _other -> + {:error, {:child_supervisor_unavailable, :not_started}} + end + end + + defp wait_for_supervisor_recovery do + case Process.whereis(@supervisor) do + supervisor_pid when is_pid(supervisor_pid) -> + ref = Process.monitor(supervisor_pid) + + receive do + {:DOWN, ^ref, :process, ^supervisor_pid, _reason} -> :ok + after + 25 -> Process.demonitor(ref, [:flush]) + end + + _other -> + :ok + end + end + + defp start_without_supervisor(%ChildWork{} = child_work, supervisor_reason) do + try do + GenServer.start(__MODULE__, child_work) + catch + :exit, reason -> + {:error, {:child_supervisor_unavailable, supervisor_reason, {:isolated_start_failed, reason}}} + end end @spec snapshot(pid()) :: {:ok, ChildWork.t()} @@ -123,14 +177,14 @@ defmodule JidoCode.Conversations.ChildWorker do maybe_allow_test_sandbox(runtime_pid, runtime_spec) {:reply, {:ok, child_work}, - %{ - state - | runtime_pid: runtime_pid, - runtime_ref: runtime_ref, - runtime_spec: runtime_spec, - runtime_status: :running, - runtime_managed?: true - }} + %{ + state + | runtime_pid: runtime_pid, + runtime_ref: runtime_ref, + runtime_spec: runtime_spec, + runtime_status: :running, + runtime_managed?: true + }} end @impl true @@ -149,8 +203,7 @@ defmodule JidoCode.Conversations.ChildWorker do when terminal_kind in [:completed, :failed] and is_map(payload) do with {:ok, updated_child_work} <- apply_runtime_terminal_update(state.child_work, payload), :ok <- dispatch_runtime_payload(updated_child_work, payload) do - {:stop, :normal, - clear_runtime(%{state | child_work: updated_child_work, runtime_status: :idle})} + {:stop, :normal, clear_runtime(%{state | child_work: updated_child_work, runtime_status: :idle})} else _other -> {:stop, :normal, clear_runtime(%{state | runtime_status: :idle})} @@ -161,8 +214,7 @@ defmodule JidoCode.Conversations.ChildWorker do when is_map(payload) do with {:ok, updated_child_work} <- apply_runtime_update(child_work, payload), :ok <- dispatch_runtime_payload(updated_child_work, payload) do - {:noreply, - clear_runtime(%{state | child_work: updated_child_work, runtime_status: :idle})} + {:noreply, clear_runtime(%{state | child_work: updated_child_work, runtime_status: :idle})} else _other -> {:noreply, clear_runtime(%{state | runtime_status: :idle})} diff --git a/lib/jido_code/conversations/coordinator.ex b/lib/jido_code/conversations/coordinator.ex index d8d9421..b1bf9ec 100644 --- a/lib/jido_code/conversations/coordinator.ex +++ b/lib/jido_code/conversations/coordinator.ex @@ -328,24 +328,28 @@ defmodule JidoCode.Conversations.Coordinator do with {:ok, prepared_state} <- maybe_prepare_runtime_scope(state, next_turn_id), %Turn{} = next_turn <- Map.fetch!(prepared_state.turns, next_turn_id), {:ok, running_turn} <- Turn.transition(next_turn, :running), - child_work <- ChildWork.new(prepared_state.conversation, running_turn), - {:ok, pid} <- ChildWorker.start(child_work), - {:ok, running_child_work} <- ChildWorker.snapshot(pid) do - running_turn = %{running_turn | child_work_id: running_child_work.id} + child_work <- ChildWork.new(prepared_state.conversation, running_turn) do + case start_child_worker(child_work) do + {:ok, pid, running_child_work} -> + running_turn = %{running_turn | child_work_id: running_child_work.id} + + next_state = + prepared_state + |> Map.put(:active_turn_id, next_turn_id) + |> Map.put(:work_queue, remaining_turn_ids) + |> update_in([:child_work_order], &(&1 ++ [running_child_work.id])) + |> put_in([:child_worker_pids, running_child_work.id], pid) + |> store_turn(running_turn, actor: running_turn.actor, message_id: running_turn.command_id) + |> store_child_work(running_child_work, + actor: running_child_work.actor, + message_id: running_turn.command_id + ) - next_state = - prepared_state - |> Map.put(:active_turn_id, next_turn_id) - |> Map.put(:work_queue, remaining_turn_ids) - |> update_in([:child_work_order], &(&1 ++ [running_child_work.id])) - |> put_in([:child_worker_pids, running_child_work.id], pid) - |> store_turn(running_turn, actor: running_turn.actor, message_id: running_turn.command_id) - |> store_child_work(running_child_work, - actor: running_child_work.actor, - message_id: running_turn.command_id - ) + maybe_schedule_runtime_for_turn(next_state, running_turn.id) - maybe_schedule_runtime_for_turn(next_state, running_turn.id) + {:error, reason} -> + {:ok, append_turn_activation_failed_event(prepared_state, next_turn, reason)} + end end end @@ -419,7 +423,7 @@ defmodule JidoCode.Conversations.Coordinator do defp restart_child_worker_for_runtime(state, child_work_id) do case Map.fetch(state.child_works, child_work_id) do {:ok, %ChildWork{} = child_work} -> - with {:ok, pid} <- ChildWorker.start(child_work) do + with {:ok, pid, _running_child_work} <- start_child_worker(child_work) do {:ok, pid, put_in(state, [:child_worker_pids, child_work_id], pid)} end @@ -428,6 +432,31 @@ defmodule JidoCode.Conversations.Coordinator do end end + defp start_child_worker(%ChildWork{} = child_work) do + case ChildWorker.start(child_work) do + {:ok, pid} when is_pid(pid) -> + case child_worker_snapshot(pid) do + {:ok, %ChildWork{} = running_child_work} -> + {:ok, pid, running_child_work} + + {:error, _reason} = error -> + maybe_terminate_child_worker(pid) + error + end + + {:error, _reason} = error -> + error + end + end + + defp child_worker_snapshot(pid) when is_pid(pid) do + try do + ChildWorker.snapshot(pid) + catch + :exit, reason -> {:error, {:child_worker_snapshot_failed, reason}} + end + end + defp runtime_spec(state, turn, child_work) do shared_context = Snapshot.from_state(state).shared_context @@ -829,7 +858,7 @@ defmodule JidoCode.Conversations.Coordinator do {:ok, pid} -> child_work = Map.fetch!(state.child_works, child_work_id) {:ok, settled_child_work} = ChildWork.settle(child_work, turn_state) - _ = DynamicSupervisor.terminate_child(JidoCode.Conversations.ChildSupervisor, pid) + maybe_terminate_child_worker(pid) state |> store_child_work(settled_child_work, actor: actor) @@ -1267,6 +1296,20 @@ defmodule JidoCode.Conversations.Coordinator do }) end + defp append_turn_activation_failed_event(state, %Turn{} = turn, reason) do + append_event(state, "turn.activation_failed", %{ + actor: turn.actor, + message_id: turn.command_id, + turn_id: turn.id, + correlation: turn_correlation(turn), + payload: %{ + "state" => Atom.to_string(turn.state), + "command_type" => turn.command_type, + "error" => child_worker_start_error(reason) + } + }) + end + defp append_status_event(state, normalized_command) do append_event(state, "conversation.status_changed", %{ actor: normalized_command.actor, @@ -1474,25 +1517,38 @@ defmodule JidoCode.Conversations.Coordinator do end defp maybe_terminate_child_worker(pid) when is_pid(pid) do - case Process.whereis(JidoCode.Conversations.ChildSupervisor) do - supervisor when is_pid(supervisor) -> - if Process.alive?(pid) do + if Process.alive?(pid) do + case Process.whereis(JidoCode.Conversations.ChildSupervisor) do + supervisor when is_pid(supervisor) -> try do - _ = DynamicSupervisor.terminate_child(JidoCode.Conversations.ChildSupervisor, pid) + case DynamicSupervisor.terminate_child(JidoCode.Conversations.ChildSupervisor, pid) do + :ok -> :ok + {:error, :not_found} -> terminate_child_worker_process(pid) + {:error, _reason} -> terminate_child_worker_process(pid) + end catch - :exit, _reason -> :ok + :exit, _reason -> terminate_child_worker_process(pid) end - else - :ok - end - _other -> - :ok + _other -> + terminate_child_worker_process(pid) + end + else + :ok end end defp maybe_terminate_child_worker(_pid), do: :ok + defp terminate_child_worker_process(pid) when is_pid(pid) do + if Process.alive?(pid) do + Process.exit(pid, :shutdown) + :ok + else + :ok + end + end + defp maybe_allow_test_sandbox(sandbox_owner, starter_pid) do [sandbox_owner, starter_pid] |> Enum.filter(&is_pid/1) @@ -1560,6 +1616,24 @@ defmodule JidoCode.Conversations.Coordinator do defp child_work_event_name(_previous_child_work, %ChildWork{state: :failed}), do: "tool.failed" defp child_work_event_name(_previous_child_work, _updated_child_work), do: nil + defp child_worker_start_error({:child_supervisor_unavailable, reason}) do + %{ + "error_type" => "conversation_child_supervisor_unavailable", + "detail" => "Conversation child work could not start because the child supervisor was unavailable.", + "reason" => inspect(reason), + "remediation" => "Retry or resume the conversation turn after conversation runtime supervision recovers." + } + end + + defp child_worker_start_error(reason) do + %{ + "error_type" => "conversation_child_worker_start_failed", + "detail" => "Conversation child work could not start.", + "reason" => inspect(reason), + "remediation" => "Retry or resume the conversation turn after conversation runtime services recover." + } + end + defp turn_correlation(%Turn{} = turn) do %{} |> maybe_put("command_id", turn.command_id) diff --git a/lib/jido_code/conversations/runtime.ex b/lib/jido_code/conversations/runtime.ex index 50a4ee5..612be20 100644 --- a/lib/jido_code/conversations/runtime.ex +++ b/lib/jido_code/conversations/runtime.ex @@ -313,7 +313,7 @@ defmodule JidoCode.Conversations.Runtime do end defp invoke(%{workflow: workflow} = request, runtime_spec, readiness) - when workflow in [:plan, :execute, :review, :explain] do + when workflow in [:plan, :execute, :refactor, :review, :explain] do actor = map_get(runtime_spec, :actor) case select_context_source(workflow) do @@ -332,7 +332,7 @@ defmodule JidoCode.Conversations.Runtime do end defp invoke_memory_workflow(%{workflow: workflow} = request, readiness, actor) - when workflow in [:plan, :execute, :review, :explain] do + when workflow in [:plan, :execute, :refactor, :review, :explain] do memory_opts = [workspace_path: readiness.workspace_path, prepare: :recover_if_needed] result = @@ -359,6 +359,17 @@ defmodule JidoCode.Conversations.Runtime do llm_selection: readiness.llm_selection ) + :refactor -> + MemoryWorkflowService.refactor( + request.managed_repo_id, + request.work_item_id, + request.instruction, + memory: memory_opts, + workspace_path: readiness.workspace_path, + actor: actor, + llm_selection: readiness.llm_selection + ) + :review -> MemoryWorkflowService.review( request.managed_repo_id, @@ -427,7 +438,7 @@ defmodule JidoCode.Conversations.Runtime do end defp invoke_workspace(%{workflow: workflow} = request, readiness, actor, extra_opts) - when workflow in [:plan, :execute, :review, :explain] do + when workflow in [:plan, :execute, :refactor, :review, :explain] do opts = [workspace_path: readiness.workspace_path, actor: actor, llm_selection: readiness.llm_selection] |> Keyword.merge(List.wrap(extra_opts)) @@ -449,6 +460,14 @@ defmodule JidoCode.Conversations.Runtime do opts ) + :refactor -> + AgentWorkspace.refactor_work( + request.managed_repo_id, + request.work_item_id, + request.instruction, + opts + ) + :review -> AgentWorkspace.review_work( request.managed_repo_id, @@ -621,7 +640,7 @@ defmodule JidoCode.Conversations.Runtime do end _workflow -> - # For execute workflow, use workspace with semantic fallback + # Execute and refactor use workspace dispatch with optional semantic context. invoke_workspace_with_semantic_fallback(request, readiness, actor) end end @@ -679,7 +698,7 @@ defmodule JidoCode.Conversations.Runtime do %{ "kind" => "progress", "summary" => - "Requesting clarification before choosing whether to plan, implement, review, or explain this repository work.", + "Requesting clarification before choosing whether to plan, implement, refactor, review, or explain this repository work.", "workflow" => "clarify", "context_source" => context_source_name(request.context_source), "referenced_files" => request.referenced_files, @@ -737,7 +756,7 @@ defmodule JidoCode.Conversations.Runtime do } end - defp prompt_memory_kinds(workflow) when workflow in [:execute, :review] do + defp prompt_memory_kinds(workflow) when workflow in [:execute, :refactor, :review] do [ :active_constraint, :accepted_tool_result, @@ -999,6 +1018,7 @@ defmodule JidoCode.Conversations.Runtime do case workflow do :plan -> Map.get(result, :plan) || Map.get(result, "plan") :execute -> Map.get(result, :changes) || Map.get(result, "changes") + :refactor -> Map.get(result, :refactoring) || Map.get(result, "refactoring") :review -> Map.get(result, :feedback) || Map.get(result, "feedback") :explain -> Map.get(result, :explanation) || Map.get(result, "explanation") end @@ -1009,12 +1029,15 @@ defmodule JidoCode.Conversations.Runtime do text = (instruction || "") |> String.downcase() contains_any?(text, ["clarify", "needs input", "which file", "what file"]) or - (workflow in [:execute, :review] and contains_any?(text, ["file", "module", "function"])) + (workflow in [:execute, :refactor, :review] and contains_any?(text, ["file", "module", "function"])) end defp clarification_prompt(:execute), do: "Which file or module should I change first?" + defp clarification_prompt(:refactor), + do: "Which file, module, or behavior-preserving structure should I refactor first?" + defp clarification_prompt(:review), do: "Which file, module, or diff should I review first?" @@ -1022,12 +1045,12 @@ defmodule JidoCode.Conversations.Runtime do do: "Which file or module should I inspect first?" defp workflow_clarification_prompt do - "Do you want me to plan, implement, review, or explain this request?" + "Do you want me to plan, implement, refactor, review, or explain this request?" end defp select_context_source(nil), do: :workflow_clarification - defp select_context_source(:execute) do + defp select_context_source(workflow) when workflow in [:execute, :refactor] do cond do memory_enabled?() -> :memory_workflow semantic_enabled?() -> :workspace_with_semantic diff --git a/lib/jido_code/conversations/work_resolution.ex b/lib/jido_code/conversations/work_resolution.ex index 1b12b7e..9e39498 100644 --- a/lib/jido_code/conversations/work_resolution.ex +++ b/lib/jido_code/conversations/work_resolution.ex @@ -10,7 +10,7 @@ defmodule JidoCode.Conversations.WorkResolution do alias JidoCode.Conversations.{Conversation, Turn} alias JidoCode.Conversations.WorkflowRouter - @governed_workflows [:plan, :execute, :review, :explain] + @governed_workflows [:plan, :execute, :refactor, :review, :explain] @type resolution_summary :: map() @@ -142,6 +142,7 @@ defmodule JidoCode.Conversations.WorkResolution do defp workflow_label(nil), do: "governed" defp workflow_label(:plan), do: "planning" defp workflow_label(:execute), do: "implementation" + defp workflow_label(:refactor), do: "refactoring" defp workflow_label(:review), do: "review" defp workflow_label(:explain), do: "follow-up" defp workflow_label(_workflow), do: "governed" @@ -152,6 +153,9 @@ defmodule JidoCode.Conversations.WorkResolution do defp workflow_resolution_reason(:execute), do: "Implementation work must continue through the canonical governed WorkItem loop." + defp workflow_resolution_reason(:refactor), + do: "Behavior-preserving refactoring work must remain attached to canonical governed WorkItem scope." + defp workflow_resolution_reason(:review), do: "Review work must remain attached to canonical governed WorkItem scope." diff --git a/lib/jido_code/conversations/workflow_router.ex b/lib/jido_code/conversations/workflow_router.ex index 69eddde..3df61a1 100644 --- a/lib/jido_code/conversations/workflow_router.ex +++ b/lib/jido_code/conversations/workflow_router.ex @@ -10,7 +10,7 @@ defmodule JidoCode.Conversations.WorkflowRouter do deterministic text heuristics. """ - @workflows [:plan, :execute, :review, :explain] + @workflows [:plan, :execute, :refactor, :review, :explain] @surface_intent_keys ["surface_workflow", "surface_intent", "workflow_hint", "workflow"] @@ -41,13 +41,35 @@ defmodule JidoCode.Conversations.WorkflowRouter do {"fix", 8}, {"update", 5}, {"edit", 5}, - {"refactor", 6}, {"write", 5}, {"add", 4}, {"remove", 4}, {"patch", 6} ] }, + refactor: %{ + phrases: [ + {"refactor this", 18}, + {"refactor the", 16}, + {"behavior preserving", 20}, + {"preserve behavior", 20}, + {"without changing behavior", 20}, + {"extract function", 16}, + {"extract module", 16}, + {"rename without changing", 18}, + {"simplify without changing", 18} + ], + tokens: [ + {"refactor", 10}, + {"refactoring", 10}, + {"extract", 8}, + {"rename", 7}, + {"simplify", 6}, + {"deduplicate", 8}, + {"reorganize", 6}, + {"cleanup", 6} + ] + }, review: %{ phrases: [ {"review this", 14}, @@ -90,7 +112,7 @@ defmodule JidoCode.Conversations.WorkflowRouter do } } - @type workflow :: :plan | :execute | :review | :explain + @type workflow :: :plan | :execute | :refactor | :review | :explain @type decision :: %{ workflow: workflow() | nil, @@ -172,6 +194,11 @@ defmodule JidoCode.Conversations.WorkflowRouter do case String.trim(value) do "plan" -> :plan "execute" -> :execute + "implement" -> :execute + "implementation" -> :execute + "refactor" -> :refactor + "refactoring" -> :refactor + "refactor_work" -> :refactor "review" -> :review "explain" -> :explain _other -> nil diff --git a/lib/jido_code/memory_graph/retrieval_policy.ex b/lib/jido_code/memory_graph/retrieval_policy.ex index 8919867..60284a9 100644 --- a/lib/jido_code/memory_graph/retrieval_policy.ex +++ b/lib/jido_code/memory_graph/retrieval_policy.ex @@ -1,7 +1,7 @@ defmodule JidoCode.MemoryGraph.RetrievalPolicy do @moduledoc false - @type workflow_kind :: :plan | :execute | :review | :explain + @type workflow_kind :: :plan | :execute | :refactor | :review | :explain @type freshness_mode :: :ready_only | :allow_stale @type provenance_scope :: :none | :workflow_history | :workflow_and_governed_history @type policy :: %{ @@ -19,6 +19,7 @@ defmodule JidoCode.MemoryGraph.RetrievalPolicy do @default_limits %{ plan: 8, execute: 8, + refactor: 8, review: 10, explain: 6 } @@ -40,6 +41,14 @@ defmodule JidoCode.MemoryGraph.RetrievalPolicy do provenance_scope: :workflow_and_governed_history, follow_up_intent: :work_item }, + refactor: %{ + intent: :refactoring_constraints, + memory_kinds: [:decision, :invariant, :convention, :known_issue, :pattern, :anti_pattern], + provenance_kinds: [:plan, :review, :patch, :agent_run], + freshness: :ready_only, + provenance_scope: :workflow_and_governed_history, + follow_up_intent: :refactor + }, review: %{ intent: :review_risks, memory_kinds: [:known_issue, :anti_pattern, :decision, :lesson_learned], @@ -83,23 +92,25 @@ defmodule JidoCode.MemoryGraph.RetrievalPolicy do @intent_map %{ "planning_constraints" => :planning_constraints, "implementation_constraints" => :implementation_constraints, + "refactoring_constraints" => :refactoring_constraints, "review_risks" => :review_risks, "explanation_context" => :explanation_context, "work_item" => :work_item, + "refactor" => :refactor, "review_support" => :review_support, "explanation" => :explanation } @spec normalize(workflow_kind(), keyword() | map() | nil) :: {:ok, policy()} | {:error, atom()} - def normalize(workflow, nil) when workflow in [:plan, :execute, :review, :explain] do + def normalize(workflow, nil) when workflow in [:plan, :execute, :refactor, :review, :explain] do {:ok, default_policy(workflow)} end - def normalize(workflow, opts) when workflow in [:plan, :execute, :review, :explain] and is_list(opts) do + def normalize(workflow, opts) when workflow in [:plan, :execute, :refactor, :review, :explain] and is_list(opts) do normalize(workflow, Map.new(opts)) end - def normalize(workflow, attrs) when workflow in [:plan, :execute, :review, :explain] and is_map(attrs) do + def normalize(workflow, attrs) when workflow in [:plan, :execute, :refactor, :review, :explain] and is_map(attrs) do defaults = default_policy(workflow) with {:ok, intent} <- normalize_atom(Map.get(attrs, :intent) || Map.get(attrs, "intent"), defaults.intent), diff --git a/lib/jido_code/memory_graph/workflow_service.ex b/lib/jido_code/memory_graph/workflow_service.ex index 6c21e8a..c159cbb 100644 --- a/lib/jido_code/memory_graph/workflow_service.ex +++ b/lib/jido_code/memory_graph/workflow_service.ex @@ -7,8 +7,8 @@ defmodule JidoCode.MemoryGraph.WorkflowService do @moduledoc """ Product-owned memory workflow boundary over AgentWorkspace. - This module keeps planning, coding, review, and explanation flows explicit - about when durable memory or workflow provenance context is requested and + This module keeps planning, coding, refactoring, review, and explanation + flows explicit about when durable memory or workflow provenance context is requested and returns bounded memory input maps rather than raw graph query details. """ @@ -17,7 +17,7 @@ defmodule JidoCode.MemoryGraph.WorkflowService do alias JidoCode.MemoryGraph alias JidoCode.MemoryGraph.{CaptureEnvelope, ProductFeedback, ProductService, RetrievalPolicy} - @type workflow_kind :: :plan | :execute | :review | :explain + @type workflow_kind :: :plan | :execute | :refactor | :review | :explain @type workflow_result :: {:ok, map()} | {:error, term()} | {:error, atom(), map()} @workflow_provenance_actor Actor.factory_system_actor(%{ "id" => "system:memory-graph-workflow-provenance", @@ -39,6 +39,11 @@ defmodule JidoCode.MemoryGraph.WorkflowService do run_workflow(:execute, managed_repo_id, work_item_id, instruction, opts) end + @spec refactor(String.t(), String.t(), String.t(), keyword()) :: workflow_result() + def refactor(managed_repo_id, work_item_id, instruction, opts \\ []) do + run_workflow(:refactor, managed_repo_id, work_item_id, instruction, opts) + end + @spec explain(String.t(), String.t(), String.t(), keyword()) :: workflow_result() def explain(managed_repo_id, work_item_id, instruction, opts \\ []) do run_workflow(:explain, managed_repo_id, work_item_id, instruction, opts) @@ -49,7 +54,7 @@ defmodule JidoCode.MemoryGraph.WorkflowService do def follow_up_context(_result), do: nil defp run_workflow(workflow, managed_repo_id, work_item_id, instruction, opts) - when workflow in [:plan, :execute, :review, :explain] and is_list(opts) do + when workflow in [:plan, :execute, :refactor, :review, :explain] and is_list(opts) do with {:ok, memory_opts} <- normalize_memory_opts(Keyword.get(opts, :memory)), {:ok, workflow_provenance} <- workflow_provenance_context( @@ -96,6 +101,9 @@ defmodule JidoCode.MemoryGraph.WorkflowService do defp invoke_workspace(:execute, managed_repo_id, work_item_id, instruction, opts), do: AgentWorkspace.execute_work(managed_repo_id, work_item_id, instruction, opts) + defp invoke_workspace(:refactor, managed_repo_id, work_item_id, instruction, opts), + do: AgentWorkspace.refactor_work(managed_repo_id, work_item_id, instruction, opts) + defp invoke_workspace(:review, managed_repo_id, work_item_id, instruction, opts), do: AgentWorkspace.review_work(managed_repo_id, work_item_id, instruction, opts) @@ -399,6 +407,28 @@ defmodule JidoCode.MemoryGraph.WorkflowService do } end + defp shape_result( + :refactor, + managed_repo_id, + work_item_id, + instruction, + raw_result, + memory_input, + workflow_provenance + ) do + %{ + workflow: :refactor, + managed_repo_id: managed_repo_id, + work_item_id: work_item_id, + instruction: instruction, + refactoring: Map.get(raw_result, :refactoring), + memory_input: memory_input, + workflow_provenance: provenance_summary(workflow_provenance), + follow_up_context: follow_up_context(:refactor, memory_input, workflow_provenance), + llm_selection: Map.get(raw_result, :llm_selection) + } + end + defp shape_result(:review, managed_repo_id, work_item_id, instruction, raw_result, memory_input, workflow_provenance) do %{ workflow: :review, diff --git a/test/jido_code/conversations_coordinator_test.exs b/test/jido_code/conversations_coordinator_test.exs index 4361a35..6c32277 100644 --- a/test/jido_code/conversations_coordinator_test.exs +++ b/test/jido_code/conversations_coordinator_test.exs @@ -506,6 +506,112 @@ defmodule JidoCode.ConversationsCoordinatorTest do assert superseded_snapshot.queued_turn_ids == [queued_turn_id] end + test "child worker start uses child supervisor when available" do + conversation = conversation_fixture() + assert_child_supervisor_accepts_child!(conversation) + end + + test "queued child work starts during normal activation" do + conversation = conversation_fixture() + assert_child_supervisor_accepts_child!(conversation) + start_coordinator!(conversation) + + actor = %{"id" => "operator-supervised-child-work", "actor_class" => "operator"} + + assert {:ok, first_snapshot} = + Coordinator.admit_command( + conversation.id, + %{type: "turn.submit", payload: %{instruction: "Inspect the current branch."}}, + actor + ) + + assert {:ok, queued_snapshot} = + Coordinator.admit_command( + conversation.id, + %{type: "turn.submit", payload: %{instruction: "Prepare the follow-up work."}}, + actor + ) + + queued_turn_id = List.first(queued_snapshot.queued_turn_ids) + + assert {:ok, activated_snapshot} = + Coordinator.settle_child_work( + conversation.id, + first_snapshot.active_child_work_id, + :completed, + %{result: %{summary: "First child work completed."}}, + actor + ) + + assert activated_snapshot.active_turn.id == queued_turn_id + assert activated_snapshot.active_child_work.state == :running + refute Enum.any?(activated_snapshot.events, &(&1.name == "turn.activation_failed")) + + pid = coordinator_child_worker_pid(conversation.id, activated_snapshot.active_child_work_id) + assert Process.alive?(pid) + end + + test "queued replacement turn activates while child supervisor name is temporarily unavailable" do + conversation = conversation_fixture() + start_coordinator!(conversation) + + actor = %{"id" => "operator-child-supervisor-unavailable", "actor_class" => "operator"} + + assert {:ok, first_snapshot} = + Coordinator.admit_command( + conversation.id, + %{type: "turn.submit", payload: %{instruction: "Inspect the failing workflow."}}, + actor + ) + + assert {:ok, second_snapshot} = + Coordinator.admit_command( + conversation.id, + %{type: "turn.submit", payload: %{instruction: "Prepare the old follow-up plan."}}, + actor + ) + + queued_turn_id = List.first(second_snapshot.queued_turn_ids) + + assert {:ok, steering_snapshot} = + Coordinator.admit_command( + conversation.id, + %{ + type: "turn.steer", + payload: %{instruction: "Narrow the scope to the failing test only."} + }, + actor + ) + + replacement_turn = List.last(steering_snapshot.turns) + + assert {:ok, superseded_snapshot} = + with_unregistered_child_supervisor(fn -> + Coordinator.settle_child_work( + conversation.id, + first_snapshot.active_child_work_id, + :cancelled, + %{result: %{reason: "Steering replaced the previous turn."}}, + actor + ) + end) + + superseded_turn = + Enum.find(superseded_snapshot.turns, &(&1.id == first_snapshot.active_turn.id)) + + replacement_turn = Enum.find(superseded_snapshot.turns, &(&1.id == replacement_turn.id)) + + assert superseded_turn.state == :superseded + assert superseded_turn.superseded_by_turn_id == replacement_turn.id + assert superseded_snapshot.active_turn.id == replacement_turn.id + assert superseded_snapshot.active_turn.state == :running + assert superseded_snapshot.active_child_work.state == :running + assert superseded_snapshot.queued_turn_ids == [queued_turn_id] + + pid = coordinator_child_worker_pid(conversation.id, superseded_snapshot.active_child_work_id) + assert Process.alive?(pid) + end + test "turn transition keeps lifecycle history explicit" do now = DateTime.utc_now() |> DateTime.truncate(:microsecond) @@ -594,13 +700,63 @@ defmodule JidoCode.ConversationsCoordinatorTest do defp start_coordinator!(conversation) do start_supervised!( - {Coordinator, - {conversation, - starter_pid: self(), - sandbox_owner: Process.get({JidoCode.Repo, :sandbox_owner})}} + {Coordinator, {conversation, starter_pid: self(), sandbox_owner: Process.get({JidoCode.Repo, :sandbox_owner})}} ) end + defp coordinator_child_worker_pid(conversation_id, child_work_id) do + Coordinator.via_tuple(conversation_id) + |> :sys.get_state() + |> Map.fetch!(:child_worker_pids) + |> Map.fetch!(child_work_id) + end + + defp supervised_child_worker?(pid) when is_pid(pid) do + JidoCode.Conversations.ChildSupervisor + |> DynamicSupervisor.which_children() + |> Enum.any?(fn {_id, child_pid, _type, _modules} -> child_pid == pid end) + end + + defp assert_child_supervisor_accepts_child!(conversation) do + now = DateTime.utc_now() |> DateTime.truncate(:microsecond) + + turn = + Turn.new(conversation.id, %{ + id: Ecto.UUID.generate(), + raw_type: "turn.submit", + payload: %{}, + admitted_at: now + }) + + child_work = ChildWork.new(conversation, turn) + + assert {:ok, pid} = + DynamicSupervisor.start_child( + JidoCode.Conversations.ChildSupervisor, + {JidoCode.Conversations.ChildWorker, child_work} + ) + + assert supervised_child_worker?(pid) + assert :ok = DynamicSupervisor.terminate_child(JidoCode.Conversations.ChildSupervisor, pid) + end + + defp with_unregistered_child_supervisor(fun) when is_function(fun, 0) do + supervisor_name = JidoCode.Conversations.ChildSupervisor + supervisor_pid = Process.whereis(supervisor_name) + + assert is_pid(supervisor_pid) + assert Process.unregister(supervisor_name) + + try do + assert Process.whereis(supervisor_name) == nil + fun.() + after + if Process.whereis(supervisor_name) == nil and Process.alive?(supervisor_pid) do + Process.register(supervisor_pid, supervisor_name) + end + end + end + defp managed_repo_fixture!(suffix) do {:ok, project} = Project.create(%{ diff --git a/test/jido_code/memory_graph_workflow_service_test.exs b/test/jido_code/memory_graph_workflow_service_test.exs index 4a664d0..44cf386 100644 --- a/test/jido_code/memory_graph_workflow_service_test.exs +++ b/test/jido_code/memory_graph_workflow_service_test.exs @@ -208,6 +208,43 @@ defmodule JidoCode.MemoryGraphWorkflowServiceTest do assert plan_resource_iri in result.follow_up_context["provenance_resources"] end + test "refactor returns bounded memory workflow inputs with refactor-focused defaults", %{ + workspace_path: workspace_path + } do + managed_repo_id = "repo-#{System.unique_integer([:positive])}" + work_item_id = "work-#{System.unique_integer([:positive])}" + revision = "rev-83-memory-refactor" + + %{memory_resource_iri: memory_resource_iri} = + seed_memory_graph!(managed_repo_id, workspace_path, revision) + + assert {:ok, result} = + WorkflowService.refactor( + managed_repo_id, + work_item_id, + "Refactor with memory context", + workspace_path: workspace_path, + memory: [ + workspace_path: workspace_path, + prepare: :recover_if_needed, + revision: revision + ] + ) + + assert result.workflow == :refactor + assert result.refactoring =~ "deterministic refactorer response" + assert result.memory_input.workflow == :refactor + assert result.memory_input.policy.intent == :refactoring_constraints + assert result.memory_input.policy.follow_up_intent == :refactor + assert :anti_pattern in result.memory_input.policy.memory_kinds + assert :agent_run in result.memory_input.policy.provenance_kinds + assert memory_resource_iri in result.memory_input.selection.memory_resources + assert result.workflow_provenance.workflow == :refactor + assert result.workflow_provenance.follow_up_intent == :refactor + assert result.follow_up_context["workflow"] == "refactor" + assert result.follow_up_context["retrieval_policy"]["intent"] == "refactoring_constraints" + end + test "workflow memory requests fail safely with explicit freshness and recovery feedback", %{ workspace_path: workspace_path } do diff --git a/test/jido_code/phase_eighty_three_integration_test.exs b/test/jido_code/phase_eighty_three_integration_test.exs new file mode 100644 index 0000000..58940f6 --- /dev/null +++ b/test/jido_code/phase_eighty_three_integration_test.exs @@ -0,0 +1,196 @@ +defmodule JidoCode.PhaseEightyThreeIntegrationTest do + # covers: architecture.conversation_orchestration.workflow_routing_is_deterministic_and_product_owned + # covers: architecture.conversation_orchestration.explicit_workflow_intent_and_continuity_take_precedence + # covers: architecture.agent_os_integration.product_work_entrypoints_route_to_workspace + use JidoCode.DataCase, async: false + + alias JidoCode.AgentWorkspace + alias JidoCode.Control.{Actor, ManagedRepo} + alias JidoCode.Conversations.WorkflowRouter + alias JidoCode.Projects.Project + alias JidoCode.Workbench.{ProjectConversation, ProjectDetail} + + test "workflow router selects refactor only for explicit behavior-preserving intent" do + assert :refactor in WorkflowRouter.workflows() + + explicit = + WorkflowRouter.resolve(%{ + payload: %{ + "instruction" => "Review the code and implement the cleanup.", + "workflow" => "refactor" + } + }) + + assert explicit.workflow == :refactor + assert explicit.source == :explicit_payload + + refactor = + WorkflowRouter.resolve(%{ + payload: %{ + "instruction" => "Refactor the parser helpers to extract duplicate behavior while preserving behavior." + } + }) + + assert refactor.workflow == :refactor + assert refactor.source == :heuristic + assert refactor.scores.refactor > refactor.scores.execute + + execute = + WorkflowRouter.resolve(%{ + payload: %{ + "instruction" => "Fix failing tests and update the implementation." + } + }) + + assert execute.workflow == :execute + assert execute.ambiguous? == false + + ambiguous = + WorkflowRouter.resolve(%{ + payload: %{ + "instruction" => "Refactor this module and review the diff." + } + }) + + assert ambiguous.workflow == nil + assert ambiguous.ambiguous? == true + assert ambiguous.candidate_workflow in [:refactor, :review] + end + + test "conversation refactor intent routes through refactorer and preserves work-item identity" do + {project, managed_repo} = managed_repo_fixture!("conversation-refactor") + + assert {:ok, project_detail} = ProjectDetail.load(project.id) + + assert {:ok, %{conversation: conversation, resumed?: false}} = + ProjectConversation.open_repo_detail( + project_detail, + actor: Actor.operator_actor(%{"id" => "operator-phase83-open"}) + ) + + track_runtime!(conversation.id, managed_repo.id) + + assert {:ok, _snapshot} = + AgentWorkspace.handle_conversation_command( + conversation.id, + %{ + type: "turn.submit", + payload: %{ + instruction: "Refactor lib/example/parser.ex to extract duplicate helpers while preserving behavior." + } + }, + actor: Actor.operator_actor(%{"id" => "operator-phase83-submit"}) + ) + + completed_snapshot = + eventually_snapshot!(conversation.id, fn snapshot -> + snapshot.active_turn == nil and snapshot.active_child_work == nil and + accepted_result_matches?(snapshot, "refactor", "deterministic refactorer response") and + routing_workflow_present?(snapshot, "refactor") + end) + + assert is_binary(completed_snapshot.work_item_id) + + assert {:ok, %{conversation: resumed_conversation, resumed?: true}} = + AgentWorkspace.open_work_item_conversation( + completed_snapshot.work_item_id, + %{}, + actor: Actor.operator_actor(%{"id" => "operator-phase83-resume"}) + ) + + assert resumed_conversation.id == conversation.id + + assert {:ok, active_conversations} = + AgentWorkspace.active_work_item_conversations(managed_repo.id, + actor: Actor.operator_actor(%{"id" => "operator-phase83-active"}) + ) + + assert Enum.count(active_conversations, &(&1.work_item_id == completed_snapshot.work_item_id)) == 1 + end + + defp accepted_result_matches?(snapshot, workflow, summary_fragment) do + Enum.any?(snapshot.shared_context["accepted_tool_results"], fn result -> + summary = get_in(result, ["result", "summary"]) || "" + + get_in(result, ["result", "workflow"]) == workflow and + String.contains?(summary, summary_fragment) + end) + end + + defp routing_workflow_present?(snapshot, workflow) do + Enum.any?(snapshot.turns, fn turn -> + get_in(turn, [:payload, "conversation_runtime", "routing", "workflow"]) == workflow + end) + end + + defp track_runtime!(conversation_id, managed_repo_id) do + on_exit(fn -> + case AgentWorkspace.stop_conversation(conversation_id) do + :ok -> :ok + {:error, _reason} -> :ok + end + + case AgentWorkspace.shutdown_kernel(managed_repo_id) do + :ok -> :ok + {:error, _reason} -> :ok + end + end) + end + + defp managed_repo_fixture!(suffix) do + {:ok, project} = + Project.create(%{ + name: "phase-eighty-three-#{suffix}", + github_full_name: "owner/phase-eighty-three-#{suffix}", + default_branch: "main", + settings: %{ + "workspace" => %{ + "workspace_environment" => "local", + "workspace_path" => workspace_path!(suffix), + "clone_status" => "ready", + "workspace_initialized" => true, + "baseline_synced" => true + } + } + }) + + {:ok, managed_repo} = + ManagedRepo.get_by_legacy_project_id(project.id, actor: Actor.operator_actor()) + + {project, managed_repo} + end + + defp workspace_path!(suffix) do + workspace_path = + Path.join( + System.tmp_dir!(), + "jido-code-phase-eighty-three-#{suffix}-#{System.unique_integer([:positive])}" + ) + + File.mkdir_p!(workspace_path) + on_exit(fn -> File.rm_rf(workspace_path) end) + workspace_path + end + + defp eventually_snapshot!(conversation_id, predicate, attempts \\ 40) + + defp eventually_snapshot!(conversation_id, predicate, attempts) + when is_binary(conversation_id) and is_function(predicate, 1) and attempts > 1 do + assert {:ok, snapshot} = AgentWorkspace.conversation_snapshot(conversation_id) + + if predicate.(snapshot) do + snapshot + else + receive do + after + 25 -> eventually_snapshot!(conversation_id, predicate, attempts - 1) + end + end + end + + defp eventually_snapshot!(conversation_id, predicate, _attempts) do + assert {:ok, snapshot} = AgentWorkspace.conversation_snapshot(conversation_id) + assert predicate.(snapshot) + snapshot + end +end diff --git a/test/jido_code/phase_fifty_two_integration_test.exs b/test/jido_code/phase_fifty_two_integration_test.exs index bd3fb4f..effc5d6 100644 --- a/test/jido_code/phase_fifty_two_integration_test.exs +++ b/test/jido_code/phase_fifty_two_integration_test.exs @@ -126,7 +126,7 @@ defmodule JidoCode.PhaseFiftyTwoIntegrationTest do eventually_snapshot!(conversation.id, fn snapshot -> match?(%{state: :awaiting_input}, snapshot.active_turn) and get_in(snapshot, [:shared_context, "pending_clarification", "prompt", "prompt"]) == - "Do you want me to plan, implement, review, or explain this request?" and + "Do you want me to plan, implement, refactor, review, or explain this request?" and snapshot.work_item_id == nil and get_in(snapshot.active_turn, [:payload, "conversation_runtime", "routing", "ambiguous"]) == true end)