diff --git a/backend/src/CodeSpace.Core/Services/Agents/AcceptanceArgv.cs b/backend/src/CodeSpace.Core/Services/Agents/AcceptanceArgv.cs new file mode 100644 index 000000000..8c33a7433 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Agents/AcceptanceArgv.cs @@ -0,0 +1,52 @@ +using System.Text.Json; + +namespace CodeSpace.Core.Services.Agents; + +/// +/// The ONE place the shape of an acceptance argv is decided: which ones can be run at all +/// () and how the supervisor surfaces below show one (). Every +/// acceptance command — the operator's floor (SupervisorGoalConfig.AcceptanceChecks), a supervisor unit's or a +/// stop payload's check, a task's own oracle — is spawned with NO shell: the first element is the program and the rest +/// are its arguments. So every reader that decides whether such a command exists (the launch projections that bake it, +/// the rehydrate and the stop grade that run it, the in-loop hook and the local verifier that wrap it) must agree on +/// what an unrunnable one is, or one of them promises a command another refused to read. +/// +public static class AcceptanceArgv +{ + /// + /// The argv as its own copy when it can be spawned, else null. Two things make one unrunnable, and only these: a + /// blank FIRST element — there is no program, and the argv is never repaired by promoting a later element into that + /// slot, which would run a command nobody wrote — and a null element or a NUL anywhere, which cannot be handed to + /// exec. Every other element is kept verbatim, blank ones after the first included: ["sh", " ", "check.sh"] + /// is three arguments, and the element boundaries are the command. + /// + public static IReadOnlyList? NormalizeOrNull(IReadOnlyList? argv) => argv is { Count: > 0 } && !string.IsNullOrWhiteSpace(argv[0]) && argv.All(token => token is not null && !token.Contains('\0')) ? argv.ToArray() : null; + + /// + /// A check's command as the three supervisor surfaces below show it — a model reading a prompt, a human reading an + /// approval card: a JSON array of strings on ONE line, cut at and marked with an + /// ellipsis, never shell text. For a TestsPass check the array is an argv, and the grader spawns every acceptance argv + /// 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, or + /// [" ", "dotnet", "test"] from dotnet test. 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, so the line is one + /// line whatever the argv holds. For the other kinds (ArtifactPresent, LlmJudge, + /// ArtifactSchema, CitationsResolve) the same list holds repo paths, not an argv, and the array keeps + /// their boundaries just as well. + /// + /// Exactly three call sites come through here: the supervisor prompt's operator-floor block, each per-subtask + /// check line, and the amendment card's proposed check, so those three never write one command two ways. It is NOT + /// the only way a command reaches a reader. Known exceptions: the grader's own $ cmd evidence line + /// (TestsPassGrader), which is a fenced record of what ran and which a failed unit's evidence tail puts into + /// the prompt beside the array line, and the two UI labels, RoomNarrative's acceptance label and + /// SupervisorPhaseSource's phase description, which still join with spaces. + /// + public static string AsLine(IReadOnlyList argv, int maxChars) + { + var json = JsonSerializer.Serialize(argv, Workflows.WorkflowJson.InterpolatedText); + + return json.Length <= maxChars ? json : json[..maxChars] + "…"; + } +} diff --git a/backend/src/CodeSpace.Core/Services/Agents/InLoopAcceptanceHook.cs b/backend/src/CodeSpace.Core/Services/Agents/InLoopAcceptanceHook.cs index 69158c3f9..2003c7b5e 100644 --- a/backend/src/CodeSpace.Core/Services/Agents/InLoopAcceptanceHook.cs +++ b/backend/src/CodeSpace.Core/Services/Agents/InLoopAcceptanceHook.cs @@ -62,8 +62,7 @@ public static class InLoopAcceptanceHook public static bool AppliesTo(AgentTask task) => task.Permissions.WriteScope == AgentWriteScope.Workspace && HasRunnableOracle(task); /// Only a well-formed argv oracle can run inside a shell hook. An authored but incomplete contract still requires final grading; file obligations are never interpreted as commands. The shape alone, whatever the write scope. - public static bool HasRunnableOracle(AgentTask task) => task.Acceptance is { Kind: null or Messages.Agents.Benchmark.BenchmarkGradingKind.TestsPass, Command.Count: > 0 } spec - && !string.IsNullOrWhiteSpace(spec.Command[0]) && spec.Command.All(value => value != null && !value.Contains('\0')); + public static bool HasRunnableOracle(AgentTask task) => task.Acceptance is { Kind: null or Messages.Agents.Benchmark.BenchmarkGradingKind.TestsPass } spec && AcceptanceArgv.NormalizeOrNull(spec.Command) is not null; /// /// The POSIX-sh Stop-hook script content, harness-PARAMETERIZED. Both Claude Code's settings.json diff --git a/backend/src/CodeSpace.Core/Services/Agents/Workspace/LocalAcceptanceVerifier.cs b/backend/src/CodeSpace.Core/Services/Agents/Workspace/LocalAcceptanceVerifier.cs index dee303a58..ff962531e 100644 --- a/backend/src/CodeSpace.Core/Services/Agents/Workspace/LocalAcceptanceVerifier.cs +++ b/backend/src/CodeSpace.Core/Services/Agents/Workspace/LocalAcceptanceVerifier.cs @@ -145,7 +145,7 @@ public async Task ObserveAsync(LocalAcceptanceReques return null; } - private static bool ValidArgv(IReadOnlyList argv) => argv.Count > 0 && !string.IsNullOrWhiteSpace(argv[0]) && argv.All(value => value != null && !value.Contains('\0')); + private static bool ValidArgv(IReadOnlyList argv) => AcceptanceArgv.NormalizeOrNull(argv) is not null; private static bool SamePath(string left, string? right) { try { return right != null && Path.GetFullPath(left) == Path.GetFullPath(right); } diff --git a/backend/src/CodeSpace.Core/Services/Supervisor/Deciders/LlmSupervisorDecider.cs b/backend/src/CodeSpace.Core/Services/Supervisor/Deciders/LlmSupervisorDecider.cs index 810b74f38..35630cdc2 100644 --- a/backend/src/CodeSpace.Core/Services/Supervisor/Deciders/LlmSupervisorDecider.cs +++ b/backend/src/CodeSpace.Core/Services/Supervisor/Deciders/LlmSupervisorDecider.cs @@ -1083,17 +1083,11 @@ private static string ClosingMoveFor(SupervisorTurnContext context) /// 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. + /// The argv is written through — the same renderer the per-subtask check + /// line and the amendment card use (the grader's own fenced evidence line is a separate record of what ran) — and + /// 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 an ellipsis. Null / empty ⇒ no block ⇒ byte-identical prompt. /// private static void AppendOperatorAcceptanceFloor(StringBuilder builder, SupervisorTurnContext context) { @@ -1102,7 +1096,7 @@ private static void AppendOperatorAcceptanceFloor(StringBuilder builder, Supervi if (context.AcceptanceChecks is not { Count: > 0 } floor) return; builder.AppendLine(OperatorFloorHeader); - builder.AppendLine($" {BoundOneLine(JsonSerializer.Serialize(floor, Workflows.WorkflowJson.InterpolatedText), maxChars)}"); + builder.AppendLine($" {AcceptanceArgv.AsLine(floor, maxChars)}"); builder.AppendLine(DoNotRepeatTheFloorAsTheStopAcceptance); builder.AppendLine(); } @@ -1527,7 +1521,7 @@ private static bool AppendPlanDecision(StringBuilder builder, SupervisorPriorDec if (effective.WaivedSubtaskIds.Contains(subtask.Id)) builder.AppendLine(" acceptance check: WAIVED by a co-signed amendment — this item carries no verification. WAIVED is not PASSED; do not report it as verified."); else if (effective.BySubtask.GetValueOrDefault(subtask.Id) is { } acceptance) - builder.AppendLine($" acceptance check ({acceptance.Kind?.ToString() ?? "TestsPass"}){(ReferenceEquals(acceptance, subtask.Acceptance) ? "" : ", AMENDED by a co-signed amendment")}: {BoundOneLine(string.Join(" ", acceptance.Command), maxChars)}"); + builder.AppendLine($" acceptance check ({acceptance.Kind?.ToString() ?? "TestsPass"}){(ReferenceEquals(acceptance, subtask.Acceptance) ? "" : ", AMENDED by a co-signed amendment")}: {AcceptanceArgv.AsLine(acceptance.Command, maxChars)}"); } return true; diff --git a/backend/src/CodeSpace.Core/Services/Supervisor/SupervisorAmendAcceptance.cs b/backend/src/CodeSpace.Core/Services/Supervisor/SupervisorAmendAcceptance.cs index cb992ed71..5fb1cbebc 100644 --- a/backend/src/CodeSpace.Core/Services/Supervisor/SupervisorAmendAcceptance.cs +++ b/backend/src/CodeSpace.Core/Services/Supervisor/SupervisorAmendAcceptance.cs @@ -31,10 +31,10 @@ public static class SupervisorAmendAcceptance /// The marker phrase EVERY amend card carries — the stable tail detectors match to recognise this card (vs the other three markers / a content ask). Pinned by a unit test so a reword is a visible decision. public const string AmendMarker = "Reply 'approve' to apply this acceptance amendment, or describe what to do instead."; - /// Per-fragment cap on the quoted reason — the card is a headline, not the failure dossier (the failing grade's evidence lives on the tape). + /// Per-fragment cap on the quoted reason and on the proposed check's argv line — the card is a headline, not the failure dossier (the failing grade's evidence lives on the tape). internal const int MaxQuotedChars = 200; - /// Rewrite the model's amend proposal into the parked ask card: the question names the target subtask, the waive-vs-amend shape, the reason, and the proposed command; the structured payload rides beside it as the root amend node; the decision-level rationale is appended last, mirroring the projector's own rationale merge (deterministic order → replay-stable idempotency key). + /// Rewrite the model's amend proposal into the parked ask card: the question names the target subtask, the waive-vs-amend shape, the reason, and the proposed command — as the JSON array (; for a TestsPass check the argv the grader would spawn, for a file-oracle kind the paths it reads), never a shell line the human co-signer would read differently from what runs; the structured payload rides beside it as the root amend node; the decision-level rationale is appended last, mirroring the projector's own rationale merge (deterministic order → replay-stable idempotency key). public static SupervisorDecision IntoAskHuman(SupervisorAmendAcceptancePayload payload, SupervisorRationale? rationale = null) { var ask = JsonSerializer.Serialize(new SupervisorAskHumanPayload { Question = QuestionFor(payload) }, AgentJson.Options); @@ -117,7 +117,7 @@ private static string QuestionFor(SupervisorAmendAcceptancePayload payload) question.Append($"\nReason: {Quote(payload.Reason)}"); if (!payload.Waive && payload.Acceptance is not null) - question.Append($"\nProposed check: {Quote(string.Join(" ", payload.Acceptance.Command))}"); + question.Append($"\nProposed check: {AcceptanceArgv.AsLine(payload.Acceptance.Command, MaxQuotedChars)}"); question.Append($"\n{AmendMarker}"); diff --git a/backend/src/CodeSpace.Core/Services/Supervisor/SupervisorTurnService.Rehydrate.cs b/backend/src/CodeSpace.Core/Services/Supervisor/SupervisorTurnService.Rehydrate.cs index 655ef1c36..1cb242990 100644 --- a/backend/src/CodeSpace.Core/Services/Supervisor/SupervisorTurnService.Rehydrate.cs +++ b/backend/src/CodeSpace.Core/Services/Supervisor/SupervisorTurnService.Rehydrate.cs @@ -93,8 +93,9 @@ public async Task RehydrateFromDecisionLogAsync(Guid supe : EmptyGuidSet; // The operator's OBJECTIVE acceptance command (L4 A3) — the argv a resolve verdict is graded against. Null - // (none / all-blank) → no objective grade runs, the resolver self-report marker stands (byte-identical to pre-A3). - var acceptanceCommand = NormalizeCommand(goalConfig?.AcceptanceChecks); + // (none, or an argv with no program or a NUL) → no objective grade runs, the resolver self-report marker stands + // (byte-identical to pre-A3). + var acceptanceCommand = AcceptanceArgv.NormalizeOrNull(goalConfig?.AcceptanceChecks); // C3 narrowing: the run's own ORACLE INVENTORY — the floor's program file(s), derived from the operator's // argv through the SAME extraction every protected grade uses. It is what separates a judge the run owns @@ -803,11 +804,11 @@ private async Task FoldUnitAcceptanceGradeAsync(Supervi } var spec = subtaskId is not null && acceptanceBySubtask.TryGetValue(subtaskId, out var found) ? found : null; - var command = spec is not null ? NormalizeCommand(spec.Command) : null; + var command = spec is not null ? AcceptanceArgv.NormalizeOrNull(spec.Command) : null; - // Ungraded — keep the result byte-identical — when the unit has no contract or its command is all-blank - // (the dominant no-contract case). A MULTI-repo unit (RepositoryResults present) is graded PER REPO below, - // mirroring the run-level stop floor's multi-target grade — it is no longer silently skipped. + // Ungraded — keep the result byte-identical — when the unit has no contract or its command has no program + // or carries a NUL (the dominant no-contract case). A MULTI-repo unit (RepositoryResults present) is graded + // PER REPO below, mirroring the run-level stop floor's multi-target grade — it is no longer silently skipped. if (command is null) { graded.Add(results[i]); @@ -1363,8 +1364,8 @@ internal async Task ApplyStopAcceptanceGradeAsync(Superviso // Each gate carries its FULL spec (triad S7 — a model-check's rubric/schema payload rides the spec); the // operator floor stays a bare TestsPass argv spec (the operator never authors a kind on this path). - var floorCommand = NormalizeCommand(context.AcceptanceChecks); - var modelCommand = NormalizeCommand(modelAcceptance?.Command); + var floorCommand = AcceptanceArgv.NormalizeOrNull(context.AcceptanceChecks); + var modelCommand = AcceptanceArgv.NormalizeOrNull(modelAcceptance?.Command); var gates = new (string Label, SupervisorAcceptanceSpec? Spec)[] { @@ -1864,9 +1865,6 @@ private sealed record ResolveContributorRow(Guid Id, Guid TeamId, AgentRunStatus return ids is { Count: > 0 } ? ids : null; } - /// Preserve argv boundaries exactly. A blank executable or invalid token never promotes a later argument into the executable. - private static IReadOnlyList? NormalizeCommand(IReadOnlyList? command) => command is { Count: > 0 } && !string.IsNullOrWhiteSpace(command[0]) && command.All(a => a is not null && !a.Contains('\0')) ? command.ToArray() : null; - /// /// How many WorkflowRun ancestors this supervisor run already has (PR-E E5 depth cap) — walks the /// parent_run_id chain exactly as SubworkflowService.EnsureWithinDepthAsync does, bounded by diff --git a/backend/src/CodeSpace.Core/Services/Tasks/Projection/Builders/SingleAgent/SingleAgentDefinitionBuilder.cs b/backend/src/CodeSpace.Core/Services/Tasks/Projection/Builders/SingleAgent/SingleAgentDefinitionBuilder.cs index 7f2db0b48..f047fec74 100644 --- a/backend/src/CodeSpace.Core/Services/Tasks/Projection/Builders/SingleAgent/SingleAgentDefinitionBuilder.cs +++ b/backend/src/CodeSpace.Core/Services/Tasks/Projection/Builders/SingleAgent/SingleAgentDefinitionBuilder.cs @@ -1,5 +1,6 @@ using System.Text.Json; using CodeSpace.Core.DependencyInjection; +using CodeSpace.Core.Services.Agents; using CodeSpace.Messages.Dtos.Workflows; using CodeSpace.Messages.Tasks; using CodeSpace.Messages.Tasks.Effort; @@ -31,9 +32,8 @@ public sealed class SingleAgentDefinitionBuilder : IWorkflowDefinitionBuilder, I /// The route's deliverable shape, normalized — an unknown / absent shape reads as code, today's behaviour. private static string Shape(TaskBuildContext context) => DeliverableShapes.Normalize(context.Route.DeliverableShape); - /// The operator's EXECUTABLE floor, blanks dropped. Non-empty ⇒ the operator authored the oracle and it wins over any shape-derived one. - private static IReadOnlyList? OperatorArgv(TaskBuildContext context) => - context.AcceptanceChecks is { Count: > 0 } command && !string.IsNullOrWhiteSpace(command[0]) && command.All(c => c is not null && !c.Contains('\0')) ? command.ToArray() : null; + /// The operator's EXECUTABLE floor, verbatim — blank arguments after the program are arguments. Non-null ⇒ the operator authored a runnable oracle and it wins over any shape-derived one. + private static IReadOnlyList? OperatorArgv(TaskBuildContext context) => AcceptanceArgv.NormalizeOrNull(context.AcceptanceChecks); /// /// THIS agent's objective oracle. Precedence, and the whole point of the shape axis: diff --git a/backend/src/CodeSpace.Core/Services/Tasks/Projection/Builders/Supervisor/SupervisorDefinitionBuilder.cs b/backend/src/CodeSpace.Core/Services/Tasks/Projection/Builders/Supervisor/SupervisorDefinitionBuilder.cs index 62542d24d..12f516641 100644 --- a/backend/src/CodeSpace.Core/Services/Tasks/Projection/Builders/Supervisor/SupervisorDefinitionBuilder.cs +++ b/backend/src/CodeSpace.Core/Services/Tasks/Projection/Builders/Supervisor/SupervisorDefinitionBuilder.cs @@ -1,5 +1,6 @@ using System.Text.Json; using CodeSpace.Core.DependencyInjection; +using CodeSpace.Core.Services.Agents; using CodeSpace.Core.Services.Agents.Workspace; using CodeSpace.Messages.Agents; using CodeSpace.Messages.Dtos.Workflows; @@ -136,9 +137,10 @@ private static JsonElement BuildSupervisorConfig(TaskBuildContext context) // model targets them; NOT executed, distinct from the acceptanceChecks argv floor). Omitted when empty (byte-identical). AddIfPresent(config, "acceptanceCriteria", context.AcceptanceCriteria is { Count: > 0 } criteria ? criteria.ToList() : null); // The operator's EXECUTABLE acceptance floor (S4b) — an argv run against the reviewable head at the terminal - // stop; a non-zero exit fails the stop + withholds the branch. Blank entries dropped; omitted when empty - // (byte-identical). The free-text criteria above steer the model; this floor VERIFIES the result. - AddIfPresent(config, "acceptanceChecks", context.AcceptanceChecks is { Count: > 0 } checks && !string.IsNullOrWhiteSpace(checks[0]) && checks.All(c => c is not null && !c.Contains('\0')) ? checks.ToArray() : null); + // stop; a non-zero exit fails the stop + withholds the branch. Baked verbatim, blank arguments after the program + // included; omitted when empty or when it has no program / carries a NUL (byte-identical). The free-text criteria + // above steer the model; this floor VERIFIES the result. + AddIfPresent(config, "acceptanceChecks", AcceptanceArgv.NormalizeOrNull(context.AcceptanceChecks)); // The S3 plan-confirmation gate — every authored plan version parks for the operator's confirmation before any // agent is created. Omitted when off (byte-identical). AddIfPresent(config, "requirePlanConfirmation", context.RequirePlanConfirmation ? (object)true : null); diff --git a/backend/src/CodeSpace.Core/Services/Tasks/SpecPreview/TaskSpecAcceptanceAssessment.cs b/backend/src/CodeSpace.Core/Services/Tasks/SpecPreview/TaskSpecAcceptanceAssessment.cs index 436783f10..81b3d7076 100644 --- a/backend/src/CodeSpace.Core/Services/Tasks/SpecPreview/TaskSpecAcceptanceAssessment.cs +++ b/backend/src/CodeSpace.Core/Services/Tasks/SpecPreview/TaskSpecAcceptanceAssessment.cs @@ -1,3 +1,4 @@ +using CodeSpace.Core.Services.Agents; using CodeSpace.Messages.Contracts; using CodeSpace.Messages.Tasks; @@ -15,7 +16,7 @@ internal static class TaskSpecAcceptanceAssessment var reason = "Source support is unknown; this proposal is not prefilled as a mandatory check."; var status = TaskSpecEvidenceStatus.Unknown; var source = TaskSpecCheckSource.ProposedUnverified; - var validCommand = argv.Length is > 0 and <= 256 && !string.IsNullOrWhiteSpace(argv[0]) && argv.All(t => t is not null && !t.Contains('\0')); + var validCommand = argv.Length <= 256 && AcceptanceArgv.NormalizeOrNull(argv) is not null; var citations = review?.Citations ?? []; var validCitations = context is not null && citations.Count is > 0 and <= 8; foreach (var citation in citations.Take(8)) diff --git a/backend/src/CodeSpace.Core/Services/Tasks/SpecPreview/TaskSpecSourceArgv.cs b/backend/src/CodeSpace.Core/Services/Tasks/SpecPreview/TaskSpecSourceArgv.cs index 1a350b39d..30af1d7b2 100644 --- a/backend/src/CodeSpace.Core/Services/Tasks/SpecPreview/TaskSpecSourceArgv.cs +++ b/backend/src/CodeSpace.Core/Services/Tasks/SpecPreview/TaskSpecSourceArgv.cs @@ -1,4 +1,5 @@ using System.Text.Json; +using CodeSpace.Core.Services.Agents; namespace CodeSpace.Core.Services.Tasks.SpecPreview; @@ -15,7 +16,7 @@ public static TaskSpecArgvResolution Resolve(TaskSpecCompilation compilation, Ta try { var argv = JsonSerializer.Deserialize(reference.Quote); - if (argv is not { Length: > 0 and <= 256 } || string.IsNullOrWhiteSpace(argv[0]) || argv.Any(a => a is null || a.Contains('\0'))) return new([], null, error); + if (argv is not { Length: <= 256 } || AcceptanceArgv.NormalizeOrNull(argv) is null) return new([], null, error); return new(argv, matches[0], null); } catch (JsonException) { return new([], null, error); } diff --git a/backend/src/CodeSpace.Messages/Agents/SupervisorTurnContext.cs b/backend/src/CodeSpace.Messages/Agents/SupervisorTurnContext.cs index c5f6b1073..b3a4392fd 100644 --- a/backend/src/CodeSpace.Messages/Agents/SupervisorTurnContext.cs +++ b/backend/src/CodeSpace.Messages/Agents/SupervisorTurnContext.cs @@ -156,11 +156,13 @@ public sealed record SupervisorTurnContext public Dtos.Agents.SupervisorAgentProfile? AgentProfile { get; init; } /// - /// The operator's OBJECTIVE acceptance floor (carried from SupervisorGoalConfig.AcceptanceChecks, blank - /// entries dropped) — the argv a terminal verdict is graded against. Enforced on the resolve verdict (L4 - /// A3) AND on every terminal stop (L4 C1), so it gates the run's FINAL reviewable head even on a clean - /// (no-conflict) run that never resolved — the model can tighten acceptance with its own stop command but can - /// never ship past this floor. null (none / all-blank) ⇒ no operator grade runs (byte-identical). + /// The operator's OBJECTIVE acceptance floor (carried from SupervisorGoalConfig.AcceptanceChecks verbatim — + /// blank entries AFTER the program are kept, because every element is an argument) — the argv a terminal verdict is + /// graded against. Enforced on the resolve verdict (L4 A3) AND on every terminal stop (L4 C1), so it + /// gates the run's FINAL reviewable head even on a clean (no-conflict) run that never resolved — the model can + /// tighten acceptance with its own stop command but can never ship past this floor. null (none, or an argv + /// with a blank first element or a NUL, which AcceptanceArgv.NormalizeOrNull refuses whole) ⇒ no operator + /// grade runs (byte-identical). /// public IReadOnlyList? AcceptanceChecks { get; init; } diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/Supervisor/SupervisorDecisionEvalDeciderTests.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/Supervisor/SupervisorDecisionEvalDeciderTests.cs index a2e03b8fd..9b2d5aaf9 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Workflows/Supervisor/SupervisorDecisionEvalDeciderTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/Supervisor/SupervisorDecisionEvalDeciderTests.cs @@ -154,6 +154,31 @@ public async Task A_retry_aimed_at_a_finished_unit_is_re_asked_and_the_re_aimed_ decision.RetryTargetReasked.ShouldBeTrue("…and the decision says it cost a second round-trip, so a scored run can be told apart from a clean one"); } + /// + /// The operator-floor block's effect on STOPPING is measured here only while the prompt the REAL decider puts on the + /// wire for a stop-only tape actually carries the floor — the fidelity corpus pins BuildUserPromptForTest, and this + /// is the same claim at the eval's own grain: drive, see what the model was shown, score. clean-integration accepts + /// nothing but a stop, so a model that reads the block's "drive the work until it passes" as a reason to keep working + /// fails it — and the canned stop still passes with the floor in front of it, so the tape's right answer is unmoved. + /// + [Fact] + public async Task The_stop_only_scenario_is_decided_under_the_operator_floor_block() + { + var scenario = Scenario("clean-integration"); + var client = new CannedStructuredClient(CorrectDecisionJson(scenario.Name)); + + var decision = await DecideAsync(scenario, client); + + var shown = client.LastUserPrompt.ShouldNotBeNull("the decider never asked the model anything"); + + shown.ShouldContain(LlmSupervisorDecider.OperatorFloorHeader, Case.Sensitive, + "the scenario that measures what the floor block does to a stop must show the model the floor it is graded under"); + shown.ShouldContain($"""{LlmSupervisorDecider.OperatorFloorHeader}{Environment.NewLine} ["dotnet","test"]{Environment.NewLine}""", Case.Sensitive, + "…and the argv itself, on the line right under the header — a floor block that lost its argv tells the model a check runs without saying which"); + scenario.AcceptedKinds.ShouldBe([SupervisorDecisionKinds.Stop], "the floor's presence must not widen or move the only right answer on this tape"); + SupervisorDecisionEval.Score(scenario, decision).Pass.ShouldBeTrue("a stop over a clean, fully integrated tape is still the right answer with the operator floor on the wire"); + } + private static SupervisorGoldenScenario Scenario(string name) => SupervisorDecisionGoldenScenarios.All.Single(s => s.Name == name); /// Drive the real decider over a SEQUENCE of replies — the payload-less first reply then the correction — so the eval exercises the coherence gate's re-ask, not just the happy projection. @@ -165,9 +190,11 @@ private static async Task DecideAsync(SupervisorGoldenScenar return await decider.DecideAsync(scenario.Context, CancellationToken.None); } - private static async Task DecideAsync(SupervisorGoldenScenario scenario, string cannedModelJson) + private static Task DecideAsync(SupervisorGoldenScenario scenario, string cannedModelJson) => DecideAsync(scenario, new CannedStructuredClient(cannedModelJson)); + + private static async Task DecideAsync(SupervisorGoldenScenario scenario, CannedStructuredClient client) { - var registry = new LLMClientRegistry(new ILLMClient[] { new CannedStructuredClient(cannedModelJson) }); + var registry = new LLMClientRegistry(new ILLMClient[] { client }); var decider = new LlmSupervisorDecider(registry, new StubPoolSelector(CannedProvider), new CodeSpace.Core.Services.Agents.AgentHarnessRegistry(System.Array.Empty()), RealModelLiveWire.Personas(), new InMemoryTapeSummaryStore(), new NullRepoGrounding(), NullLogger.Instance); return await decider.DecideAsync(scenario.Context, CancellationToken.None); @@ -224,10 +251,18 @@ private sealed class CannedStructuredClient : ILLMClient, IStructuredLLMClient private readonly JsonElement _json; public CannedStructuredClient(string modelJson) => _json = JsonDocument.Parse(modelJson).RootElement.Clone(); + /// The user prompt of the last request — what the model was actually shown. + public string? LastUserPrompt { get; private set; } + public string Provider => CannedProvider; public Task CompleteAsync(LLMCompletionRequest request, CancellationToken cancellationToken) => throw new NotSupportedException("the eval uses the structured path only"); - public Task CompleteStructuredAsync(StructuredLLMCompletionRequest request, CancellationToken cancellationToken) => - Task.FromResult(new StructuredLLMCompletion { Json = _json, Model = request.Model }); + + public Task CompleteStructuredAsync(StructuredLLMCompletionRequest request, CancellationToken cancellationToken) + { + LastUserPrompt = request.UserPrompt; + + return Task.FromResult(new StructuredLLMCompletion { Json = _json, Model = request.Model }); + } } /// An that walks a fixed sequence of replies (the last one repeats) — the seam a bounded repair round-trip is visible at. diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/Supervisor/SupervisorDecisionGoldenScenarios.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/Supervisor/SupervisorDecisionGoldenScenarios.cs index 0ffcef1af..1f2025b2a 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Workflows/Supervisor/SupervisorDecisionGoldenScenarios.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/Supervisor/SupervisorDecisionGoldenScenarios.cs @@ -162,7 +162,22 @@ public static class SupervisorDecisionGoldenScenarios AcceptedKinds = new[] { SupervisorDecisionKinds.Merge }, }; - /// The merge integrated CLEANLY (a reviewable branch exists, no conflict) → STOP and ship; the goal is met and nothing remains (re-merging / re-spawning is churn). + /// + /// The merge integrated CLEANLY (a reviewable branch exists, no conflict) → STOP and ship; the goal is met and nothing remains (re-merging / re-spawning is churn). + /// + /// It carries the operator's acceptance FLOOR () so the decision eval can see + /// what the floor block does to STOPPING. The block tells the model to "drive the work until it passes before declaring + /// success" over a check whose verdict it can never see before it stops, so the way that copy can hurt most is a model + /// that reads it as a reason not to stop. carries a floor too, but accepts + /// retry, stop and ask_human: a model that answered retry because of the block still passes it. Until this tape, no + /// scenario whose only right answer is a stop carried one. + /// + /// This tape, and not , because the floor's presence must not be able to change the + /// right answer here. Every unit is green and integrated, so a dotnet test floor is consistent with stopping and + /// stop stays the ONLY answer. The cap-spent tape records "the build still fails and the tests do not pass" and accepts + /// ask_human beside stop, so under the same floor the block could legitimately move the answer, and a miss would be + /// unreadable — the floor, or the judgement? + /// private static SupervisorGoldenScenario CleanIntegration() => new() { Name = "clean-integration", @@ -173,7 +188,7 @@ public static class SupervisorDecisionGoldenScenarios Agent(Agent1, "Succeeded", summary: "s1", branch: "agent/s1"), Agent(Agent2, "Succeeded", summary: "s2", branch: "agent/s2")), CleanMerge(), - }), + }, DeclaredAcceptanceFloor), AcceptedKinds = new[] { SupervisorDecisionKinds.Stop }, }; @@ -680,9 +695,9 @@ private static SupervisorPriorDecision ConfirmationAnswered(string answer) }; /// - /// The operator's objective floor for — real argv, because - /// is the command a terminal verdict is graded against, not - /// a label. + /// The operator's objective floor for and + /// — real argv, because is the command a terminal verdict is graded + /// against, not a label. /// A PROPERTY for the reason is one, and this one was caught the hard way: /// as a static readonly field declared below it was still null while the /// scenarios were being built, so the floor never reached the fold and the quality line recited @@ -728,10 +743,10 @@ private static SupervisorPriorDecision WaiveApproved(string subtaskId, long sequ /// /// is the OPERATOR's objective floor /// (), and null — the default, and what every scenario but - /// passes — leaves the context byte-identical to before it was a - /// parameter. It is threaded HERE rather than applied with a with at the call site because the quality - /// fold below reads it: a floor attached after the fold would be invisible to the recommendation that is - /// supposed to be derived from it. + /// and passes — leaves the context + /// byte-identical to before it was a parameter. It is threaded HERE rather than applied with a with at the + /// call site because the quality fold below reads it: a floor attached after the fold would be invisible to the + /// recommendation that is supposed to be derived from it. /// private static SupervisorTurnContext Context(int turn, IReadOnlyList priors, IReadOnlyList? acceptanceChecks = null) { diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/Supervisor/SupervisorGoldenPromptFidelityTests.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/Supervisor/SupervisorGoldenPromptFidelityTests.cs index c0577200f..616b36800 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Workflows/Supervisor/SupervisorGoldenPromptFidelityTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/Supervisor/SupervisorGoldenPromptFidelityTests.cs @@ -1,5 +1,6 @@ using System.Security.Cryptography; using System.Text; +using System.Text.Json; using CodeSpace.Core.Services.Completion; using CodeSpace.Core.Services.Supervisor; using CodeSpace.Core.Services.Supervisor.Deciders; @@ -631,7 +632,55 @@ 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: every unit's agent-reported text — its closing summary, or the error its harness reported — moved + /// THIS RE-PIN: two prompt moves, each attributed to its own named set. + /// + /// (1) A tape whose ONLY right answer is a stop now carries the operator's acceptance floor. + /// clean-integration declares ["dotnet", "test"], so the operator-floor block (the re-pin two below) + /// renders on it too and the decision eval can see what that block does to STOPPING. Its one carrier until now, + /// repeat-failure-under-a-declared-check, accepts retry, stop and ask_human, so a model that answered retry + /// BECAUSE of the block passed it. The tape was chosen over resolve-cap-spent because the floor's presence must + /// not be able to change its right answer: every unit on it is green and integrated, where the cap-spent tape records a + /// failing build and accepts ask_human beside stop. The block is the only thing that moved there — + /// derives that for both carriers — + /// and keeps the measurement from + /// quietly going back to one that cannot see a refusal to stop. + /// + /// (2) A check's argv is an array wherever the prompt shows one besides the floor. The CURRENT PLAN block joined + /// the elements of a unit's acceptance check (…) with a space, and the amendment card quoted its proposed check + /// the same way — which lost every element boundary (["sh", " ", "check.sh"] read as two arguments, and + /// [" ", "dotnet", "test"] as dotnet test) and read a pipe inside an argument as a pipeline, for an argv + /// the grader spawns with no shell. Both now go through the renderer the floor block uses + /// (AcceptanceArgv.AsLine), so a command is never written two ways in one prompt: the card sat a few words from + /// the plan block's AMENDED…: ["dotnet","test"] and still said dotnet test. It moves exactly + /// — the tapes that show a unit's check, repeat-failure-under-a-declared-check + /// (the plan's own) and amended-oracle-awaiting-retry (the co-signed replacement) — and + /// , the tapes whose card proposes one, amended-oracle-awaiting-retry and + /// amended-oracle-discarded-by-replan (whose two cards outlive the re-plan that discarded their amendments). + /// derives both sets and + /// requires the corpus with those arrays joined back, and the new floor withheld, to digest to + /// — the pin this constant held before this re-pin, the agent-report + /// fence's. So the movers are four of the 29 — clean-integration, repeat-failure-under-a-declared-check, + /// amended-oracle-awaiting-retry and amended-oracle-discarded-by-replan — the other 25 are + /// byte-identical to it, and their scores stay comparable across the change. + /// + /// These moves reach scenarios the superseded anchors measured, and neither exclusion set changes. + /// clean-integration is one of the original scenarios, inside all four anchors, and + /// amended-oracle-awaiting-retry and amended-oracle-discarded-by-replan are inside the 25-scenario roster + /// anchor. + /// and name scenarios ADDED after a pin; + /// excluding one a pin DID measure would recompute that pin over a corpus it never saw, which is the degradation their + /// docs warn about. So, like the quality block, the cost recital, the merge mask and the fence before them, the moves + /// are undone by — the floor block is a pure insertion, the argv arrays + /// pure substitutions — and every anchor still reproduces its own digest over its own corpus. The fence's own receipt + /// () and the floor block's now unwind this re-pin + /// first, for the same reason. + /// + /// What did NOT move, on purpose: the SYSTEM prompt, the stopped-now recital (neither the floor, a check line nor + /// a card reaches it), SupervisorQualityFacts, the scenario count, and every AcceptedKinds — the floor only + /// adds a block to a tape that already accepts a stop, and the arrays only rewrite the text of lines a tape already + /// showed. The waive card on waived-unit-cleanly-integrated proposes no check, so it is byte-identical. + /// + /// PREVIOUS RE-PIN: every unit's agent-reported text — its closing summary, or the error its harness reported — moved /// off the one raw line it shared with the unit's status and behind the data prefix the oracle's evidence tail /// already used ( | ), under a header that names it agent-reported data /// (LlmSupervisorDecider.RenderAgentReport). Rendered raw, a summary could restage the server's own @@ -656,9 +705,9 @@ public void Only_a_scenario_missing_a_required_stage_renders_a_different_prompt_ /// (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, + /// — then the one scenario whose context declared a floor, /// repeat-failure-under-a-declared-check (["dotnet", "test"]) — and nothing else moves. - /// derives that rather than + /// 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 @@ -789,13 +838,26 @@ 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 = "6586d47c358c273cb04fe0b662473382a09fe4058916fb8c96e18c1bcc79a946"; + private const string GoldenPromptDigest = "8082603c53395c055e885c1d03b81d363d768048c41bf5ed4c6759e4b0d9c53e"; + + /// + /// The pin this corpus carried from the agent-report fence's arrival until a stop-only tape carried a floor and a + /// check's argv became an array: the floor block rendered on ONE tape (repeat-failure-under-a-declared-check), + /// and the CURRENT PLAN block and the amendment card joined a check's elements with spaces. + /// Superseded, never deleted — it is the fixed point + /// measures today's + /// rendering against: with every check argv joined back and the floor withheld from the tape that gained one since + /// (), the corpus must still digest to it over all 29 scenarios, so the + /// move is those two changes and nothing else. A digest whose predecessor is deleted can only ever be compared with itself. + /// + private const string PreStopTapeFloorCorpusDigest = "6586d47c358c273cb04fe0b662473382a09fe4058916fb8c96e18c1bcc79a946"; /// /// The pin this corpus carried while each unit's closing summary (or reported error) rendered RAW on its /// agent {k}: line. Superseded, never deleted: - /// winds the fence back over today's rendering and requires this digest over all 29 scenarios, so the fence is - /// the ONLY thing the re-pin moved. + /// winds the fence back over the rendering that was pinned just after it ( + /// undoes the two moves since) and requires this digest over all 29 scenarios, so the fence is the ONLY thing that + /// re-pin moved. /// private const string PreAgentReportFenceCorpusDigest = "6b9c621bf49c7e5858a1289f62581319c3974d8f86cab3272c54d1574084c893"; @@ -803,9 +865,11 @@ public void Only_a_scenario_missing_a_required_stage_renders_a_different_prompt_ /// 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. + /// measures today's rendering + /// against: the corpus with every floor withheld, every check argv joined the way it was written then and the + /// agent-report fence wound back, must still digest to it over all 29 scenarios, so the moves since are the floor + /// block, the fence and the check argv arrays and nothing else. A digest whose predecessor is deleted can only + /// ever be compared with itself. /// private const string PreOperatorFloorCorpusDigest = "2dafcdc7e52c3d22d1d2d209bb05bbe701191131defeb9e11f26360ef70bf5fb"; @@ -889,14 +953,15 @@ public void Only_a_scenario_missing_a_required_stage_renders_a_different_prompt_ [Fact] public void The_rendered_corpus_matches_its_pinned_digest() { - // The historical receipt: wind the five named blocks back to what they replaced — the roster to the - // mask block it grew out of, the cap-aware closing line to the invitation it retired, the cap-aware ending - // to its unconditional predecessor, and the newly inserted per-unit cost recital to absence — and the pin that - // stood before them must return, over the 25 scenarios that pin was measured at (AddedSinceTheRosterPin — - // the per-pin-corpus rule the set below states). A block that drifted into this commit fails here, where the named - // ones are still separable from it, instead of hiding inside the re-pin below. + // The historical receipt: wind every later change back to what it replaced — AsRenderedBeforeTheTurnRoster + // lists the nine: the roster to the mask block it grew out of, the cap-aware closing lines to the invitation and + // the unconditional ending they retired, the cost and quality recitals and the floor block to absence, the fence + // and the check argv arrays to the raw line and the joined text they replaced — and the pin that stood before them + // must return, over the 25 scenarios that pin was measured at (AddedSinceTheRosterPin — the per-pin-corpus rule the + // set below states). A block that drifted into this commit fails here, where the named ones are still separable + // from it, instead of hiding inside the re-pin below. Digest(RenderedCorpus(s => AsRenderedBeforeTheTurnRoster(LlmSupervisorDecider.BuildUserPromptForTest(s.Context), s.Context), PredatesTheRosterPin)).ShouldBe(StaticVerbRosterCorpusDigest, - "undoing the four named prompt blocks no longer reproduces the pin this corpus carried before them — an unrelated delta entered the historical reconstruction"); + "undoing the named prompt blocks no longer reproduces the pin this corpus carried before them — an unrelated delta entered the historical reconstruction"); // This re-pin's receipt: over the scenarios that predate it, today's rendering still digests to the // superseded pin — so the move is corpus GROWTH and nothing else, and every score taken under the old pin @@ -1102,34 +1167,76 @@ 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). + /// The scenarios whose context carries an operator acceptance floor — the named receipt for the floor half of the + /// current , exactly like and + /// are for theirs. A TWO-element set because the floor is the run-level argv the + /// stop path runs, and this corpus declares it in two places, both with the same ["dotnet", "test"]: + /// repeat-failure-under-a-declared-check, whose argv is also the one 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), and clean-integration, the tape whose ONLY right answer is a stop — which is why it carries one: + /// see . /// private static readonly HashSet CarriesAnOperatorFloor = new(StringComparer.Ordinal) { - "repeat-failure-under-a-declared-check", + "repeat-failure-under-a-declared-check", "clean-integration", + }; + + /// + /// The scenarios that gained their floor AFTER was taken — the receipt for + /// the floor half of this re-pin, and what turns that pin's own corpus (the floor on + /// repeat-failure-under-a-declared-check alone) back into a rendering: the floor is withheld from exactly these. + /// + private static readonly HashSet GainedAFloorSinceThePreviousPin = new(StringComparer.Ordinal) + { + "clean-integration", }; /// - /// 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. + /// The scenarios whose CURRENT PLAN block shows a unit's own acceptance check — the receipt for the check-line half of + /// this re-pin. Two tapes, each through a different door: repeat-failure-under-a-declared-check's plan authors + /// s1's oracle, and amended-oracle-awaiting-retry shows s2's co-signed REPLACEMENT through the amend overlay. + /// The other co-sign tape, amended-oracle-discarded-by-replan, shows none: the re-plan that follows its co-signs + /// discarded them, so nothing is in force to show — its cards still move, see . + /// + private static readonly HashSet CarriesASubtaskCheck = new(StringComparer.Ordinal) + { + "amended-oracle-awaiting-retry", "repeat-failure-under-a-declared-check", + }; + + /// + /// The scenarios whose tape carries an amendment card that PROPOSES a replacement check — the receipt for the card half + /// of this re-pin. The card is the production builder's (SupervisorAmendAcceptance.IntoAskHuman), its question + /// reaches the prompt on the ask_human line, and its proposed check is now the same array the floor and the check line + /// use. Both co-sign tapes carry such cards — amended-oracle-awaiting-retry's one, and + /// amended-oracle-discarded-by-replan's two, which stay on the tape after the re-plan that discarded their + /// amendments. waived-unit-cleanly-integrated's card is a WAIVE: it proposes no check, so it does not move. + /// + private static readonly HashSet CarriesAnAmendCard = new(StringComparer.Ordinal) + { + "amended-oracle-awaiting-retry", "amended-oracle-discarded-by-replan", + }; + + /// + /// The named receipt for the floor half of 's move: the OPERATOR ACCEPTANCE FLOOR + /// block renders on the tapes that carry a floor and on no other, and it is the ONLY thing that moved there. 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; each 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 (and every check argv joined the way + /// it was written then, and the fence wound back) 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. + /// line spelled out here is the argv, which is the fixture's own floor, shared by both carriers. + /// + /// The string-level wind-back the superseded anchors use () is checked + /// here against the context-level one () over EVERY scenario — a mirror with no drift + /// detector is a test that passes while production changes, and those anchors would stop reproducing their pins + /// without saying why. /// [Fact] - public void Only_the_tape_that_carries_an_operator_floor_gains_the_floor_block() + public void Only_the_tapes_that_carry_an_operator_floor_gain_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"); @@ -1139,32 +1246,108 @@ public void Only_the_tape_that_carries_an_operator_floor_gains_the_floor_block() 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(s => AsReportedBeforeTheFence(FloorWithheld(s), s.Context))).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"); + Digest(RenderedCorpus(s => AsReportedBeforeTheFence(AsJoinedBeforeTheArgvArray(FloorWithheld(s)), s.Context))).ShouldBe(PreOperatorFloorCorpusDigest, + "with every floor withheld (each check argv joined as it was, and the fence wound back) 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"); + foreach (var scenario in SupervisorDecisionGoldenScenarios.All) + { + var prompt = LlmSupervisorDecider.BuildUserPromptForTest(scenario.Context); + + WithoutTheOperatorFloorBlock(prompt).ShouldBe(FloorWithheld(scenario), + $"'{scenario.Name}': the wind-back the superseded anchors route through must undo exactly what withholding the floor does — no more, no less"); + + if (!CarriesAnOperatorFloor.Contains(scenario.Name)) continue; + + FloorWithheld(scenario).ShouldNotContain(LlmSupervisorDecider.OperatorFloorHeader, Case.Sensitive, + $"'{scenario.Name}' fixture check: the block must come from the floor alone — if the tape already shows its header elsewhere, the presence assertion below proves nothing"); + prompt.ShouldContain(block, Case.Sensitive, + $"'{scenario.Name}' carries the floor, so the model is shown that floor's block — present, and not merely some other byte moved"); + prompt.Replace(block, string.Empty, StringComparison.Ordinal).ShouldBe(FloorWithheld(scenario), + $"'{scenario.Name}': the prompt minus the block's own lines must be exactly its floor-withheld prompt — so the block is the ONLY thing that moved"); + } } /// - /// The named receipt for 's move: agent-reported text moved behind the fence and - /// nothing else moved. With the fence wound back the corpus must digest to - /// — the pin before the fence — and the scenarios whose prompt moved must be exactly the ones whose rendered tape - /// carries an agent's summary or reported error. + /// Why the floor is on a stop-only tape at all: the floor block tells the model to drive the work until the check passes + /// "before declaring success", over a verdict it can never see before it stops, so the way that copy can hurt most is a + /// model that reads it as a reason NOT to stop. A carrier that also accepts retry or ask_human cannot show that — a + /// model that answered retry because of the block passes it. So at least one tape that carries a floor must accept + /// nothing but a stop, or the decision eval is back to measuring the block on a tape that forgives the failure it + /// exists to catch. The cheap structural guard of 's + /// kind: an accepted set is fixture data, so no live run can notice it widen. + /// + [Fact] + public void A_tape_whose_only_right_answer_is_a_stop_carries_an_operator_floor() + { + var stopOnly = SupervisorDecisionGoldenScenarios.All + .Where(s => s.Context.AcceptanceChecks is { Count: > 0 } && s.AcceptedKinds.SequenceEqual([SupervisorDecisionKinds.Stop])) + .Select(s => s.Name) + .ToList(); + + stopOnly.ShouldNotBeEmpty("the decision eval can only see the floor block suppress a stop on a tape whose accepted set is exactly {stop} — every carrier that also accepts retry or ask_human passes a model that over-read the block"); + } + + /// + /// The named receipt for the argv-array half of 's move: a check's argv is an array on + /// the two surfaces the prompt shows one besides the floor — a unit's own acceptance check (…) line and an + /// amendment card's Proposed check: — and on no other tape did anything change. Derived — every scenario's prompt + /// is compared with the same prompt's arrays joined back the way the decider and the card builder used to write them, + /// and for each surface the set that differs must be exactly its named receipt + /// (, ) — and anchored: with both surfaces joined + /// back and the floor withheld from the tape that gained one since (), the + /// corpus must digest to , the pin held before this re-pin. So the whole move + /// is those changes, and no scenario outside the named sets moved — asserted per scenario below as well, so a failure + /// names the tape. + /// + [Fact] + public void Only_the_tapes_that_show_a_check_argv_write_it_as_an_array_not_a_joined_line() + { + ShowingAnArgvArray(JoinedCheckLine).ShouldBe(CarriesASubtaskCheck.ToList(), ignoreOrder: true, + "the set of scenarios that show a unit's own check must match the named receipt — a tape whose check line did not become an array, or one that gained a check line, is a re-pin nobody attributed"); + ShowingAnArgvArray(JoinedProposedCheck).ShouldBe(CarriesAnAmendCard.ToList(), ignoreOrder: true, + "the set of scenarios whose amendment card proposes a check must match the named receipt — a card that still quotes a shell line, or a tape that gained one, is a re-pin nobody attributed"); + + var moved = SupervisorDecisionGoldenScenarios.All.Where(s => LlmSupervisorDecider.BuildUserPromptForTest(s.Context) != AsRenderedAtThePreviousPin(s)).Select(s => s.Name).ToList(); + + moved.ShouldBe(GainedAFloorSinceThePreviousPin.Union(CarriesASubtaskCheck).Union(CarriesAnAmendCard).ToList(), ignoreOrder: true, + "every scenario that moved since the previous pin must be named by one of the receipts — a mover in none is a re-pin nobody attributed"); + + Digest(RenderedCorpus(AsRenderedAtThePreviousPin)).ShouldBe(PreStopTapeFloorCorpusDigest, + "with the argv arrays joined back and the new floor withheld, the corpus must digest to the pin it carried before this re-pin — anything else drifted into the same commit, so the move is then not these changes alone"); + + foreach (var name in CarriesASubtaskCheck) + { + var checkLines = PromptFor(name).Split(Environment.NewLine).Where(line => line.StartsWith(UnitCheckLinePrefix, StringComparison.Ordinal)).ToList(); + + checkLines.ShouldNotBeEmpty($"'{name}' is named as showing a unit's check, so its prompt must carry a check line"); + checkLines.ShouldAllBe(line => line.EndsWith(""": ["dotnet","test"]""", StringComparison.Ordinal), + $"'{name}' must show its unit's check as the JSON array the floor block uses, never a joined line — one command is never described two ways in one prompt"); + } + + foreach (var name in CarriesAnAmendCard) + { + var cardLines = PromptFor(name).Split(Environment.NewLine).Where(line => line.Contains(ProposedCheckLabel, StringComparison.Ordinal)).ToList(); + + cardLines.ShouldNotBeEmpty($"'{name}' is named as carrying a card that proposes a check, so its prompt must show one"); + cardLines.ShouldAllBe(line => line.Contains($"""{ProposedCheckLabel}["dotnet","test"] {SupervisorAmendAcceptance.AmendMarker}""", StringComparison.Ordinal), + $"'{name}' must show its card's proposed check as the same JSON array, never a joined line — one command is never described two ways in one prompt"); + } + } + + /// + /// The named receipt for the agent-report fence's re-pin ( is the pin it + /// produced): agent-reported text moved behind the fence and nothing else moved. With the fence wound back — and the + /// two later moves undone first, — the corpus must digest to + /// , the pin before the fence, and the scenarios whose prompt moved must + /// be exactly the ones whose rendered tape carries an agent's summary or reported error. /// [Fact] public void Only_the_agent_reported_text_moved_behind_the_fence() { - Digest(RenderedCorpus(s => AsReportedBeforeTheFence(LlmSupervisorDecider.BuildUserPromptForTest(s.Context), s.Context))).ShouldBe(PreAgentReportFenceCorpusDigest, - "with the fence wound back the corpus must digest to the pin it carried before the fence — anything else drifted into the same commit"); + Digest(RenderedCorpus(s => AsReportedBeforeTheFence(AsRenderedAtThePreviousPin(s), s.Context))).ShouldBe(PreAgentReportFenceCorpusDigest, + "with the fence wound back (and the stop-tape floor and the check argv arrays undone) the corpus must digest to the pin it carried before the fence — anything else drifted into the same commit"); var moved = SupervisorDecisionGoldenScenarios.All.Where(s => LlmSupervisorDecider.BuildUserPromptForTest(s.Context) is var prompt && prompt != AsReportedBeforeTheFence(prompt, s.Context)).Select(s => s.Name).ToList(); var carriers = SupervisorDecisionGoldenScenarios.All.Where(s => s.Context.PriorDecisions @@ -1180,6 +1363,72 @@ public void Only_the_agent_reported_text_moved_behind_the_fence() /// 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 }); + /// One scenario's prompt as was taken over it: every check argv joined (a unit's check line, an amendment card's proposed check), and the floor present only on the tape that carried one then. + private static string AsRenderedAtThePreviousPin(SupervisorGoldenScenario scenario) => + AsJoinedBeforeTheArgvArray(LlmSupervisorDecider.BuildUserPromptForTest(GainedAFloorSinceThePreviousPin.Contains(scenario.Name) ? scenario.Context with { AcceptanceChecks = null } : scenario.Context)); + + /// The operator-floor block's four lines — header, argv, closing note, and the blank line that ends it — removed from a prompt that has one, and the prompt returned untouched when it has none. Taken off the lines the decider wrote rather than retyped, so it does not depend on which argv the floor holds. + private static string WithoutTheOperatorFloorBlock(string prompt) + { + var lines = prompt.Split(Environment.NewLine).ToList(); + var at = lines.IndexOf(LlmSupervisorDecider.OperatorFloorHeader); + + if (at < 0) return prompt; + + lines[at + 2].ShouldBe(LlmSupervisorDecider.DoNotRepeatTheFloorAsTheStopAcceptance, "the block is header, ONE argv line, closing note — a block of another shape is one this wind-back would cut the wrong lines out of"); + lines[at + 3].ShouldBeEmpty("a blank line ends the block"); + + lines.RemoveRange(at, 4); + + return string.Join(Environment.NewLine, lines); + } + + private const string UnitCheckLinePrefix = " acceptance check ("; + + private const string ProposedCheckLabel = "Proposed check: "; + + /// + /// The prompt as it read before a check's argv was an array on the two surfaces that show one besides the floor: each + /// unit acceptance check (…): ["a","b"] line () and each amendment card's + /// Proposed check: ["a","b"] () back to its elements joined with one space, + /// which is what the decider and the card builder wrote then. Read back off the array the code now writes rather than + /// retyped, so the elements are the ones it wrote. The corpus's argvs are plain — no newline, none near a line's bound + /// — so the join is exactly what the old rule printed. A waived unit's line (acceptance check: WAIVED …) + /// carries no argv and is left alone. + /// + private static string AsJoinedBeforeTheArgvArray(string prompt) => string.Join(Environment.NewLine, prompt.Split(Environment.NewLine).Select(line => JoinedProposedCheck(JoinedCheckLine(line)))); + + /// The scenarios whose rendered prompt has at least one line rewrites — the tapes that show that surface's argv array. Derived off the rendering, so the named receipts are checked against what the prompt really says. + private static List ShowingAnArgvArray(Func joinedBack) => + SupervisorDecisionGoldenScenarios.All.Where(s => LlmSupervisorDecider.BuildUserPromptForTest(s.Context).Split(Environment.NewLine).Any(line => joinedBack(line) != line)).Select(s => s.Name).ToList(); + + private static string JoinedCheckLine(string line) + { + if (!line.StartsWith(UnitCheckLinePrefix, StringComparison.Ordinal)) return line; + + // "…acceptance check (TestsPass): [...]" and "…acceptance check (TestsPass), AMENDED by a co-signed amendment: [...]" — + // neither the kind nor the amendment marker holds a ": ", so the first one separates the label from the array. + var separator = line.IndexOf(": ", StringComparison.Ordinal) + ": ".Length; + + return line[..separator] + string.Join(" ", JsonSerializer.Deserialize(line[separator..])!); + } + + private static string JoinedProposedCheck(string line) + { + var label = line.IndexOf(ProposedCheckLabel, StringComparison.Ordinal); + + if (label < 0) return line; + + // The card's question reaches the prompt on ONE line, its newlines flattened, with the marker sentence right after + // the check: "…Proposed check: [...] Reply 'approve' to apply this acceptance amendment, …". + var start = label + ProposedCheckLabel.Length; + var end = line.IndexOf($" {SupervisorAmendAcceptance.AmendMarker}", start, StringComparison.Ordinal); + + end.ShouldBeGreaterThan(start, "the card's marker follows its proposed check on the line — a card the prompt's line bound cut short of its marker is one this wind-back cannot read"); + + return line[..start] + string.Join(" ", JsonSerializer.Deserialize(line[start..end])!) + line[end..]; + } + private static string PromptFor(string name) => LlmSupervisorDecider.BuildUserPromptForTest(SupervisorDecisionGoldenScenarios.All.Single(s => s.Name == name).Context); @@ -1282,36 +1531,39 @@ private static string RenderedCorpus(Func rend " To reconcile: choose 'resolve' — the server spawns ONE agent that reconciles these branches, builds, and runs the tests, then you merge again. Or stop to leave the conflict for a human."; /// - /// One scenario's prompt wound back to the rendering that produced the superseded pins below. FIVE blocks are - /// undone, and they are the five later changes moved: + /// One scenario's prompt wound back to the rendering that produced the superseded pins below. NINE blocks are + /// undone, one for each later change that moved a prompt those pins had already measured: /// /// the turn's VERB ROSTER, replaced by the action mask it grew out of — as that mask read before it /// could withhold amend_acceptance (); /// the conflicted-integration block's cap-aware closing line, replaced by the invitation it retired; /// the prompt's cap-aware CLOSING SENTENCE, replaced by the unconditional "then merge the successful - /// results, then stop." it retired. + /// results, then stop." it retired; /// the per-unit MODEL/TOKEN/COST recitation, removed because it did not exist when any superseded digest - /// was recorded. - /// the clean-integration merge mask and closing sentence, restored to their prior offer. + /// was recorded; + /// the QUALITY POLICY block (P22-9b), removed for the same reason; + /// the clean-integration merge mask and closing sentence, restored to their prior offer; /// each unit's agent-reported text, back on the one raw line it rendered as before the fence - /// (). + /// (); + /// the OPERATOR FLOOR block, removed (); + /// a check's argv array — a unit's check line, an amendment card's proposed check — joined back + /// (). /// /// It exists because the roster renders on EVERY turn where the mask rendered on most, so a superseded digest /// recomputed over today's raw rendering can no longer reproduce itself, and every receipt below would have to /// be deleted or re-pinned into a tautology. /// - /// Winding them back keeps the receipts, and makes them stronger than a re-pin would: the roster is a pure - /// INSERTION over the mask, the unit-cost recital is a pure insertion, and the other two are pure substitutions, - /// so undoing exactly those four must return - /// the pre-commit bytes — which is what the anchors assert by still reproducing their old digests over the - /// corpus each was taken at. Anything else that drifted into this commit shows up as a failure here rather than - /// as a digest nobody can attribute. + /// Winding them back keeps the receipts, and makes them stronger than a re-pin would: the roster (over the + /// mask), the cost and quality recitals and the floor block are pure INSERTIONS and the other five are pure + /// substitutions, so undoing exactly those nine must return the pre-change bytes — which is what the anchors assert by + /// still reproducing their old digests over the corpus each was taken at. Anything else that drifted into the commit + /// shows up as a failure here rather than as a digest nobody can attribute. /// - /// THE COST, stated plainly: every anchor that routes through this helper is now BLIND to the four blocks - /// it undoes. A reword of the roster, the per-unit cost recital, the cap-aware closing line, or the closing - /// sentence moves no superseded digest — by construction, since the wind-back derives the roster and cost-recital - /// bytes from today's renderers. Only the live catches a change in them, so a - /// re-pin of THAT constant is the only + /// THE COST, stated plainly: every anchor that routes through this helper is now BLIND to the nine blocks it + /// undoes. A reword of any of them — the roster, the cost and quality recitals, the cap-aware closing lines, the + /// fence's header, the floor block's own lines — moves no superseded digest, by construction, since the wind-back + /// derives those bytes from today's renderers (and reads the argv arrays back off the arrays the code writes). Only + /// the live catches a change in them, so a re-pin of THAT constant is the only /// place such a change becomes visible, and the per-block unit tests /// (SupervisorActionRosterTests) are what pin the copy itself. /// @@ -1321,8 +1573,13 @@ private static string AsRenderedBeforeTheTurnRoster(string prompt, SupervisorTur var roster = $"{Environment.NewLine}{SupervisorActionRoster.Render(context)}{Environment.NewLine}"; var mask = AsMaskedBeforeTheAmendArm(context) is { } withheld ? $"{Environment.NewLine}{withheld}{Environment.NewLine}" : string.Empty; + // The operator-floor block (a pure INSERTION, on every tape that carries a floor) and the check argv arrays (pure + // SUBSTITUTIONS: a unit's check line and an amendment card's proposed check, on every tape that shows one) — + // neither existed when any superseded digest was recorded, and they reach scenarios those digests DID measure, so + // they are wound back rather than excluded. + var beforeTheFloor = AsJoinedBeforeTheArgvArray(WithoutTheOperatorFloorBlock(prompt)); var unitCosts = SupervisorBudgetRecitation.RenderUnits(context.PriorDecisions, context.ModelPrices); - var beforeUnitCosts = unitCosts is null ? prompt : prompt.Replace($"{Environment.NewLine}{unitCosts}{Environment.NewLine}", string.Empty, StringComparison.Ordinal); + var beforeUnitCosts = unitCosts is null ? beforeTheFloor : beforeTheFloor.Replace($"{Environment.NewLine}{unitCosts}{Environment.NewLine}", string.Empty, StringComparison.Ordinal); // P22-9b — the QUALITY POLICY block, removed the same way the per-unit cost recital is: a pure INSERTION // that renders on every tape with an attempted unit, so a superseded digest recomputed over today's raw diff --git a/backend/tests/CodeSpace.UnitTests/Agents/AcceptanceArgvTests.cs b/backend/tests/CodeSpace.UnitTests/Agents/AcceptanceArgvTests.cs new file mode 100644 index 000000000..21d45f951 --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Agents/AcceptanceArgvTests.cs @@ -0,0 +1,71 @@ +using CodeSpace.Core.Services.Agents; +using Shouldly; + +namespace CodeSpace.UnitTests.Agents; + +/// +/// 🟢 Unit: the ONE rule for which acceptance argvs can be run (). The launch projections, +/// the rehydrate and the stop grade, the in-loop hook and the local verifier all ask it, so what it refuses and what it +/// keeps is pinned here once, by the decision it makes rather than through any one caller — and +/// AcceptanceArgvInventoryTests pins that those callers really do ask it instead of keeping a copy. +/// +[Trait("Category", "Unit")] +public class AcceptanceArgvTests +{ + [Fact] + public void No_argv_is_no_command() + { + AcceptanceArgv.NormalizeOrNull(null).ShouldBeNull("an absent floor is the ordinary shape, not a fault"); + AcceptanceArgv.NormalizeOrNull(Array.Empty()).ShouldBeNull("an empty argv names no program"); + } + + [Theory] + [InlineData("")] + [InlineData(" ")] + [InlineData("\t\n")] + public void A_blank_program_refuses_the_whole_argv_and_never_promotes_a_later_argument_into_it(string program) + { + AcceptanceArgv.NormalizeOrNull([program, "sh", "check.sh"]).ShouldBeNull("the argv is refused whole — promoting 'sh' into the program slot would run a command nobody wrote"); + AcceptanceArgv.NormalizeOrNull([program]).ShouldBeNull("a lone blank element is not a program either"); + } + + [Theory] + [InlineData(new[] { "sh\0", "check.sh" }, "in the program")] + [InlineData(new[] { "sh", "check\0.sh" }, "inside a later argument")] + [InlineData(new[] { "sh", "\0" }, "as a later argument on its own")] + public void A_nul_anywhere_refuses_the_whole_argv(string[] argv, string where) + { + AcceptanceArgv.NormalizeOrNull(argv).ShouldBeNull($"a NUL {where} cannot be handed to exec, so nothing of this argv may run"); + } + + [Fact] + public void A_null_element_refuses_the_whole_argv() + { + AcceptanceArgv.NormalizeOrNull(new[] { "sh", null!, "check.sh" }).ShouldBeNull("a null element is a hole in the argument list, not an empty argument"); + AcceptanceArgv.NormalizeOrNull(new[] { null!, "sh" }).ShouldBeNull("a null program is no program"); + } + + [Theory] + [InlineData(new[] { "sh", " ", "check.sh" }, "a whitespace argument between two others")] + [InlineData(new[] { "sh", "" }, "an empty trailing argument")] + [InlineData(new[] { "custom-check", "", " ", "quoted argument" }, "empty, blank and space-bearing arguments together")] + [InlineData(new[] { "dotnet", "test", "--filter", "Category=Unit|Category=Smoke" }, "a pipe and an equals sign are characters, not syntax")] + public void Every_element_after_the_program_is_kept_verbatim_blank_ones_included(string[] argv, string what) + { + AcceptanceArgv.NormalizeOrNull(argv).ShouldBe(argv, $"{what} — the element boundaries ARE the command, and TaskLaunchFlowTests pins [\"sh\", \" \", \"check.sh\"] surviving launch"); + } + + [Fact] + public void The_result_is_its_own_copy_so_a_caller_holding_the_config_cannot_move_a_graded_argv() + { + var argv = new[] { "sh", "check.sh" }; + + var normalized = AcceptanceArgv.NormalizeOrNull(argv)!; + + normalized.ShouldNotBeSameAs(argv); + + argv[1] = "other.sh"; + + normalized.ShouldBe(["sh", "check.sh"], "the argv the grader runs is the one that was normalised, whatever happens to the list it came from"); + } +} diff --git a/backend/tests/CodeSpace.UnitTests/Agents/SupervisorAmendAcceptanceTests.cs b/backend/tests/CodeSpace.UnitTests/Agents/SupervisorAmendAcceptanceTests.cs index a470fe6ef..70ce316b1 100644 --- a/backend/tests/CodeSpace.UnitTests/Agents/SupervisorAmendAcceptanceTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Agents/SupervisorAmendAcceptanceTests.cs @@ -34,7 +34,7 @@ public void The_projector_rewrites_an_amend_proposal_into_a_parked_ask_card() var question = JsonDocument.Parse(decision.PayloadJson!).RootElement.GetProperty("question").GetString()!; question.ShouldContain("AMEND subtask 's1'", customMessage: "the card names the target"); - question.ShouldContain("sh check.sh", customMessage: "the card quotes the proposed check"); + question.ShouldContain("""["sh","check.sh"]""", customMessage: "the card quotes the proposed check as the argv the grader would spawn"); question.ShouldContain(SupervisorAmendAcceptance.AmendMarker, customMessage: "the marker is the card's identity"); var readBack = SupervisorAmendAcceptance.ReadAmend(decision.PayloadJson)!; @@ -43,6 +43,41 @@ public void The_projector_rewrites_an_amend_proposal_into_a_parked_ask_card() readBack.Acceptance!.Command.ShouldBe(new[] { "sh", "check.sh" }); } + [Theory] + [InlineData(new[] { " ", "dotnet", "test" }, """[" ","dotnet","test"]""", "a blank program reads as one, never trimmed into `dotnet test` for the human who co-signs it")] + [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, so the card keeps the line structure it was written with")] + [InlineData(new[] { "custom-check", "", " ", "quoted argument" }, """["custom-check",""," ","quoted argument"]""", "empty, blank and space-bearing elements")] + public void The_card_quotes_the_proposed_check_as_the_argv_the_grader_would_spawn_and_nothing_else(string[] argv, string expected, string what) + { + var decision = SupervisorAmendAcceptance.IntoAskHuman(Amend(spec: new SupervisorAcceptanceSpec { Command = argv })); + + var question = JsonDocument.Parse(decision.PayloadJson!).RootElement.GetProperty("question").GetString()!; + + question.ShouldContain($"\nProposed check: {expected}\n{SupervisorAmendAcceptance.AmendMarker}", Case.Sensitive, + customMessage: $"{what} — the grader spawns this argv with NO shell, so the card is the array itself between its label and the marker, the same line the supervisor prompt shows"); + } + + [Theory] + [InlineData(0, false)] // exactly at the card's cap — verbatim, no ellipsis + [InlineData(1, true)] // one over — cut at the cap and marked with an ellipsis, like the reason beside it + public void The_proposed_check_is_capped_like_the_reason_beside_it(int overTheCap, bool cut) + { + // 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', SupervisorAmendAcceptance.MaxQuotedChars + overTheCap - 4); + var json = $"[\"{element}\"]"; + + var decision = SupervisorAmendAcceptance.IntoAskHuman(Amend(spec: new SupervisorAcceptanceSpec { Command = new[] { element } })); + + var question = JsonDocument.Parse(decision.PayloadJson!).RootElement.GetProperty("question").GetString()!; + + var shown = cut ? json[..SupervisorAmendAcceptance.MaxQuotedChars] + "…" : json; + + question.ShouldContain($"\nProposed check: {shown}\n{SupervisorAmendAcceptance.AmendMarker}", Case.Sensitive, + customMessage: "the argv is model-authored and unbounded, and the card is a headline — a longer line is cut and marked, never left to push the marker off the card"); + } + [Fact] public void A_waive_proposal_names_the_forgone_verification_on_its_card() { diff --git a/backend/tests/CodeSpace.UnitTests/Agents/SupervisorDeciderTests.cs b/backend/tests/CodeSpace.UnitTests/Agents/SupervisorDeciderTests.cs index 1aaa13d70..7c0f84188 100644 --- a/backend/tests/CodeSpace.UnitTests/Agents/SupervisorDeciderTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Agents/SupervisorDeciderTests.cs @@ -179,6 +179,64 @@ public void The_floor_block_follows_the_acceptance_criteria_it_is_the_executable 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"); } + // ── A subtask's OWN check is shown the way the floor is: an argv the grader spawns with no shell, never a shell line ── + + private const string CheckLinePrefix = " acceptance check (TestsPass): "; + + /// A live plan whose one subtask authors as its own acceptance check — the CURRENT PLAN block shows it on that item's acceptance check line. + private static SupervisorPriorDecision PlanWithCheck(params string[] command) => new() + { + Id = Guid.NewGuid(), Sequence = 1, DecisionKind = SupervisorDecisionKinds.Plan, Status = SupervisorDecisionStatus.Succeeded, + PayloadJson = JsonSerializer.Serialize(new SupervisorPlanPayload { Goal = "ship", Subtasks = new[] { new SupervisorPlannedSubtask { Id = "s1", Title = "Audit", Instruction = "audit it", Acceptance = new SupervisorAcceptanceSpec { Command = command } } } }, AgentJson.Options), + OutcomeJson = """{"planned":["s1"],"count":1}""", + }; + + [Fact] + public void The_per_subtask_check_line_keeps_every_argv_element_boundary_the_grader_will_run() + { + // The argv the floor line keeps (TaskLaunchFlowTests pins ["sh", " ", "check.sh"] surviving launch). A model authors + // per-unit checks too, and the joined line it used to be shown read that one as two arguments. + var prompt = LlmSupervisorDecider.BuildUserPromptForTest(Context(turnNumber: 1, PlanWithCheck("sh", " ", "check.sh"))); + + prompt.ShouldContain($$"""{{CheckLinePrefix}}["sh"," ","check.sh"]{{Environment.NewLine}}""", 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 shown a command that is not the one graded"); + } + + [Theory] + [InlineData(new[] { "sh", " ", "check.sh" }, "a whitespace element is its own argument")] + [InlineData(new[] { "dotnet", "test", "--filter", "Category=Unit|Category=Smoke" }, "a pipe inside an argument is a character, not a pipeline")] + [InlineData(new[] { "echo", "it's & +1" }, "what the default encoder would turn into \\u00XX codes")] + [InlineData(new[] { "sh", "-c", "a\nb" }, "a newline stays a visible escape, never the space a flattening would make of it")] + [InlineData(new[] { "custom-check", "", " ", "quoted argument" }, "empty, blank and space-bearing elements")] + public void The_per_subtask_check_line_writes_an_argv_exactly_as_the_floor_line_does(string[] argv, string what) + { + var floor = LlmSupervisorDecider.BuildUserPromptForTest(Context() with { AcceptanceChecks = argv }).Split(Environment.NewLine); + var headerAt = Array.IndexOf(floor, LlmSupervisorDecider.OperatorFloorHeader); + + headerAt.ShouldBeGreaterThanOrEqualTo(0, "fixture check: the floor block must be on the wire, or there is no floor line to compare the check line with"); + + var floorLine = floor[headerAt + 1].TrimStart(); + + LlmSupervisorDecider.BuildUserPromptForTest(Context(turnNumber: 1, PlanWithCheck(argv))) + .ShouldContain($"{CheckLinePrefix}{floorLine}{Environment.NewLine}", Case.Sensitive, customMessage: $"{what} — and one command is written one way in one prompt: the floor and a unit's own check share the renderer (AcceptanceArgv.AsLine), so neither can read as shell text while the other reads as an array"); + } + + [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_per_subtask_check_line_is_bounded_at_400_chars_like_the_floor_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(turnNumber: 1, PlanWithCheck(element))); + + var shown = bounded ? json[..400] + "…" : json; + + prompt.ShouldContain($"{CheckLinePrefix}{shown}{Environment.NewLine}", Case.Sensitive, customMessage: "a unit's check is unbounded model config and this line is a fixed per-turn cost the tape compaction cannot shrink"); + } + // ── 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/SupervisorPriorDecisionLegibilityTests.cs b/backend/tests/CodeSpace.UnitTests/Agents/SupervisorPriorDecisionLegibilityTests.cs index 3addb0f75..d0b19e4cd 100644 --- a/backend/tests/CodeSpace.UnitTests/Agents/SupervisorPriorDecisionLegibilityTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Agents/SupervisorPriorDecisionLegibilityTests.cs @@ -197,8 +197,31 @@ public void The_live_plan_shows_the_EFFECTIVE_acceptance_check_not_the_amended_a var prompt = LlmSupervisorDecider.BuildUserPromptForTest(Context(plan, approved)); prompt.ShouldContain("AMENDED by a co-signed amendment", customMessage: "the co-signed replacement is the check in force and must be marked as such"); - prompt.ShouldContain("dotnet test", customMessage: "the effective command is what the brain writes its next move against"); - prompt.ShouldNotContain("npm test", customMessage: "the amended-away original must not be shown as live"); + prompt.ShouldContain("""AMENDED by a co-signed amendment: ["dotnet","test"]""", customMessage: "the effective command is what the brain writes its next move against — anchored on the plan line, because the approved card above it quotes the same array and would satisfy a bare match on its own"); + prompt.ShouldNotContain("""["npm","test"]""", customMessage: "the amended-away original must not be shown as live"); + } + + [Fact] + public void The_amendment_card_and_the_plan_line_show_the_amended_check_the_same_way() + { + // The card used to quote its proposed check through string.Join(" ", …), a few words from the plan line's array: the + // model and the human co-signer were told one command two ways, and a pipe in an argument read as a pipeline. + var plan = Plan("""{"goal":"ship","subtasks":[{"id":"s1","title":"Add the parser","instruction":"write the parser","acceptance":{"command":["npm","test"]}}]}"""); + + var card = SupervisorAmendAcceptance.IntoAskHuman(new SupervisorAmendAcceptancePayload + { + SubtaskId = "s1", Reason = "npm is not installed in this repository", + Acceptance = new SupervisorAcceptanceSpec { Command = new[] { "dotnet", "test", "--filter", "Category=Unit|Category=Smoke" } }, + }); + var approved = Ask(2, card.PayloadJson, """{"question":"q","answer":"approve"}"""); + + var prompt = LlmSupervisorDecider.BuildUserPromptForTest(Context(plan, approved)); + + var array = """["dotnet","test","--filter","Category=Unit|Category=Smoke"]"""; + + prompt.ShouldContain($"AMENDED by a co-signed amendment: {array}", customMessage: "the plan line shows the check in force as the argv array"); + prompt.ShouldContain($"Proposed check: {array} {SupervisorAmendAcceptance.AmendMarker}", customMessage: "the approved card shows the SAME array, between its label and the marker — one command, one way"); + prompt.ShouldNotContain("dotnet test --filter", customMessage: "a joined line reads the pipe inside the argument as a pipeline, and cannot show where one argument ends"); } [Fact] @@ -213,7 +236,7 @@ public void A_waived_plan_item_says_WAIVED_and_never_recites_a_check_nothing_gra prompt.ShouldContain("WAIVED by a co-signed amendment"); prompt.ShouldContain("WAIVED is not PASSED", customMessage: "the B2 invariant holds at every door, this one included"); - prompt.ShouldNotContain("npm test", customMessage: "a waived item carries NO oracle — reciting one invents a contract the run does not have"); + prompt.ShouldNotContain("""["npm","test"]""", customMessage: "a waived item carries NO oracle — reciting one invents a contract the run does not have"); } // ── a budget-blocked wave ─────────────────────────────────────────────────────── diff --git a/backend/tests/CodeSpace.UnitTests/Architecture/AcceptanceArgvInventoryTests.cs b/backend/tests/CodeSpace.UnitTests/Architecture/AcceptanceArgvInventoryTests.cs new file mode 100644 index 000000000..fd3f7d81a --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Architecture/AcceptanceArgvInventoryTests.cs @@ -0,0 +1,159 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Text.RegularExpressions; +using CodeSpace.Core.Services.Agents; +using Shouldly; + +namespace CodeSpace.UnitTests.Architecture; + +/// +/// Fail-closed floor for the shape that was pasted five times: the rule for which acceptance argvs can be run at all — +/// a blank first element or a NUL anywhere makes the whole argv unrunnable, and every other element is kept. One +/// statement of it () is only one if nothing else restates it: a private +/// copy that drifts reads the operator's floor differently from the code that runs it, so the launch bakes a command +/// the grade refuses, or the grade runs one the prompt never showed. AcceptanceArgvTests pins what the helper +/// decides; it cannot see a caller that stopped asking it, which is what this pins. +/// +/// Two halves. Every reader that decides whether an argv can run — the two launch projections, the supervisor +/// rehydrate and stop grade, the in-loop Stop hook, the local verifier, the two spec-preview validators — must ask the +/// helper, at every site: each file pins how many times it does, because the rehydrate has four and one of them +/// reverting to a private copy leaves the file still asking the helper three times. And no other file under +/// backend/src, in any assembly, may spell the rule out itself, unless it is named in +/// with the reason: each entry pins a COUNT as well as a reason, so a new copy in +/// an already-named file is as loud as one in a new file. +/// +/// The scan is a text heuristic, not a parser, in the shape of , +/// and it matches per FILE, not per line: a file whose code tests a first element for blankness +/// (IsNullOrWhiteSpace(x[0])) AND tests for a NUL (Contains('\0'), IndexOf('\0')) states the rule, +/// wherever the two clauses sit. A per-line match missed exactly that: PlannerAcceptanceDraft writes the null/NUL +/// clause on one line and the blank-first clause on the next. A copy spelled some other way still slips past the second +/// half and is caught by the first, since the site that holds it no longer asks the helper. +/// +[Trait("Category", "Unit")] +public class AcceptanceArgvInventoryTests +{ + private const string ScannedRoot = "backend/src"; + private const string Helper = "AcceptanceArgv.NormalizeOrNull("; + + /// A first element tested for blankness — the rule's program clause. Any identifier or member path is the argv, so command[0] and spec.Command[0] both match. + private static readonly Regex BlankFirst = new(@"IsNullOrWhiteSpace\(\s*[\w.!?]+\s*\[0\]\s*\)", RegexOptions.Compiled); + + /// A NUL tested for — the rule's token clause, in either literal form and either method the repo writes it with. + private static readonly Regex NulTest = new(@"(Contains|IndexOf)\(\s*['""]\\0['""]", RegexOptions.Compiled); + + /// The readers that decide whether an acceptance argv can run, each through and never a copy of it, with how many sites ask it: the rehydrate's are the operator floor's resolve grade and oracle inventory, a unit's own check, and the stop gate's floor and model check. + private static readonly IReadOnlyDictionary AsksTheHelper = new Dictionary(StringComparer.Ordinal) + { + ["CodeSpace.Core/Services/Agents/InLoopAcceptanceHook.cs"] = 1, + ["CodeSpace.Core/Services/Agents/Workspace/LocalAcceptanceVerifier.cs"] = 1, + ["CodeSpace.Core/Services/Supervisor/SupervisorTurnService.Rehydrate.cs"] = 4, + ["CodeSpace.Core/Services/Tasks/Projection/Builders/SingleAgent/SingleAgentDefinitionBuilder.cs"] = 1, + ["CodeSpace.Core/Services/Tasks/Projection/Builders/Supervisor/SupervisorDefinitionBuilder.cs"] = 1, + ["CodeSpace.Core/Services/Tasks/SpecPreview/TaskSpecAcceptanceAssessment.cs"] = 1, + ["CodeSpace.Core/Services/Tasks/SpecPreview/TaskSpecSourceArgv.cs"] = 1, + }; + + /// The files that still spell the rule out, with how many lines hold its blank-first clause. The helper is the one statement; the planner's payload check is the one reader that cannot ask it. + private static readonly IReadOnlyDictionary StatesTheRuleByDesign = new Dictionary(StringComparer.Ordinal) + { + ["CodeSpace.Core/Services/Agents/AcceptanceArgv.cs"] = (1, "the one statement of the rule."), + ["CodeSpace.Core/Services/Workflows/Planning/PlannerAcceptanceDraft.cs"] = (1, "checks a planner-authored payload and must tell the re-ask WHICH clause failed — a null or NUL element and a blank executable are two separate messages, one per violated clause — and the same method checks file-oracle paths, where ANY blank is wrong. The helper answers only runnable or not, so folding it would make both messages one."), + }; + + private sealed record SourceFile(string Name, IReadOnlyList<(string Text, int Number)> Code); + + private static readonly Lazy> Sources = new(ReadSources); + + [Fact] + public void Every_reader_that_decides_an_argv_can_run_asks_the_helper_at_every_site() + { + var asked = Sources.Value + .Select(file => (file.Name, Calls: file.Code.Sum(line => line.Text.Split(Helper).Length - 1))) + .Where(file => file.Calls > 0) + .ToDictionary(file => file.Name, file => file.Calls, StringComparer.Ordinal); + + asked.ShouldNotBeEmpty("the source scan found no caller of the helper at all — every check here would pass vacuously"); + + var drift = AsksTheHelper.Keys.Union(asked.Keys) + .Where(file => asked.GetValueOrDefault(file) != AsksTheHelper.GetValueOrDefault(file)) + .OrderBy(file => file, StringComparer.Ordinal) + .Select(file => $"{file} — expected {AsksTheHelper.GetValueOrDefault(file)} site(s) asking the helper, found {asked.GetValueOrDefault(file)}") + .ToList(); + + drift.ShouldBeEmpty( + $"the sites asking {nameof(AcceptanceArgv)}.{nameof(AcceptanceArgv.NormalizeOrNull)} changed. A new reader or site belongs in {nameof(AsksTheHelper)}; " + + "a LOST one is a private copy of the rule again, which can drift from the helper unseen — ask the helper instead:\n " + string.Join("\n ", drift)); + } + + [Fact] + public void No_other_file_states_the_rule_again() + { + var found = RuleStatements(); + + found.ShouldNotBeEmpty("the source scan found no statement of the rule at all — every check here would pass vacuously"); + + var offenders = found + .Where(file => !StatesTheRuleByDesign.ContainsKey(file.Key) || StatesTheRuleByDesign[file.Key].Lines != file.Value.Count) + .Select(file => $"{file.Key}:{string.Join(",", file.Value)} — {(StatesTheRuleByDesign.TryGetValue(file.Key, out var e) ? $"named for {e.Lines}, found {file.Value.Count}" : "not named")}") + .OrderBy(name => name, StringComparer.Ordinal) + .ToList(); + + offenders.ShouldBeEmpty( + $"these spell out which acceptance argvs can run — a blank first element and a NUL — instead of asking {nameof(AcceptanceArgv)}.{nameof(AcceptanceArgv.NormalizeOrNull)}. " + + $"A copy drifts: ask the helper, or name the file in {nameof(StatesTheRuleByDesign)} with the reason it must restate the rule:\n " + string.Join("\n ", offenders)); + } + + [Fact] + public void The_named_files_do_not_rot() + { + var found = RuleStatements(); + + foreach (var (file, entry) in StatesTheRuleByDesign) + { + found.ShouldContainKey(file, $"named file '{file}' no longer states the rule — remove it from {nameof(StatesTheRuleByDesign)}"); + entry.Reason.ShouldNotBeNullOrWhiteSpace($"named file '{file}' must say why it restates the rule"); + } + } + + /// Every file under the scanned root whose code tests a NUL AND a first element's blankness, with the lines that hold the blank-first clause. Per file, so a copy split across lines is still one statement of the rule. Comment lines are prose about the rule, not a statement of it. + private static Dictionary> RuleStatements() + { + var sites = new Dictionary>(StringComparer.Ordinal); + + foreach (var file in Sources.Value.Where(file => file.Code.Any(line => NulTest.IsMatch(line.Text)))) + { + var lines = file.Code.Where(line => BlankFirst.IsMatch(line.Text)).Select(line => line.Number).ToList(); + + if (lines.Count > 0) sites[file.Name] = lines; + } + + return sites; + } + + private static IReadOnlyList ReadSources() + { + var root = Path.Combine(RepositoryRoot(), ScannedRoot); + + return Directory.EnumerateFiles(root, "*.cs", SearchOption.AllDirectories) + .Select(file => (Path: file, Name: Path.GetRelativePath(root, file).Replace(Path.DirectorySeparatorChar, '/'))) + .Where(file => !file.Name.Split('/').Any(segment => segment is "obj" or "bin")) + .Select(file => new SourceFile(file.Name, CodeLines(file.Path))) + .ToList(); + } + + private static IReadOnlyList<(string Text, int Number)> CodeLines(string file) => File.ReadAllLines(file) + .Select((text, index) => (Text: text.TrimStart(), Number: index + 1)) + .Where(line => !line.Text.StartsWith("//", StringComparison.Ordinal) && !line.Text.StartsWith('*')) + .ToList(); + + private static string RepositoryRoot() + { + var dir = new DirectoryInfo(AppContext.BaseDirectory); + + while (dir is not null && !Directory.Exists(Path.Combine(dir.FullName, "backend"))) dir = dir.Parent; + + return dir?.FullName ?? throw new InvalidOperationException($"repository root not found walking up from {AppContext.BaseDirectory}"); + } +} diff --git a/backend/tests/CodeSpace.UnitTests/Tasks/TaskSpecAcceptanceEvidenceTests.cs b/backend/tests/CodeSpace.UnitTests/Tasks/TaskSpecAcceptanceEvidenceTests.cs index 6ba0e0a56..2e42e9d41 100644 --- a/backend/tests/CodeSpace.UnitTests/Tasks/TaskSpecAcceptanceEvidenceTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Tasks/TaskSpecAcceptanceEvidenceTests.cs @@ -93,6 +93,24 @@ public void Invalid_argv_cannot_be_adopted_even_when_the_review_reports_support( result.AcceptanceCriteria.ShouldBe(["The report satisfies the user's requirements."]); } + [Theory] + [InlineData(256, true)] // exactly at the bound — adopted + [InlineData(257, false)] // one over — refused as malformed, whatever the review says + public void A_proposed_argv_is_adopted_up_to_256_tokens_and_refused_beyond(int tokens, bool adopted) + { + var result = TaskSpecCompiler.ToSuggestion(Compilation() with { AcceptanceChecks = Enumerable.Repeat("custom-audit", tokens).ToArray() }, Context(TaskSpecRepositoryState.NotRequested), Review("user-explicit", "supported", "goal", Goal))!; + + if (adopted) + { + result.AcceptanceProposal!.Status.ShouldBe(TaskSpecEvidenceStatus.Supported, "a well-formed argv at the bound passes the shared rule and the bound together"); + result.AcceptanceChecks.Count.ShouldBe(tokens); + return; + } + + result.AcceptanceChecks.ShouldBeEmpty(); + result.AcceptanceProposal!.Reason.ShouldContain("malformed", customMessage: "the bound refuses it as malformed, not for some other reason"); + } + [Fact] public void Legacy_wire_json_leaves_evidence_and_usage_unknown_and_new_proposal_roundtrips_without_a_verified_claim() { diff --git a/backend/tests/CodeSpace.UnitTests/Tasks/TaskSpecSourceBoundArgvTests.cs b/backend/tests/CodeSpace.UnitTests/Tasks/TaskSpecSourceBoundArgvTests.cs index 3d8c61db6..d8d1e9235 100644 --- a/backend/tests/CodeSpace.UnitTests/Tasks/TaskSpecSourceBoundArgvTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Tasks/TaskSpecSourceBoundArgvTests.cs @@ -43,6 +43,20 @@ public void An_invalid_reference_cannot_fall_back_to_a_generated_command_even_wh suggestion.AcceptanceProposal.Reason.ShouldContain("source reference"); } + [Theory] + [InlineData(256, true)] // exactly at the bound — a literal reference of this size resolves + [InlineData(257, false)] // one over — the reference is refused, whatever the reviewer says + public void A_literal_reference_resolves_up_to_256_tokens_and_no_further(int tokens, bool resolves) + { + // Short tokens, so the quoted goal stays inside the review's 1000-character citation limit at the bound: a program, then blank arguments (which the rule keeps). + var quote = JsonSerializer.Serialize(Enumerable.Repeat("check", 1).Concat(Enumerable.Repeat("", tokens - 1))); + var context = Context("Use precisely this validation argv: " + quote); + var suggestion = TaskSpecCompiler.ToSuggestion(Compilation(new("goal", quote, "json-argv")), context, Review(context, "supported"))!; + + (suggestion.AcceptanceChecks.Count == tokens).ShouldBe(resolves, "the bound is the shared rule's companion at this site: a reference over 256 tokens is refused as malformed, one at 256 is adopted"); + if (!resolves) suggestion.AcceptanceProposal!.Reason.ShouldContain("source reference"); + } + [Fact] public void Literal_integrity_does_not_override_an_independent_negation_assessment() { diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/SupervisorDefinitionBuilderTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/SupervisorDefinitionBuilderTests.cs index ed2a05972..9d72975d1 100644 --- a/backend/tests/CodeSpace.UnitTests/Workflows/SupervisorDefinitionBuilderTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Workflows/SupervisorDefinitionBuilderTests.cs @@ -79,7 +79,7 @@ public void The_advertised_operator_adapter_bakes_exact_argv_and_omits_a_blank_e Builder.OperatorAcceptance.GradingKind.ToString().ShouldBe("TestsPass"); Builder.Build(Context()).Nodes.Single(n => n.Id == "sup").Config.TryGetProperty("acceptanceChecks", out _).ShouldBeFalse("no floor ⇒ key omitted (byte-identical)"); - Builder.Build(Context(acceptanceChecks: new[] { " ", "must-not-become-executable" })).Nodes.Single(n => n.Id == "sup").Config.TryGetProperty("acceptanceChecks", out _).ShouldBeFalse("an all-blank floor collapses to omitted"); + Builder.Build(Context(acceptanceChecks: new[] { " ", "must-not-become-executable" })).Nodes.Single(n => n.Id == "sup").Config.TryGetProperty("acceptanceChecks", out _).ShouldBeFalse("a floor whose first element is blank has no program, so it collapses to omitted"); } [Fact]