mirror of
https://github.com/software-mansion/argent.git
synced 2026-09-14 19:27:14 +08:00
Ignacy Łątka
9ce76824f6
## Defect
`VariantProposalStore`
(packages/tool-server/src/utils/variant-proposals.ts) could leave a
round in the `completed && !consumed` state, and a variant proposed in
that state was silently dropped from the next selection outcome.
`submitSelection()` sets `completed = true, consumed = false`, and only
sets `consumed = true` when there is a parked waiter to settle
(`toSettle.length > 0`). When the preview window POSTs a selection while
no `await_user_selection` is currently parked (the window lingers open
after a submit, so the user can complete a selection during the next
proposing phase, or after a pending-timeout before the agent re-awaits),
the round stays `completed && !consumed`.
The next `proposeVariant()` for a NEW element called
`autoRollIfConsumed()`, which only reset the store when `completed &&
consumed`. So it did not roll: the new element was appended to the
already-completed round. The subsequent `awaitSelection()` then hit the
fast path `if (this.completed && !this.consumed)` and returned the
frozen `lastOutcome` captured at submit time — which predates the new
element. `proposeVariant` reported success (`totalElements` incremented,
"staged"), but the element never appeared in any outcome and was never
presented for selection.
### Repro (dropped element)
- `proposeVariant("Alpha")` -> `submitSelection(Alpha)` with no awaiter
parked -> `proposeVariant("Beta")` (returns `totalElements: 2`) ->
`await awaitSelection()` resolves a completed outcome whose
`selections`/`unselected` contain "Alpha" but NOT "Beta". Beta is
silently dropped.
## Fix
Roll a fresh round whenever a proposal lands on an already-`completed`
round, not only when `completed && consumed`. The helper (renamed
`autoRollIfCompleted`) now calls `reset()` on `this.completed`, so a
post-completed proposal begins a new round and the new element is staged
into it. The legitimate submit-then-await fast path (submit with no
waiter, then await with no new proposal) is unchanged — `completed`
stays true with no intervening propose, so the frozen outcome is still
handed back and the round consumed.
With the fix, the repro above starts round 2 with "Beta" as a live
proposal; the next await presents Beta (as a pending proposal) instead
of dropping it.
## Tests
- New regression test
`packages/tool-server/test/variant-store-propose-after-submit.test.ts` —
fails before the fix (Beta dropped from the completed outcome), passes
after (Beta presented in the fresh round).
- All existing variant-proposals tests pass, including the
submit-then-await fast path and the await-then-submit consumed path.
- Full tool-server suite: 1622 passed. `tsc --build` clean; eslint 0
problems on changed files.
40 lines
1.8 KiB
TypeScript
40 lines
1.8 KiB
TypeScript
import { describe, it, expect } from "vitest";
|
|
import { VariantProposalStore } from "../src/utils/variant-proposals";
|
|
|
|
// A CLI (`argent lens`) session boundary is a fresh start on BOTH ends: a
|
|
// proposal staged during a session that the user never submitted must NOT bleed
|
|
// into a subsequent NON-CLI round's outcome once the session ends.
|
|
describe("VariantProposalStore — CLI session boundary resets stale round state", () => {
|
|
it("an unsubmitted lens proposal does not bleed into a later non-CLI outcome", async () => {
|
|
const s = new VariantProposalStore();
|
|
s.setCliSession(true, [{ id: "x", name: "X" }]);
|
|
s.proposeVariant({ element: "Alpha", variant: { name: "a", summary: "s" } }); // staged, never submitted
|
|
s.setCliSession(false); // session ends
|
|
|
|
// A later NON-CLI flow proposes a different element and awaits.
|
|
const beta = s.proposeVariant({ element: "Beta", variant: { name: "b", summary: "s" } });
|
|
s.submitSelection({ selections: [{ elementId: beta.elementId, variantId: beta.variantId }] });
|
|
const out = await s.awaitSelection({ timeoutMs: 5 });
|
|
|
|
expect(out.status).toBe("completed");
|
|
if (out.status === "completed") {
|
|
const names = [
|
|
...out.selections.map((x) => x.element),
|
|
...out.unselected.map((x) => x.element),
|
|
];
|
|
expect(names).toContain("Beta");
|
|
// Alpha belonged to the ended lens session — it must be gone.
|
|
expect(names).not.toContain("Alpha");
|
|
}
|
|
});
|
|
|
|
it("ending a clean CLI session does not gratuitously bump the round", () => {
|
|
const s = new VariantProposalStore();
|
|
s.setCliSession(true);
|
|
s.setCliSession(false); // nothing staged -> no reset
|
|
expect(s.proposeVariant({ element: "Foo", variant: { name: "f", summary: "s" } }).round).toBe(
|
|
1
|
|
);
|
|
});
|
|
});
|