Leave the skills screen on escape

737b03d61372 · AtlantisPleb · 5d ago · parent 0607b7dfa2d9

Leave the skills screen on escape

Escape did not return from `/skills`. Ctrl+C did, which is why the screen was a
trap rather than obviously broken: one way out worked.

The two keys took different paths. Ctrl+C is one byte with no ambiguity and the
screen handled it where it arrived. A bare escape might be the start of a
sequence, so the interface holds it for a window and releases it through
`onEscape` when nothing follows -- and `onEscape` knew only about the chat, so a
lone escape asked the session to interrupt a reply that was not running and left
the screen up.

`onEscape` now leaves the screen, and the screen's own handler defers to it, so
both the byte recognised in its chunk and the byte released by the timer take
one path out.

The test that missed this sent another key immediately after the escape, which
resolves it inside the same chunk and never starts the timer. The new one sends
a lone escape and waits out the window, which is what a terminal does. It fails
without the fix.

Deploy story

What this commit did to the running system — joined from the forge receipt chain, the part a commit page elsewhere cannot show.

Not deployed through the forge lane

No push, promotion, build, or deploy receipt references this commit (receipts are scanned over a bounded recent window). Changes shipped by full node replacement carry their proof in the release gate receipt instead.

Changed files

modified packages/openagents-cli/src/coder-ui.ts
modified packages/openagents-cli/test/coder-ui.test.ts

Diff

2 files changed, +42 -2


    /** A lone escape: interrupt if there is something to interrupt, else clear. */
    const onEscape = () => {
      // Every settled bare escape lands here, whether it was recognised in the
      // chunk it arrived in or held for the window and released by the timer.
      // Leaving the screen out of this path is what made escape work in a test,
      // where another key follows immediately, and not in a terminal, where a
      // lone escape byte is all that ever arrives.
      if (screen === "skills") {
        screen = "chat";
        painted = [];
        render();
        return;
      }
      if (!session.interrupt()) composer = "";
      render();
    };

            }
            index += sequence.length;
            if (sequence === "\x1b") {
              screen = "chat";
              painted = [];
              onEscape();
              continue;
            } else if (sequence === "\x1b[A" || sequence === "\x1bOA") {
              skillRow = Math.max(0, skillRow - 1);
            } else if (sequence === "\x1b[B" || sequence === "\x1bOB") {

523	523		`await screenUnderTest.running;`
524	524		`});`
525	525
	526	+
	527	+	`it("returns on a lone escape, the way a terminal sends one", async () => {`
	528	+	`const screenUnderTest = await open(selection());`
	529	+	`expect(screenUnderTest.rows().join("\n")).toContain("Skills");`
	530	+
	531	+	`// One escape byte and nothing after it. The interface holds a bare escape`
	532	+	`// for its window in case a sequence follows, so this only leaves the screen`
	533	+	`// once the window has passed -- which is the case a test that sends another`
	534	+	`// key immediately never exercises, and the one every terminal sends.`
	535	+	`screenUnderTest.stdin.emit("data", "\x1b");`
	536	+	`await new Promise((resolve) => setTimeout(resolve, 80));`
	537	+
	538	+	`expect(screenUnderTest.rows().join("\n")).not.toContain("space toggles");`
	539	+
	540	+	`screenUnderTest.stdin.emit("data", "\x04");`
	541	+	`await screenUnderTest.running;`
	542	+	`});`
	543	+
	544	+	`it("returns on ctrl+c without ending the session", async () => {`
	545	+	`const screenUnderTest = await open(selection());`
	546	+
	547	+	`screenUnderTest.stdin.emit("data", "\x03");`
	548	+
	549	+	`expect(screenUnderTest.rows().join("\n")).not.toContain("space toggles");`
	550	+
	551	+	`screenUnderTest.stdin.emit("data", "\x04");`
	552	+	`await screenUnderTest.running;`
	553	+	`});`
	554	+
526	555		`it("says so when the workspace has no skills", async () => {`
527	556		`const stdin = new FakeIn();`
528	557		`const stdout = new FakeOut();`

685	685
686	686		`/** A lone escape: interrupt if there is something to interrupt, else clear. */`
687	687		`const onEscape = () => {`
	688	+	`// Every settled bare escape lands here, whether it was recognised in the`
	689	+	`// chunk it arrived in or held for the window and released by the timer.`
	690	+	`// Leaving the screen out of this path is what made escape work in a test,`
	691	+	`// where another key follows immediately, and not in a terminal, where a`
	692	+	`// lone escape byte is all that ever arrives.`
	693	+	`if (screen === "skills") {`
	694	+	`screen = "chat";`
	695	+	`painted = [];`
	696	+	`render();`
	697	+	`return;`
	698	+	`}`
688	699		`if (!session.interrupt()) composer = "";`
689	700		`render();`
690	701		`};`

732	743		`}`
733	744		`index += sequence.length;`
734	745		`if (sequence === "\x1b") {`
735		-	`screen = "chat";`
736		-	`painted = [];`
	746	+	`onEscape();`
	747	+	`continue;`
737	748		`} else if (sequence === "\x1b[A" \|\| sequence === "\x1bOA") {`
738	749		`skillRow = Math.max(0, skillRow - 1);`
739	750		`} else if (sequence === "\x1b[B" \|\| sequence === "\x1bOB") {`