diff --git a/backend/src/CodeSpace.Core/Services/Supervisor/Deciders/LlmSupervisorDecider.cs b/backend/src/CodeSpace.Core/Services/Supervisor/Deciders/LlmSupervisorDecider.cs index fbff73a9b..344fc4c11 100644 --- a/backend/src/CodeSpace.Core/Services/Supervisor/Deciders/LlmSupervisorDecider.cs +++ b/backend/src/CodeSpace.Core/Services/Supervisor/Deciders/LlmSupervisorDecider.cs @@ -844,6 +844,10 @@ private static string BuildUserPrompt(SupervisorTurnContext context, string cata builder.AppendLine(); } + // The operator's acceptance FLOOR — the argv the server runs on every branch a stop ships. Unlike the criteria above, + // which are only a yardstick, this one RUNS, and a failure ends the run. Null / empty ⇒ no block ⇒ byte-identical prompt. + AppendOperatorAcceptanceFloor(builder, context); + // DC-2a: the operator's OWN pre-declared delivery preference — tell the model WHY a delivery proposal it // authors may be overridden, so it stops re-proposing an already-vetoed contract turn after turn. Only // an OPERATOR declaration renders here (never the model's own prior proposal, which needs no explaining @@ -1047,6 +1051,62 @@ private static string ClosingMoveFor(SupervisorTurnContext context) return reach == SupervisorLandingReach.ReconcileFirst && withheld == SupervisorActionMask.UnacceptedReconciliation ? ClosingReconcileBeforeLanding : ClosingAlreadyIntegrated; } + /// + /// The operator-floor block's header — a stable prompt landmark the tests key on, like + /// . The timeout and the outcome word are the stop path's own + /// constants, so the copy cannot drift from what the stop does. + /// + /// "Every branch a stop ships", because a stop that published no branch has nothing to grade and records the + /// floor as not graded rather than passed. It ends on "before declaring success", not "before stopping": the model + /// can never observe the floor's verdict before it stops, and an honest exit (gave_up, ask_human) + /// stays open on a run that cannot pass — the instruction is how to reach a SUCCESS stop, never a reason to keep + /// working forever. + /// + /// A static readonly rather than a const only because C# cannot fold an int constant into a + /// constant string — the alternative is retyping the timeout here, which is exactly the drift this avoids. + /// + internal static readonly string OperatorFloorHeader = $"Operator acceptance floor (the server runs this argv on every branch a stop ships, {SupervisorLane.AcceptanceGradeTimeoutSeconds} s timeout each; if it fails, those branches are withheld and the run ends as {SupervisorOutcome.AcceptanceFailedOutcome} — a stop is final, so no turn is left to fix it; drive the work until it passes before declaring success):"; + + /// + /// The operator-floor block's last line. The STOP payload's own acceptance is a second gate, graded beside the + /// floor and never in place of it, so repeating the floor there only runs it twice on every branch, each in its own + /// clone and each up to the timeout. It names the stop payload because a subtask's (or a phase's, or an amendment's) + /// acceptance is a different field — and a per-unit check that runs the same command is the ONE way the model + /// can see a verdict before it stops, so that use is welcome. + /// + internal const string DoNotRepeatTheFloorAsTheStopAcceptance = "Do not repeat it as the stop payload's acceptance: the stop runs it regardless, so a copy only runs it twice on every branch. A subtask's own acceptance is a separate per-unit check."; + + /// + /// The operator's acceptance FLOOR, recited: the argv ApplyStopAcceptanceGradeAsync runs on every branch a + /// stop ships. A failing floor withholds those branches and ends the run + /// , and a stop is final, so no turn is left to fix it — yet the + /// decider rendered only the free-text criteria and the system prompt names "the operator's floor" without showing + /// it, so the brain drove the work blind to the one check that decides the run. + /// + /// The argv is written as a JSON array of strings, never as shell text. The grader spawns it with NO shell + /// (TestsPassGrader hands the first element to the runner as the program and the rest as its arguments), so a + /// shell-looking line misleads: --filter Category=Unit|Category=Smoke reads as a pipe, and a joined line cannot + /// tell ["sh", " ", "check.sh"] from two arguments. The array keeps every element boundary and every + /// character — WorkflowJson.InterpolatedText, the repo's own way to write an array into prompt text, leaves + /// the quote, pipe, ampersand, angle brackets and plus as themselves — and it escapes a newline, which + /// would otherwise flatten into a space. + /// + /// The line is bounded like the per-subtask check line: the argv is unbounded operator config and this block is + /// a fixed per-turn cost the tape compaction cannot shrink, so a longer one is cut and marked with the class's + /// ellipsis. Null / empty ⇒ no block ⇒ byte-identical prompt. + /// + private static void AppendOperatorAcceptanceFloor(StringBuilder builder, SupervisorTurnContext context) + { + const int maxChars = 400; + + if (context.AcceptanceChecks is not { Count: > 0 } floor) return; + + builder.AppendLine(OperatorFloorHeader); + builder.AppendLine($" {BoundOneLine(JsonSerializer.Serialize(floor, Workflows.WorkflowJson.InterpolatedText), maxChars)}"); + builder.AppendLine(DoNotRepeatTheFloorAsTheStopAcceptance); + builder.AppendLine(); + } + /// /// Render the plan's dependency FRONTIER (loopability — the server enforces DependsOn ordering at spawn): the /// subtasks READY to spawn now (every dependency accepted) and those still BLOCKED on a dependency, so the model diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/Supervisor/SupervisorGoldenPromptFidelityTests.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/Supervisor/SupervisorGoldenPromptFidelityTests.cs index 5ca3f56dc..e88a045b1 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Workflows/Supervisor/SupervisorGoldenPromptFidelityTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/Supervisor/SupervisorGoldenPromptFidelityTests.cs @@ -631,7 +631,38 @@ public void Only_a_scenario_missing_a_required_stage_renders_a_different_prompt_ /// it) by the re-pin receipt above — a digest whose predecessor is deleted can only ever be compared with itself. /// /// - /// THIS RE-PIN: merge is withheld from a tape whose newest staged work is a reconciliation the tape + /// THIS RE-PIN: the prompt gained the OPERATOR ACCEPTANCE FLOOR block — the argv the server runs on every branch a + /// stop ships () — rendered right after the acceptance-criteria + /// block by LlmSupervisorDecider.AppendOperatorAcceptanceFloor, and ONLY when the context carries one. Until + /// now the brain drove every run blind to the one check that decides it: a failing floor withholds those branches and + /// ends the run AcceptanceFailed, and a stop is final, so no turn is left to fix it — yet the decider rendered + /// only the free-text criteria, and the system prompt said "the operator's floor" without ever showing it. The block + /// states what runs (the argv, written as a JSON array because the grader spawns it with no shell), the timeout, the + /// consequence, and that repeating the floor as the stop payload's acceptance only runs it twice. It says + /// "before declaring success" rather than "before stopping", so it never contradicts the honest exits + /// (gave_up, ask_human) the system prompt and the closing move still offer. + /// + /// The moved bytes are attributed per tape, not claimed: the block renders on exactly + /// — the one scenario whose context declares a floor, + /// repeat-failure-under-a-declared-check (["dotnet", "test"]) — and nothing else moves. + /// derives that rather than + /// restating it: it renders every scenario with and without its floor and requires the movers to be exactly that + /// set; it requires the carrier's prompt, minus the block's own lines (taken from the decider's constants, never + /// retyped), to equal its floor-withheld prompt, so the block is the ONLY thing that moved; and it requires the + /// corpus rendered with every floor withheld to digest to — the pin this + /// constant held before the block existed. So the other 28 scenarios are byte-identical and their scores stay + /// comparable across the change. The three older anchors need no edit: each is recomputed over a subset that + /// already excludes this scenario (, ), + /// so none of them was ever measured over it. + /// + /// What did NOT move, on purpose: the SYSTEM prompt (its digest gates paid qualification through + /// QualificationRuntimeGate), the stopped-now recital (precomposed from the durable tape, and mirrored by + /// this corpus's own RenderStoppedNowRecital), and SupervisorQualityFacts — the floor is a run-level + /// gate, not a unit-level declared check, which + /// + /// still pins. + /// + /// PREVIOUS RE-PIN: merge is withheld from a tape whose newest staged work is a reconciliation the tape /// records as NOT verified, and the two blocks that still named the verb unconditionally now defer to that mask — /// the CURRENT PLAN STATE block's finished-plan line () and, /// for the budget-remaining half of the pair, the closing move @@ -745,7 +776,17 @@ public void Only_a_scenario_missing_a_required_stage_renders_a_different_prompt_ /// (merge, run 34085079257 at 24/25). /// pins which rosters offer it — the set was EMPTY across all 25 before that change. /// - private const string GoldenPromptDigest = "2dafcdc7e52c3d22d1d2d209bb05bbe701191131defeb9e11f26360ef70bf5fb"; + private const string GoldenPromptDigest = "6b9c621bf49c7e5858a1289f62581319c3974d8f86cab3272c54d1574084c893"; + + /// + /// The pin this corpus carried while the operator's acceptance floor never reached the prompt: the decider read + /// the free-text criteria and nothing else, so a floor on a context changed not one byte of the rendering. + /// Superseded, never deleted — it is the fixed point + /// measures today's rendering + /// against: the corpus with every floor withheld must still digest to it over all 29 scenarios, so the move is + /// the floor block and nothing else. A digest whose predecessor is deleted can only ever be compared with itself. + /// + private const string PreOperatorFloorCorpusDigest = "2dafcdc7e52c3d22d1d2d209bb05bbe701191131defeb9e11f26360ef70bf5fb"; /// /// The pin this corpus carried while the VERB ROSTER was a static sentence in the turn-invariant system prompt — @@ -1039,6 +1080,62 @@ public void A_pre_spawn_tape_is_byte_identical_because_it_has_nothing_to_recomme } } + /// + /// The scenarios whose context carries an operator acceptance floor — the named receipt for the current + /// , exactly like and + /// are for theirs. A ONE-element set because the floor is the run-level argv the + /// stop path runs, and this corpus declares it in one place: repeat-failure-under-a-declared-check, whose + /// ["dotnet", "test"] is also the argv s1's own per-unit oracle declares (the fixture reuses one argv for + /// both grains, so the floor block and the plan-state check line name the same command there). + /// + private static readonly HashSet CarriesAnOperatorFloor = new(StringComparer.Ordinal) + { + "repeat-failure-under-a-declared-check", + }; + + /// + /// The named receipt for 's move: the OPERATOR ACCEPTANCE FLOOR block renders on + /// the tape that carries a floor and on no other, and it is the ONLY thing that moved. Derived, not claimed — + /// every scenario is rendered as carried and with its floor withheld, and the set that differs must be exactly the + /// named one; the carrier's prompt minus the block's own lines must equal its floor-withheld prompt; and the + /// wind-back anchors the "before" half: the corpus with every floor withheld must digest to + /// , the pin held before the block existed. Without that anchor the + /// receipt would compare today's code with itself, and a corpus that drifted for an unrelated reason would still + /// report a clean, attributable re-pin. + /// + /// Presence, not wording: the block's header and closing note are taken FROM the decider's constants and + /// never retyped (this file pins arms and presence, and the copy is the decider's own unit tests' job); the only + /// line spelled out here is the argv, which is the fixture's own floor. + /// + [Fact] + public void Only_the_tape_that_carries_an_operator_floor_gains_the_floor_block() + { + SupervisorDecisionGoldenScenarios.All.Where(s => s.Context.AcceptanceChecks is { Count: > 0 }).Select(s => s.Name).ShouldBe(CarriesAnOperatorFloor.ToList(), ignoreOrder: true, + "the set of scenarios whose context declares an operator floor must match the named receipt beside the digest — a floor added to another tape moves that tape's prompt, so it must be re-pinned and attributed"); + + var moved = SupervisorDecisionGoldenScenarios.All.Where(s => LlmSupervisorDecider.BuildUserPromptForTest(s.Context) != FloorWithheld(s)).Select(s => s.Name).ToList(); + + moved.ShouldBe(CarriesAnOperatorFloor.ToList(), ignoreOrder: true, + "the set of scenarios whose prompt moved must match the named receipt — a tape whose floor the block did not render, or a tape that gained the block without one, is a re-pin nobody attributed"); + + Digest(RenderedCorpus(FloorWithheld)).ShouldBe(PreOperatorFloorCorpusDigest, + "with every floor withheld the corpus must digest to the pin it carried before the block existed — anything else drifted into the same commit, so the move is then not the floor block alone"); + + var carrier = SupervisorDecisionGoldenScenarios.All.Single(s => s.Name == "repeat-failure-under-a-declared-check"); + var withFloor = LlmSupervisorDecider.BuildUserPromptForTest(carrier.Context); + var block = string.Join(Environment.NewLine, LlmSupervisorDecider.OperatorFloorHeader, """ ["dotnet","test"]""", LlmSupervisorDecider.DoNotRepeatTheFloorAsTheStopAcceptance) + Environment.NewLine + Environment.NewLine; + + FloorWithheld(carrier).ShouldNotContain("""["dotnet","test"]""", Case.Sensitive, + "fixture check: that argv line must come from the floor block alone — if the tape already shows it elsewhere, the presence assertion below proves nothing"); + withFloor.ShouldContain(block, Case.Sensitive, + "the tape that carries the floor shows the model that floor's block — present, and not merely some other byte moved"); + withFloor.Replace(block, string.Empty, StringComparison.Ordinal).ShouldBe(FloorWithheld(carrier), + "the carrier's prompt minus the block's own lines must be exactly its floor-withheld prompt — so the block is the ONLY thing that moved"); + } + + /// One scenario's prompt with its operator floor withheld — what every prompt read before the floor reached the brain, since the decider rendered the free-text criteria and never this field. + private static string FloorWithheld(SupervisorGoldenScenario scenario) => LlmSupervisorDecider.BuildUserPromptForTest(scenario.Context with { AcceptanceChecks = null }); + private static string PromptFor(string name) => LlmSupervisorDecider.BuildUserPromptForTest(SupervisorDecisionGoldenScenarios.All.Single(s => s.Name == name).Context); diff --git a/backend/tests/CodeSpace.UnitTests/Agents/SupervisorDeciderTests.cs b/backend/tests/CodeSpace.UnitTests/Agents/SupervisorDeciderTests.cs index cd7bde5d7..38381f27f 100644 --- a/backend/tests/CodeSpace.UnitTests/Agents/SupervisorDeciderTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Agents/SupervisorDeciderTests.cs @@ -111,6 +111,74 @@ public void The_prompt_never_fabricates_a_decline_when_the_operator_only_pinned_ prompt.ShouldContain("release", customMessage: "the operator's branch pin should still reach the model somehow"); } + // ── The operator's acceptance FLOOR is told to the model: the server runs it on every branch a stop ships, so the work must be driven to IT ── + + [Fact] + public void The_prompt_shows_the_model_the_operator_floor_its_stop_will_be_graded_by() + { + var prompt = LlmSupervisorDecider.BuildUserPromptForTest(Context() with { AcceptanceChecks = new[] { "dotnet", "test", "--filter", "Category=Unit|Category=Smoke" } }); + + var block = string.Join(Environment.NewLine, LlmSupervisorDecider.OperatorFloorHeader, """ ["dotnet","test","--filter","Category=Unit|Category=Smoke"]""", LlmSupervisorDecider.DoNotRepeatTheFloorAsTheStopAcceptance) + Environment.NewLine + Environment.NewLine; + + prompt.ShouldContain(block, Case.Sensitive, customMessage: "the whole block: the header, the argv as a JSON array with no shell prompt in front of it (the grader never runs it through a shell), the closing note, then a blank line"); + } + + [Fact] + public void The_floor_header_reads_its_timeout_and_outcome_word_from_the_stop_paths_own_constants() + { + LlmSupervisorDecider.OperatorFloorHeader.ShouldContain($"{SupervisorLane.AcceptanceGradeTimeoutSeconds} s timeout each", Case.Sensitive, customMessage: "the timeout the model reads is the grader's own constant, so the copy cannot drift from what actually kills the check"); + LlmSupervisorDecider.OperatorFloorHeader.ShouldContain($"the run ends as {SupervisorOutcome.AcceptanceFailedOutcome}", Case.Sensitive, customMessage: "a failing floor's consequence is stated in the durable outcome's own word"); + } + + [Fact] + public void The_floor_line_keeps_every_argv_element_boundary_the_grader_will_run() + { + // A floor production CAN emit: the rehydrate keeps blank elements after the executable + // (SupervisorTurnServiceTests.Rehydrate_and_the_stop_grader_preserve_exact_operator_argv), and + // TaskLaunchFlowTests pins ["sh", " ", "check.sh"] surviving launch — so this is not a fixture the real system never holds. + var prompt = LlmSupervisorDecider.BuildUserPromptForTest(Context() with { AcceptanceChecks = new[] { "sh", " ", "check.sh" } }); + + prompt.ShouldContain(""" ["sh"," ","check.sh"]""", Case.Sensitive, customMessage: "the whitespace element is ITS OWN argument — the grader runs it as one"); + prompt.ShouldNotContain("sh check.sh", customMessage: "a plain join reads that argv as two arguments, so the model would be told to satisfy a command that is not the one graded"); + } + + [Theory] + [InlineData(new[] { "dotnet", "test", "--filter", "Category=Unit|Category=Smoke" }, """["dotnet","test","--filter","Category=Unit|Category=Smoke"]""")] // a pipe inside an argument is a character, not a pipeline + [InlineData(new[] { "echo", "it's & +1" }, """["echo","it's & +1"]""")] // what the default encoder would turn into \u00XX codes + [InlineData(new[] { "sh", "-c", "a\nb" }, """["sh","-c","a\nb"]""")] // a newline stays a visible escape… + [InlineData(new[] { "sh", "-c", "a b" }, """["sh","-c","a b"]""")] // …so it never reads as the space a flattening would make of it + [InlineData(new[] { "custom-check", "", " ", "quoted argument" }, """["custom-check",""," ","quoted argument"]""")] // what the rehydrate keeps: empty, blank and space-bearing elements + public void The_floor_line_is_the_argv_as_a_json_array_with_nothing_a_reader_needs_lost(string[] argv, string expected) + { + var prompt = LlmSupervisorDecider.BuildUserPromptForTest(Context() with { AcceptanceChecks = argv }); + + prompt.ShouldContain($" {expected}{Environment.NewLine}", Case.Sensitive, customMessage: "the grader spawns this argv with NO shell, so the line is the array itself: every element boundary and every character a reader needs, nothing escaped that JSON does not require"); + } + + [Theory] + [InlineData(400, false)] // exactly at the bound — verbatim, no ellipsis + [InlineData(401, true)] // one over — cut at 400 and marked with the class's ellipsis + public void The_floor_line_is_bounded_at_400_chars_like_the_per_subtask_check_line(int lineLength, bool bounded) + { + // A one-element argv is four characters of JSON punctuation (["…"]) around its text, so the line is exactly that long. + var element = new string('x', lineLength - 4); + var json = $"[\"{element}\"]"; + + var prompt = LlmSupervisorDecider.BuildUserPromptForTest(Context() with { AcceptanceChecks = new[] { element } }); + + var shown = bounded ? json[..400] + "…" : json; + + prompt.ShouldContain($" {shown}{Environment.NewLine}", Case.Sensitive, customMessage: "the argv is unbounded operator config and this line is a fixed per-turn cost the tape compaction cannot shrink"); + } + + [Fact] + public void The_floor_block_follows_the_acceptance_criteria_it_is_the_executable_half_of() + { + var prompt = LlmSupervisorDecider.BuildUserPromptForTest(Context() with { AcceptanceCriteria = new[] { "no regressions" }, AcceptanceChecks = new[] { "dotnet", "test" } }); + + prompt.ShouldContain($"- no regressions{Environment.NewLine}{Environment.NewLine}{LlmSupervisorDecider.OperatorFloorHeader}", Case.Sensitive, customMessage: "the criteria say what done means and the floor is the command that grades it — one after the other, nothing between them"); + } + // ── P1e compaction ladder: a re-planned run renders only the LATEST plan full; superseded plans collapse to a digest ── [Fact] diff --git a/backend/tests/CodeSpace.UnitTests/Agents/SupervisorTurnServiceTests.cs b/backend/tests/CodeSpace.UnitTests/Agents/SupervisorTurnServiceTests.cs index 3cb24eb7e..46c00afcc 100644 --- a/backend/tests/CodeSpace.UnitTests/Agents/SupervisorTurnServiceTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Agents/SupervisorTurnServiceTests.cs @@ -45,6 +45,41 @@ public async Task Rehydrate_and_the_stop_grader_preserve_exact_operator_argv() grader.LastCall!.Value.Command.ShouldBe(argv); } + [Fact] + public async Task The_decider_prompt_shows_the_rehydrated_operator_floor_with_its_argv_boundaries() + { + // Through the real writer: the operator's configured argv, as the rehydrate hands it to the decider — not a context built by hand. + var config = GoalConfigWithRepo() with { AcceptanceChecks = new[] { "custom-check", "", " ", "quoted argument" } }; + + var context = await Service(new FakeSupervisorDecisionLog()).RehydrateFromDecisionLogAsync(_runId, _teamId, "sup", "goal", config, CancellationToken.None); + + var prompt = LlmSupervisorDecider.BuildUserPromptForTest(context); + + prompt.ShouldContain(LlmSupervisorDecider.OperatorFloorHeader, Case.Sensitive, customMessage: "a rehydrated floor is recited at all"); + prompt.ShouldContain(""" ["custom-check",""," ","quoted argument"]""", Case.Sensitive, customMessage: "the floor the stop grader runs is the floor the brain reads — element for element, blank ones included"); + } + + public static TheoryData FloorsTheRehydrateRefuses => new() + { + null, // no floor configured — the ordinary run + Array.Empty(), // an empty list + new[] { "", "x" }, // a blank executable never promotes a later argument + new[] { " ", "x" }, + new[] { "x", "a\0b" }, // one NUL voids the whole argv, whichever element holds it + }; + + [Theory] + [MemberData(nameof(FloorsTheRehydrateRefuses))] + public async Task The_decider_prompt_has_no_floor_block_for_an_argv_the_rehydrate_refuses(string[]? argv) + { + var config = GoalConfigWithRepo() with { AcceptanceChecks = argv }; + + var context = await Service(new FakeSupervisorDecisionLog()).RehydrateFromDecisionLogAsync(_runId, _teamId, "sup", "goal", config, CancellationToken.None); + + context.AcceptanceChecks.ShouldBeNull("fixture check: the rehydrate hands the decider NO floor for this argv — were it kept, the prompt assertion below would be answering a different question"); + LlmSupervisorDecider.BuildUserPromptForTest(context).ShouldNotContain(LlmSupervisorDecider.OperatorFloorHeader, Case.Sensitive, customMessage: "a floor the stop path cannot run is a floor nothing is graded by, so there is nothing to recite"); + } + [Theory] [InlineData("")] [InlineData(" ")]