Stop the test suite taking the reader's clipboard

5d376a2ac912 · AtlantisPleb · · parent 16e901594594

Stop the test suite taking the reader's clipboard

A path pasted from the clipboard did not exist, and the file that did exist had
a different name four seconds earlier. The clipboard was not the reader's.

`/export` copies its path to the clipboard, and two tests called the real
exporter: one wrote `<stamp>-repo-atif.json` into `~/.openagents/exports`, took
the clipboard, and deleted the file it had just named. A test run four seconds
after a real export replaced that export's path with a path to nothing. The
reader pasted it and was told, correctly, that it did not exist.

The clipboard is now opt-out and both tests opt out. The session takes an
export directory so its test writes into its own, and asserts it wrote there
rather than deleting afterwards — a suite that has to clean up after itself in
someone's directory should not have been writing there.

Checked by comparing the clipboard across a full run: unchanged. One leftover
`repo-atif.json` from a run whose cleanup did not fire is removed.

416 tests pass.

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-export.ts
  • modified packages/openagents-cli/src/coder-session.ts
  • modified packages/openagents-cli/test/coder-export.test.ts
  • modified packages/openagents-cli/test/coder-session.test.ts

Diff

4 files changed, +52 -7

packages/openagents-cli/src/coder-export.ts modified +11 -1

@@ -235,6 +235,15 @@ export function exportTrajectory(

235 235
    readonly version: string;
236 236
    readonly now?: Date | undefined;
237 237
    readonly directory?: string | undefined;
238
    /**
239
     * Whether the path goes to the system clipboard. On by default.
240
     *
241
     * Off for anything that is not a person exporting: a test that took the
242
     * clipboard replaced a reader's own export path with one pointing at a file
243
     * the test then deleted, and the reader pasted it and was told the path did
244
     * not exist.
245
     */
246
    readonly copy?: boolean | undefined;
238 247
  },
239 248
): ExportedTrajectory {
240 249
  const at = options.now ?? new Date();

@@ -276,5 +285,6 @@ export function exportTrajectory(

276 285
  const path = join(directory, fileName(snapshot.repository, at));
277 286
  writeFileSync(path, `${JSON.stringify(document, undefined, 2)}\n`, "utf8");
278 287
279
  return { path, copied: copyToClipboard(path), steps: steps.length };
288
  const copy = options.copy ?? true;
289
  return { path, copied: copy && copyToClipboard(path), steps: steps.length };
280 290
}
packages/openagents-cli/src/coder-session.ts modified +10

@@ -374,6 +374,13 @@ export class CoderSession {

374 374
     * buys nothing.
375 375
     */
376 376
    private readonly standing?: string,
377
    /**
378
     * Where `/export` writes, and whether it takes the clipboard.
379
     *
380
     * For tests. A suite that wrote into the reader's own export directory and
381
     * took their clipboard is a suite that changed the machine it was checking.
382
     */
383
    private readonly exports?: { readonly directory: string },
377 384
  ) {
378 385
    // A child reporting progress has to reach the renderer, and the renderer
379 386
    // subscribes to the session rather than to the registry, so the session

@@ -528,6 +535,9 @@ export class CoderSession {

528 535
          model: this.source.modelId ?? this.source.model,
529 536
          toolDefinitions: this.source.toolDefinitions?.(),
530 537
          version: VERSION,
538
          ...(this.exports === undefined
539
            ? {}
540
            : { directory: this.exports.directory, copy: false }),
531 541
        });
532 542
        this.notice(
533 543
          `Exported ${String(written.steps)} step${written.steps === 1 ? "" : "s"} as ATIF to ${written.path}` +
packages/openagents-cli/test/coder-export.test.ts modified +21

@@ -33,6 +33,8 @@ const write = (entries: ReadonlyArray<CoderEntry>) => {

33 33
    version: "0.3.5",
34 34
    now: new Date(AT),
35 35
    directory,
36
    // A suite must not take the reader's clipboard.
37
    copy: false,
36 38
  });
37 39
  return {
38 40
    result,

@@ -214,6 +216,25 @@ describe("exporting a conversation as ATIF", () => {

214 216
    expect(document["final_metrics"]).toEqual({ total_steps: 1 });
215 217
  });
216 218
219
220
  it("takes the clipboard only when asked to", () => {
221
    const directory = mkdtempSync(join(tmpdir(), "coder-export-"));
222
223
    // The default is a person exporting, and they want the path. Anything else
224
    // — a test above all — must say so: a suite that took the clipboard once
225
    // replaced a reader's own export path with one pointing at a file the suite
226
    // then deleted, and the reader pasted it and was told it did not exist.
227
    const quiet = exportTrajectory(snapshot([entry({ role: "you", text: "hi" })]), {
228
      model: "m",
229
      version: "0",
230
      now: new Date(AT),
231
      directory,
232
      copy: false,
233
    });
234
235
    expect(quiet.copied).toBe(false);
236
  });
237
217 238
  it("writes one file per export, named so they sort by time", () => {
218 239
    const { directory } = write([entry({ role: "you", text: "one" })]);
219 240
packages/openagents-cli/test/coder-session.test.ts modified +10 -6

@@ -1,4 +1,6 @@

1
import { rmSync } from "node:fs";
1
import { mkdtempSync } from "node:fs";
2
import { tmpdir } from "node:os";
3
import { join } from "node:path";
2 4
import { describe, expect, it } from "vitest";
3 5
4 6
import {

@@ -410,7 +412,10 @@ describe("the /export command", () => {

410 412
        prompts.push(prompt);
411 413
      },
412 414
    };
413
    const session = new CoderSession(reply, "repo", "main");
415
    // Its own directory, and no clipboard: a test that writes where a reader
416
    // exports, and takes their clipboard, changes the machine it is checking.
417
    const directory = mkdtempSync(join(tmpdir(), "coder-session-export-"));
418
    const session = new CoderSession(reply, "repo", "main", undefined, undefined, { directory });
414 419
415 420
    await session.submit("/export");
416 421

@@ -420,10 +425,9 @@ describe("the /export command", () => {

420 425
    expect(prompts).toEqual([]);
421 426
    expect(turns).toBe(0);
422 427
423
    // The wiring writes a real file. Take it away again: a test suite should
424
    // not leave anything behind in the directory a person exports into.
425
    const written = /as ATIF to (\S+\.json)/.exec(entries[1]?.text ?? "")?.[1];
426
    if (written !== undefined) rmSync(written, { force: true });
428
    // Written into the test's own directory, so there is nothing to clean out
429
    // of the reader's.
430
    expect(entries[1]?.text).toContain(directory);
427 431
  });
428 432
});
429 433

This page updates live while a promote is in flight · changelog