diff --git a/apps/edge/internal/openai/artifact_pair.go b/apps/edge/internal/openai/artifact_pair.go index a5d6c8f3..e408253d 100644 --- a/apps/edge/internal/openai/artifact_pair.go +++ b/apps/edge/internal/openai/artifact_pair.go @@ -232,12 +232,11 @@ func (s *artifactFrontierStore) expandSelectorPair( return normalizedStageOutput{}, fmt.Errorf("artifact pair write binding is unavailable") } call := output.ToolCalls[0] - planContent, planOK := call.Arguments["plan_content"].(string) - reviewContent, reviewOK := call.Arguments["review_content"].(string) - if !planOK || strings.TrimSpace(planContent) == "" || !reviewOK || strings.TrimSpace(reviewContent) == "" { - return normalizedStageOutput{}, fmt.Errorf("artifact pair tool requires complete plan_content and review_content") + paths := newReservedPaths(requestID) + planContent, reviewContent, err := renderArtifactPair(call.Arguments, paths.ReviewPath) + if err != nil { + return normalizedStageOutput{}, err } - planContent = normalizeCallerWorkspacePlanBoundary(planContent) providerID := strings.TrimSpace(call.ProviderCallID) if providerID == "" { providerID = strings.TrimSpace(call.ID) @@ -255,7 +254,6 @@ func (s *artifactFrontierStore) expandSelectorPair( if !validLogicalRequestID(providerID) { return normalizedStageOutput{}, fmt.Errorf("artifact pair provider tool id is invalid") } - paths := newReservedPaths(requestID) output.ToolCalls = []normalizedToolCall{ {ID: pairProviderCallID(providerID, "plan"), ProviderCallID: pairProviderCallID(providerID, "plan"), Name: write.toolName, Arguments: map[string]any{"path": paths.PlanPath, "content": planContent}}, {ID: pairProviderCallID(providerID, "review"), ProviderCallID: pairProviderCallID(providerID, "review"), Name: write.toolName, Arguments: map[string]any{"path": paths.ReviewPath, "content": reviewContent}}, @@ -269,24 +267,45 @@ func (s *artifactFrontierStore) expandSelectorPair( return output, nil } -func normalizeCallerWorkspacePlanBoundary(content string) string { - const marker = "## Goal\n" - start := strings.Index(content, marker) - if start < 0 { - return content +func renderArtifactPair(arguments map[string]any, reviewPath string) (string, string, error) { + goal, _ := arguments["goal"].(string) + steps := compactArtifactStrings(arguments["steps"]) + verification := compactArtifactStrings(arguments["verification"]) + if strings.TrimSpace(goal) == "" || len(steps) == 0 || len(steps) > 5 || len(verification) == 0 || len(verification) > 3 { + return "", "", fmt.Errorf("artifact pair tool requires a goal, 1-5 steps, and 1-3 verification checks") } - goalStart := start + len(marker) - goalEnd := strings.IndexByte(content[goalStart:], '\n') - if goalEnd < 0 { - goalEnd = len(content) - } else { - goalEnd += goalStart + + var plan strings.Builder + fmt.Fprintf(&plan, "# Plan\n\n## Goal\n%s %s\n\n## Steps\n", hotPathCallerWorkspacePlanBoundary, strings.TrimSpace(goal)) + for i, step := range steps { + fmt.Fprintf(&plan, "- [P%d] %s\n", i+1, step) } - goal := strings.TrimSpace(content[goalStart:goalEnd]) - if strings.HasPrefix(goal, hotPathCallerWorkspacePlanBoundary) { - return content + handoffID := len(steps) + 1 + fmt.Fprintf(&plan, "- [P%d] Read `%s`, then as the final Work action replace the whole file with one full-file Write while preserving its exact headings; mark every P item completed and record actual changes, verification evidence, and deviations.\n", handoffID, reviewPath) + plan.WriteString("\n## Verification\n") + for _, check := range verification { + fmt.Fprintf(&plan, "- %s\n", check) } - return content[:goalStart] + hotPathCallerWorkspacePlanBoundary + " " + goal + content[goalEnd:] + + var review strings.Builder + review.WriteString("# Review\n\n## Worker Item Status\n") + for i := 1; i <= handoffID; i++ { + fmt.Fprintf(&review, "- P%d: pending\n", i) + } + review.WriteString("\n## Worker Changes\nPending worker execution.\n\n## Worker Verification\nPending worker verification.\n\n## Deviations\nNone recorded.\n") + return plan.String(), review.String(), nil +} + +func compactArtifactStrings(value any) []string { + items, _ := value.([]any) + out := make([]string, 0, len(items)) + for _, item := range items { + text, _ := item.(string) + if text = strings.TrimSpace(text); text != "" { + out = append(out, text) + } + } + return out } func pairProviderCallID(base, role string) string { diff --git a/apps/edge/internal/openai/artifact_pair_test.go b/apps/edge/internal/openai/artifact_pair_test.go index af559a34..781340e6 100644 --- a/apps/edge/internal/openai/artifact_pair_test.go +++ b/apps/edge/internal/openai/artifact_pair_test.go @@ -27,18 +27,6 @@ func testLightweightPlan(t *testing.T) string { return string(plan) } -func TestNormalizeCallerWorkspacePlanBoundary(t *testing.T) { - plan := testLightweightPlan(t) - normalized := normalizeCallerWorkspacePlanBoundary(plan) - want := "## Goal\n" + hotPathCallerWorkspacePlanBoundary + " Complete the caller workspace task" - if !strings.Contains(normalized, want) { - t.Fatalf("normalized plan omitted workspace boundary: %s", normalized) - } - if second := normalizeCallerWorkspacePlanBoundary(normalized); second != normalized { - t.Fatalf("workspace boundary normalization is not idempotent: %s", second) - } -} - func testPendingReview(t *testing.T) string { t.Helper() review, err := singlerequesttemplate.RenderReview(singlerequesttemplate.DefaultReviewTemplate, singlerequesttemplate.ReviewFields{ @@ -347,10 +335,10 @@ func TestArtifactSelectorAtomicPairExpandsToCallerWrites(t *testing.T) { fixture := newArtifactPairFixture(t, "openai", true) output, err := fixture.server.artifactFrontiers.expandSelectorPair(fixture.requestID, fixture.ownerEdgeID, normalizedStageOutput{ Content: "selector-private text", Reasoning: "selector-private reasoning", - Deltas: []normalizedStageDelta{{Kind: normalizedStageDeltaTool, ToolID: "provider_pair", ToolName: hotPathArtifactPairToolName, Arguments: `{"plan_content":"old"}`}}, + Deltas: []normalizedStageDelta{{Kind: normalizedStageDeltaTool, ToolID: "provider_pair", ToolName: hotPathArtifactPairToolName, Arguments: `{"goal":"old"}`}}, ToolCalls: []normalizedToolCall{{ ID: "provider_pair", Name: hotPathArtifactPairToolName, - Arguments: map[string]any{"plan_content": testLightweightPlan(t), "review_content": testPendingReview(t)}, + Arguments: map[string]any{"goal": "Build the requested page.", "steps": []any{"Create index.html with the requested content."}, "verification": []any{"Confirm index.html contains the required marker."}}, }}, }) if err != nil { @@ -363,6 +351,13 @@ func TestArtifactSelectorAtomicPairExpandsToCallerWrites(t *testing.T) { output.ToolCalls[1].Name != "workspace" || output.ToolCalls[1].Arguments["path"] != fixture.paths.ReviewPath { t.Fatalf("expanded caller writes=%+v", output.ToolCalls) } + plan, _ := output.ToolCalls[0].Arguments["content"].(string) + review, _ := output.ToolCalls[1].Arguments["content"].(string) + if !strings.Contains(plan, "## Goal\n"+hotPathCallerWorkspacePlanBoundary+" Build the requested page.") || + !strings.Contains(plan, "- [P2] Read `"+fixture.paths.ReviewPath+"`") || + !strings.Contains(review, "- P1: pending\n- P2: pending") { + t.Fatalf("rendered artifact pair mismatch: plan=%q review=%q", plan, review) + } if output.ToolCalls[0].ProviderCallID == output.ToolCalls[1].ProviderCallID { t.Fatalf("expanded provider ids must be distinct: %+v", output.ToolCalls) } @@ -377,7 +372,7 @@ func TestArtifactSelectorAtomicPairDropsGeminiThoughtSignatureEnvelope(t *testin output, err := fixture.server.artifactFrontiers.expandSelectorPair(fixture.requestID, fixture.ownerEdgeID, normalizedStageOutput{ ToolCalls: []normalizedToolCall{{ ID: providerID, ProviderCallID: providerID, Name: hotPathArtifactPairToolName, - Arguments: map[string]any{"plan_content": testLightweightPlan(t), "review_content": testPendingReview(t)}, + Arguments: map[string]any{"goal": "Build the requested page.", "steps": []any{"Create index.html with the requested content."}, "verification": []any{"Confirm index.html contains the required marker."}}, }}, }) if err != nil { diff --git a/apps/edge/internal/openai/hot_path_direct_test.go b/apps/edge/internal/openai/hot_path_direct_test.go index ce8cb841..25235558 100644 --- a/apps/edge/internal/openai/hot_path_direct_test.go +++ b/apps/edge/internal/openai/hot_path_direct_test.go @@ -461,19 +461,12 @@ func scriptedSelectorDirective(providerBody []byte, operation string) (string, s return requestID, state, nil } required := []string{ - "Return exactly one iop_write_artifact_pair tool call", + "return exactly one iop_write_artifact_pair tool call", "PLAN path: " + paths.PlanPath, "REVIEW path: " + paths.ReviewPath, - "You are the Planner.", - "Analyze the immutable user task first.", - "Preserve every explicit requirement, constraint, deliverable, and acceptance condition.", - "# Plan\n\n## Goal\n" + hotPathCallerWorkspacePlanBoundary + " \n\n## Steps\n- [P1] \n- [P2] ", - "The pending REVIEW must use exactly the Plan's P1..Pn inventory in order", - "# Review\n\n## Worker Item Status\n- P1: pending\n- P2: pending", - "## Worker Changes\nPending worker execution.", - "## Worker Verification\nPending worker verification.", - "## Deviations\nNone recorded.", - "Do not copy the explanatory optional-status text into REVIEW.", + "Keep every explicit requirement, exact literal, filename, command, output string, and acceptance condition.", + "Do not write markdown; IOP renders the fixed Plan and pending Review templates.", + "IOP will append the final Review handoff step and matching pending status itself.", } for _, fragment := range required { if !strings.Contains(instruction, fragment) { diff --git a/apps/edge/internal/openai/provider_model_rewrite.go b/apps/edge/internal/openai/provider_model_rewrite.go index 1a93236c..6e529682 100644 --- a/apps/edge/internal/openai/provider_model_rewrite.go +++ b/apps/edge/internal/openai/provider_model_rewrite.go @@ -35,50 +35,14 @@ Do not write PLAN or REVIEW in this turn. Do not mention or infer an absolute wo case selectorInstructionPairWrite: instruction = fmt.Sprintf(`IOP caller-workspace selector instruction. Operation: pair-write -Return exactly one iop_write_artifact_pair tool call containing both complete artifact contents and no other tool call. Do not mention or infer an absolute workspace path. +Analyze the immutable user task, then return exactly one iop_write_artifact_pair tool call and no other tool call. +Keep every explicit requirement, exact literal, filename, command, output string, and acceptance condition. Do not add scope. +Write all fields in English using ASCII characters only. Supply one concise goal, 1-5 executable worker steps, and 1-3 observable verification checks. +Do not write markdown; IOP renders the fixed Plan and pending Review templates. +Do not mention or infer an absolute workspace path. Task outputs belong under the caller workspace current working directory, never under .iop unless the user explicitly requests that exact path. PLAN path: %s REVIEW path: %s -You are the Planner. Apply this compact Plan workflow before authoring the pair: -1. Analyze the immutable user task first. Do not create a separate analysis artifact. -2. Preserve every explicit requirement, constraint, deliverable, and acceptance condition. Do not invent extra scope. - Copy every exact literal, filename, command, and required output string from the task verbatim into an executable Plan step or Verification bullet. -3. Convert that analysis into 2-6 closed, executable steps. The Worker must be able to implement without rediscovering requirements or choosing among alternatives. -4. Write 1-3 deterministic verification bullets with observable pass conditions. -5. Resolve every requested output path against the caller workspace current working directory. Never place a task output beside PLAN or anywhere under .iop unless the immutable user task explicitly requests that exact path. -6. Make the final Plan step require the Worker to read the pending REVIEW path above, then as the Work stage's final action replace the whole file with one full-file Write operation while preserving its exact headings. Never use Edit or Patch for this handoff. Every Plan item status must be exactly "completed"; actual changes, actual verification evidence, and deviations must be non-empty. - -Write every PLAN field in English using ASCII characters only and use exactly this grammar: -# Plan - -## Goal -Use the caller workspace current working directory as the task root; never place task outputs under .iop. - -## Steps -- [P1] -- [P2] -- [P3] - -## Verification -- - -Omit optional step lines that are not needed. The pending REVIEW must use exactly the Plan's P1..Pn inventory in order and exactly this grammar: -# Review - -## Worker Item Status -- P1: pending -- P2: pending -- P3: pending only when P3 exists in PLAN; continue for every chosen Plan id - -## Worker Changes -Pending worker execution. - -## Worker Verification -Pending worker verification. - -## Deviations -None recorded. - -Do not copy the explanatory optional-status text into REVIEW. Emit one pending status line for every and only the actual Plan ids.`, paths.PlanPath, paths.ReviewPath) +IOP will append the final Review handoff step and matching pending status itself.`, paths.PlanPath, paths.ReviewPath) default: return "", fmt.Errorf("selector provider instruction state is invalid") } @@ -203,14 +167,19 @@ func prepareHotPathSelectorCanonicalTools(tunnel edgeservice.SubmitProviderTunne "type": "function", "function": map[string]any{ "name": hotPathArtifactPairToolName, - "description": "Author the complete IOP Plan and pending Review artifact pair in one atomic selector decision.", + "description": "Return the compact fields used by IOP to render a Plan and pending Review pair.", "parameters": map[string]any{ "type": "object", "properties": map[string]any{ - "plan_content": map[string]any{"type": "string", "description": "Complete Plan markdown"}, - "review_content": map[string]any{"type": "string", "description": "Complete pending Review markdown"}, + "goal": map[string]any{"type": "string", "description": "Concise task goal"}, + "steps": map[string]any{ + "type": "array", "description": "Executable worker steps", "items": map[string]any{"type": "string"}, "minItems": 1, "maxItems": 5, + }, + "verification": map[string]any{ + "type": "array", "description": "Observable pass checks", "items": map[string]any{"type": "string"}, "minItems": 1, "maxItems": 3, + }, }, - "required": []any{"plan_content", "review_content"}, + "required": []any{"goal", "steps", "verification"}, "additionalProperties": false, }, }, diff --git a/apps/edge/internal/openai/provider_model_rewrite_test.go b/apps/edge/internal/openai/provider_model_rewrite_test.go index 396cc68b..3c0100dd 100644 --- a/apps/edge/internal/openai/provider_model_rewrite_test.go +++ b/apps/edge/internal/openai/provider_model_rewrite_test.go @@ -329,19 +329,13 @@ func TestHotPathSelectorPairInstructionCarriesCompactPlanContract(t *testing.T) t.Fatal(err) } for _, fragment := range []string{ - "You are the Planner.", - "Analyze the immutable user task first.", - "Preserve every explicit requirement, constraint, deliverable, and acceptance condition.", - "Copy every exact literal, filename, command, and required output string", - "The Worker must be able to implement without rediscovering requirements", - "Resolve every requested output path against the caller workspace current working directory.", - "Never place a task output beside PLAN or anywhere under .iop", - hotPathCallerWorkspacePlanBoundary, - "Make the final Plan step require the Worker to read the pending REVIEW path", - "replace the whole file with one full-file Write operation", - "Never use Edit or Patch for this handoff.", - "Every Plan item status must be exactly \"completed\"", - "actual verification evidence", + "Analyze the immutable user task", + "Keep every explicit requirement, exact literal, filename, command, output string, and acceptance condition.", + "1-5 executable worker steps", + "1-3 observable verification checks", + "Do not write markdown; IOP renders the fixed Plan and pending Review templates.", + "Task outputs belong under the caller workspace current working directory", + "IOP will append the final Review handoff step and matching pending status itself.", } { if !strings.Contains(instruction, fragment) { t.Fatalf("selector instruction omitted compact Plan contract %q: %s", fragment, instruction) @@ -391,7 +385,7 @@ func TestHotPathSelectorCanonicalWriteToolReplacesCallerCommandSchema(t *testing function := tools[0].(map[string]any)["function"].(map[string]any) parameters := function["parameters"].(map[string]any) properties := parameters["properties"].(map[string]any) - if function["name"] != hotPathArtifactPairToolName || properties["plan_content"] == nil || properties["review_content"] == nil || properties["command"] != nil { + if function["name"] != hotPathArtifactPairToolName || properties["goal"] == nil || properties["steps"] == nil || properties["verification"] == nil || properties["command"] != nil { t.Fatalf("canonical write function=%+v", function) } choice := request["tool_choice"].(map[string]any)