Files
software-mansion__argent/packages/tool-server/test/flows/flow-skill-docs.test.ts
Hubert Gancarczyk 74cddedf50 docs(skills): split argent-create-flow into references and add argent-qa-flows (#730)
> Now rebased directly onto `main`. #728 and #574 have landed, so this
PR carries only its own 13 documentation commits; it still does not sit
on the #715-#726 chain.
>
> Guidance describing behaviour owned by that chain is therefore **not**
carried here: the recorder's refusal of a vacuous `hidden` and of
`await-screen-idle` (#724), its cross-tree re-probe warning (#717), the
per-step `recorded` line (#715), and `type:` failing on unconfirmed
focus (#726). Each of those paragraphs states what this base actually
does instead; they are worth restoring when that chain lands.

## `argent-create-flow`: ~5,000 words to ~900 plus three references

Every word of that skill loaded on every invocation, whether the task
was replaying a two-step fragment or authoring a full regression test.
The core skill now routes to three references read on demand:

- **`live-authoring.md`** - recording a walkthrough
- **`flow-yaml.md`** - the file format and selector vocabulary
- **`reliability-and-recovery.md`** - what to do when a step or a replay
goes wrong

Nothing is dropped in the split - it moves behind a router.

## New skill: `argent-qa-flows`

Turning a **test case, ticket, or acceptance criteria** into a
repeatable regression test is a different job from recording a path
worth replaying, and it was being done by a skill that never asks for
acceptance criteria. It orchestrates `argent-create-flow` as its engine,
records the first walkthrough live, requires every requested screen and
state to be proved with stable evidence, and completes only after the
unchanged flow passes **twice consecutively**.

## Proving a navigation

Spelled out here for the first time, as two element-level checks rather
than a route read:

```yaml
- await: { visible: { id: settings-screen } } # identity: WHICH screen
- await: { idle: true } # readiness: it stopped moving
```

Neither implies the other - a dropped tap leaves the source screen
perfectly idle, and the destination's elements enter the tree while the
transition still animates over them. Both belong after every screen
change, including the one `launch:` performs.

## Routing

The three skills were being confused by name alone, so the rules now
draw the line:

| you want | skill |
| --- | --- |
| a one-off interactive check, nothing saved | `argent-test-ui-flow` |
| a saved, replayable path | `argent-create-flow` |
| a saved test with acceptance criteria and two-pass proof |
`argent-qa-flows` |

"Record a flow" also stops being ambiguous with **screen recording**,
which is video.

## Tests

Every bundled skill's YAML frontmatter is parsed in a test, so a
malformed new skill fails here rather than at install time. The `idle`
doc guards that came with the warn-instead-of-fail change follow the
prose into `references/flow-yaml.md`: they pin the documented defaults,
settle cost and minimum timeout to the constants the parser enforces,
and hold the tool description and the reference to the same account of
what `idle` does. All 43 anchored internal links resolve.

---------

Co-authored-by: Hubert Gancarczyk <claude-hubert.gancarczyk@swmansion.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-10 14:41:23 +02:00

155 lines
7.4 KiB
TypeScript

import { describe, it, expect } from "vitest";
import { readFileSync } from "node:fs";
import * as path from "node:path";
import type { Registry } from "@argent/registry";
import {
IDLE_DEFAULT_STABLE_FOR_MS,
IDLE_DEFAULT_TIMEOUT_MS,
IDLE_MIN_STILL_INTERVALS,
IDLE_POLL_MS,
IDLE_SETTLE_SPAN_MS,
idleMinimumTimeoutMs,
parseFlow,
} from "../../src/tools/flows/flow-utils";
import { createRunFlowTool } from "../../src/tools/flows/flow-run";
/**
* Keep the core skill's scope routing concise while guarding the linked
* reference examples against parser drift.
*/
const SKILL = path.resolve(__dirname, "../../../skills/skills/argent-create-flow/SKILL.md");
const FLOW_YAML = path.resolve(
__dirname,
"../../../skills/skills/argent-create-flow/references/flow-yaml.md"
);
/**
* The three surfaces that quote the number of `idle` warnings instead of
* listing them. They cite the reference rather than restating it, so a warning
* added to the list leaves all three saying the wrong count — which is exactly
* how "five different warnings" survived a sixth being added.
*/
const WARNING_COUNT_CITATIONS = [
path.resolve(__dirname, "../../../skills/skills/argent-create-flow/references/live-authoring.md"),
path.resolve(
__dirname,
"../../../skills/skills/argent-create-flow/references/reliability-and-recovery.md"
),
path.resolve(__dirname, "../../../skills/skills/argent-qa-flows/SKILL.md"),
];
/**
* The text between two markers. BOTH are asserted: `split` on an absent
* separator returns a single-element array, so an unchecked `end` would widen
* the section silently to EOF — and the callers below then count snippets from
* the rest of the file instead of failing on the renamed heading.
*/
function between(file: string, start: string, end: string): string {
const after = readFileSync(file, "utf8").split(start)[1];
expect(after, `${start} is missing from ${file}`).toBeDefined();
const section = after!.split(end)[0]!;
expect(section, `${end} is missing from ${file} after ${start}`).not.toBe(after);
return section;
}
describe("create-flow selector-scope docs", () => {
it("keeps the core skill concise and routes every relation", () => {
const section = between(SKILL, "### Flow-only selector scopes", "\n## Workflow");
expect(section).toContain("`within`");
expect(section).toContain("`after`");
expect(section).toContain("`next`");
expect(section).toContain("references/flow-yaml.md#relational-scopes");
});
it("keeps the reference examples parsable", () => {
const section = between(FLOW_YAML, "### Relational scopes", "\n## Directives");
const snippets = [
...section.matchAll(/^\s*- ((?:tap|assert|await|type|scroll-to):.+?)(?:\s+#.*)?$/gm),
].map((m) => m[1]!);
expect(snippets).toHaveLength(3);
for (const snippet of snippets) {
expect(() => parseFlow(`steps:\n - ${snippet}\n`), snippet).not.toThrow();
}
});
});
// The `idle` account moved out of SKILL.md into the flow-yaml reference, so
// these read it there. They are otherwise the guards that came with the
// warn-instead-of-fail change: the two agent-facing descriptions of `idle`
// have to agree with what it does, and the numbers the prose quotes have to be
// the ones the parser enforces.
describe("create-flow idle docs", () => {
it("the flow-execute description and the reference agree that idle warns rather than fails", () => {
const description = createRunFlowTool({} as unknown as Registry).description;
expect(description).toContain("idle: true");
expect(description).toMatch(/never\s+fails a run/);
expect(description).not.toMatch(/FAILS on timeout/i);
const reference = readFileSync(FLOW_YAML, "utf8");
expect(reference).toContain("It **never fails a run.**");
// The one outcome that does stop a run is the window, never the app.
expect(reference).toMatch(/Only a tree source that cannot be read stops the run/);
// Both surfaces have to carry that caveat: the description is what an
// authoring agent reads, and "never fails a run" on its own is not true
// of a tree nobody could read.
expect(description).toMatch(/unreadable|cannot be read|could not be read/);
});
it("the reference's idle defaults and settle span are the ones the parser enforces", () => {
const reference = readFileSync(FLOW_YAML, "utf8");
expect(reference).toContain(`default ${IDLE_DEFAULT_STABLE_FOR_MS}`);
expect(reference).toContain(`default ${IDLE_DEFAULT_TIMEOUT_MS}`);
expect(reference).toContain(`${IDLE_SETTLE_SPAN_MS}ms a settle spans`);
expect(reference).toContain(`${IDLE_POLL_MS}ms polls`);
// The gloss has to describe the span it names: the polls the intervals are
// measured over. The round-start floor is quoted separately because it is
// the one term that IS added.
expect(IDLE_SETTLE_SPAN_MS).toBe(IDLE_MIN_STILL_INTERVALS * IDLE_POLL_MS);
expect(reference).toContain(`plus the ${IDLE_POLL_MS}ms of budget the closing round`);
// And the worked numbers it hands the author have to be the parser's.
expect(reference).toContain(
`the default ${IDLE_DEFAULT_STABLE_FOR_MS}ms hold needs ` +
`${idleMinimumTimeoutMs(IDLE_DEFAULT_STABLE_FOR_MS)}ms and an 800ms hold needs ` +
`${idleMinimumTimeoutMs(800)}ms`
);
});
it("every doc that quotes the number of idle warnings quotes the number the reference lists", () => {
const warnings = between(FLOW_YAML, "It **never fails a run.**", "\nOnly a tree source");
const listed = [...warnings.matchAll(/^- \*\*/gm)].length;
// Guard the reader itself: a section that stopped matching would count 0
// and then agree with nothing, which is not the failure we want reported.
expect(listed).toBeGreaterThan(1);
const spelled = ["zero", "one", "two", "three", "four", "five", "six", "seven", "eight"][
listed
];
expect(spelled, `no spelling for ${listed} warnings`).toBeDefined();
for (const file of WARNING_COUNT_CITATIONS) {
// Anchored on the linked citation, not on any "… warnings" phrase: these
// files also say things like "Read this file for selector warnings".
const quoted = readFileSync(file, "utf8").match(
/\[(?:which of the )?(\w+) (?:different )?warnings\]\(/
);
expect(quoted, `${file} no longer cites the idle warning count`).not.toBeNull();
expect(quoted![1], file).toBe(spelled);
}
});
it("the smallest timeout the reference's arithmetic allows is the one the parser accepts", () => {
// The reference tells an author the wait has to contain the LONGER of the
// hold and the settle's span, plus the closing round's budget. Take it at
// its word on both sides of the max — a hold under the span and one over
// it — and check each boundary both ways, since a parser that demanded a
// millisecond more would make the documented arithmetic a lie.
for (const stableFor of [IDLE_DEFAULT_STABLE_FOR_MS, 800]) {
const smallest = Math.max(IDLE_SETTLE_SPAN_MS, stableFor) + IDLE_POLL_MS;
expect(idleMinimumTimeoutMs(stableFor)).toBe(smallest);
const step = (t: number): string =>
`steps:\n - await: { idle: true, stableFor: ${stableFor}, timeout: ${t} }\n`;
expect(() => parseFlow(step(smallest)), `${stableFor}`).not.toThrow();
expect(() => parseFlow(step(smallest - 1)), `${stableFor}`).toThrow(
new RegExp(`at least ${smallest}ms`)
);
}
});
});