fix(cutting): keep plan installation behind the verified service

Review of the atomic Apply found two commit-boundary gaps:

- The plan installer checked only root program references, so a public
  caller could install a payload whose subprograms alias another live
  (even locked) part's program, or share settings between parts. The
  installer and its payload types are now internal; CuttingPlanService.Apply,
  which installs owned copies of independently replayed proposals, is the
  only public path.
- A part placed on two plates in one scope was planned once per plate; one
  plate's install replaced the program the other plate verified as fixed.
  A part repeated anywhere in the scope is now invalid input.
This commit is contained in:
aj committed 2026-10-05 00:25:43 -04:00
1 parent d37b38622f
commit b9b475ab77
2 files changed
+55 -6

No files matched your search

@@ -8,7 +8,7 @@ using OpenNest.Geometry;
namespace OpenNest.CNC.CuttingPlanning;
/// <summary>An owned planned program for one unlocked part; ownership passes to the part on Apply.</summary>
public sealed class PlannedPartProgram
internal sealed class PlannedPartProgram
{
public PlannedPartProgram(Part part, Program program, CuttingParameters parameters)
{
@@ -23,7 +23,7 @@ public sealed class PlannedPartProgram
}
/// <summary>The verified order and planned programs for one plate, bound to its captured state.</summary>
public sealed class PlateCuttingPlan
internal sealed class PlateCuttingPlan
{
public PlateCuttingPlan(PlateCuttingState expected, IEnumerable<Part> order,
IEnumerable<PlannedPartProgram> programs = null)
@@ -75,10 +75,12 @@ public sealed class CuttingCommitResult
/// Installs verified cutting plans for a whole scope at once. Nothing is searched, emitted,
/// rotated or regenerated here: inputs are validated and checked for freshness, bounds are
/// staged, then order and programs are installed synchronously and published once per plate.
/// Internal: it checks root program references only, so payloads must be owned copies of
/// independently replayed proposals. CuttingPlanService.Apply is the public entry point.
/// </summary>
public static class CuttingPlanCommit
internal static class CuttingPlanCommit
{
public static CuttingCommitResult Apply(IEnumerable<PlateCuttingPlan> plans, CancellationToken token = default) =>
internal static CuttingCommitResult Apply(IEnumerable<PlateCuttingPlan> plans, CancellationToken token = default) =>
Apply(plans, token, null);
// beforeInstall is a test seam that runs inside the install boundary, before each program.
@@ -113,6 +115,7 @@ public static class CuttingPlanCommit
var staged = new List<Staged>(scope.Length);
var targets = new HashSet<Part>(ReferenceEqualityComparer.Instance);
var members = new HashSet<Part>(ReferenceEqualityComparer.Instance);
var installed = new HashSet<Program>(ReferenceEqualityComparer.Instance);
var live = new HashSet<Program>(scope.SelectMany(p => p.Expected.Order).Select(p => p.Program),
ReferenceEqualityComparer.Instance);
@@ -128,11 +131,14 @@ public static class CuttingPlanCommit
{
return Invalid(plate, "The planned order is not exactly the plate's current parts.");
}
var members = new HashSet<Part>(order, ReferenceEqualityComparer.Instance);
// A part on two plates would let one plate's install change the other's verified program.
if (order.Any(part => !members.Add(part)))
return Invalid(plate, "A part appears on more than one plate in the commit scope.");
var onPlate = new HashSet<Part>(order, ReferenceEqualityComparer.Instance);
var programs = new List<(Part, Program, Box, CuttingParameters)>();
foreach (var planned in plan.Programs)
{
if (planned?.Part == null || !members.Contains(planned.Part) || !targets.Add(planned.Part))
if (planned?.Part == null || !onPlate.Contains(planned.Part) || !targets.Add(planned.Part))
return Invalid(plate, "Planned programs must target distinct parts of their own plate.");
if (planned.Part.LeadInsLocked)
return Invalid(plate, "A locked part's program is retained exactly and cannot be replaced.");
@@ -179,6 +179,49 @@ public class CuttingPlanCommitTests
Assert.False(ProgramContent.Equal(With("X"), With("x")));
}
[Fact]
public void Apply_PartSharedByTwoPlates_IsRefusedWithoutChange()
{
var parameters = ExplicitContourTests.Parameters();
var clean = LeadPathValidationTests.Rectangle(0, 0, 2, 2);
var prepared = PreparedContours.Capture(clean, parameters);
var shared = new Part(new Drawing("same", clean), new Vector(5, 5));
Assert.True(shared.RestoreLeadInProgram(prepared.Emit([prepared.ClosestEntry(0, new Vector(3, 1))]), false));
var nest = new Nest();
var regenerated = nest.CreatePlate();
var fixedPlate = nest.CreatePlate();
regenerated.Parts.Add(shared);
fixedPlate.Parts.Add(shared);
var results = new[]
{
CuttingPlanService.Plan(new CuttingPlanRequest(regenerated, confirmedParameters: parameters)),
CuttingPlanService.Plan(new CuttingPlanRequest(fixedPlate))
};
Assert.All(results, r => Assert.Equal(CuttingPlanStatus.Ready, r.Status));
Assert.True(results[0].ProposedOrder[0].IsRegenerated);
var program = shared.Program;
var states = new[] { regenerated, fixedPlate }.Select(p => PlateCuttingState.Capture(p)).ToArray();
var commit = CuttingPlanService.Apply(results);
// Installing through one plate would silently replace the other plate's fixed program.
Assert.Equal(CuttingCommitStatus.InvalidInput, commit.Status);
Assert.Same(program, shared.Program);
Assert.All(states, s => Assert.True(s.IsCurrent(), s.Difference()));
}
[Fact]
public void CommitInstaller_IsOnlyReachableThroughTheVerifiedService()
{
// The installer validates root references only; owned, verified payloads come from
// CuttingPlanService.Apply. Public access would accept nested aliases of live programs.
Assert.False(typeof(CuttingPlanCommit).IsPublic);
Assert.False(typeof(PlateCuttingPlan).IsPublic);
Assert.False(typeof(PlannedPartProgram).IsPublic);
Assert.True(typeof(CuttingPlanService).GetMethod(nameof(CuttingPlanService.Apply),
[typeof(IEnumerable<CuttingPlanResult>), typeof(CancellationToken)])!.IsPublic);
}
[Fact]
public void Apply_LaterPlateStale_AppliesNothingAnywhere()
{