fix: restore vf docs search, and make the test suite hermetic and green by Bradenream · Pull Request #37 · voiceflow/cli

Bradenream · 2026-10-01T20:25:21Z

Summary

Two problems made the behaviour suite untrustworthy, and one of them hid a real outage.

1. The suite ran against the developer's own HOME. Every vf it spawned read ~/.config/vf and, on macOS, the login Keychain. The no-token tests found a real session and failed. Worse, a run refreshed the real OAuth session: it rewrote oauth.json and rotated the tokens in the Keychain. test/setup.ts now gives each test file an empty temp HOME, and cuts off the OS keyring on both platforms the suite runs on:

macOS: under the empty HOME, /usr/bin/security has no default keychain.
Linux: go-keyring reaches the Secret Service over D-Bus, which HOME does not affect. So setup also points DBUS_SESSION_BUS_ADDRESS at a socket that does not exist.

Either way, vf can neither read nor overwrite the developer's tokens. The integration tests still authenticate with VF_TOKEN from .env.test.

2. Four tests were red on master (docs-command ×2, flag-errors, flag-raw-text), for three different reasons:

vf docs search is broken for every user. The docs MCP server renamed search_voiceflow_documentation to search_voiceflow, which takes question, initiator, purpose and why, and returns structuredContent. The server reports an unknown tool inside a successful result (isError: true), and vf never checked that flag: it printed "no such tool" as a search hit and exited 0. vf now calls the new tool and keeps the same {title, link, page, content} output. A tool error is now an error.
The help-text check went stale. A regeneration changed the descriptions of the two flags it named to "string value", removing their backticks. Their two cases kept passing with nothing to check, and the prose case failed. The check now takes every flag whose description has a backtick from vf --usage (16 today) and checks each one's --help. A separate case fails if it finds none.
The Markup strictness check lost its flag. A regeneration made mcp-server create --url a plain string and removed Markup from the spec. The check now uses transcript search --filters, a list of unions: the shape --url used to have. A reflection probe over all 90 JSON flags found none that is a union with a string member outside a list. So a unit test now pins targetsStringValue on its own types, which the spec cannot change.

Changes from review

Shell variables: the suite now takes its VF_* settings from .env.test alone (test/env.ts). VF_SKIP_INTEGRATION_TESTS is the one exception. Before, a VF_TOKEN and VF_WORKSPACE_ID exported in the developer's shell queued all 16 integration test files against that account.
Credential guard: test/isolation.test.ts fails if a vf spawned by the suite finds a token from any source, an OAuth session, or a config file in the real home.
vf docs search:
- finds the reply anywhere in the server's event stream;
- gives a reason for a tool error that has no text;
- takes a hit's page from its link when pageUrl is missing;
- prints [] for no matches in JSON output, and applies --jq.
Help-text check: each flag is checked against its own help entry, and its label must be a pflag type name. The --help runs go in parallel, and KDL escapes are decoded properly.
--filters strictness check: it now tells the raw-text fallback apart from a decode failure.

Before and after

	master	this PR
Behaviour suite, empty HOME	4 failed, 76 passed	79 passed
Same, against a decoy HOME holding a token and an OAuth session, with VF_TOKEN and VF_OUTPUT_FORMAT exported in the shell	9 failed; vf wrote into the decoy	79 passed; decoy byte-for-byte unchanged
`vf docs search "personal access token"`	prints "no such tool…", exit 0	5 results, exit 0
Help-text check with backtick neutralization removed	would catch 1 flag	catches all 16

Test plan

gofmt, go vet ./... and go test ./... pass; go.mod is unchanged.
New Go tests:
- internal/cli/docs_test.go covers parsing, isError, page paths and the tool's required arguments.
- internal/flagutil/stringvalue_test.go covers the raw-text type rule. Loosening the rule fails 4 of its cases.
Full behaviour suite passes 79/79 on current master (6df4604). Together with fix: stop waiting on the silent stdin an agent's shell hands vf #38–fix: hide camelCase secrets in --dry-run and --debug output #42 it passes 103/103, with a shell VF_TOKEN exported and the decoy HOME unchanged.
Run against the decoy HOME, which is unchanged afterwards.
Live checks of vf docs search in human, JSON and agent mode. Each hit's page works with vf docs get.
CI

The docs-command tests hit the live docs site, so they will catch the next contract change too.

The hermetic suite spawned vf with the developer's own HOME, so it read their ~/.config/vf and, on macOS, their login Keychain. Tests that expect no credentials (the no-token preflight cases) found a session and failed, and a run refreshed the real OAuth session: it rewrote oauth.json and rotated the tokens in the Keychain. test/setup.ts now points HOME at a fresh temp directory for each test file and removes it afterwards. Under an empty HOME, /usr/bin/security has no default keychain either, so vf can neither read nor overwrite the developer's tokens. The integration tests still authenticate with VF_TOKEN from .env.test. Checked against a decoy HOME holding a canary token and an OAuth session: before, 5 no-token tests read the canary and vf wrote oauth.lock into it; after, every one passes and the decoy is byte-for-byte unchanged.

vf docs search called search_voiceflow_documentation, a tool the docs MCP server no longer has. The server now offers search_voiceflow, which takes a question plus who is asking and why, and returns its hits as structuredContent instead of Title:/Link:/Page:/Content: text blocks. The server reports an unknown tool inside a successful JSON-RPC result (isError: true), and vf never read that flag. It printed "no such tool" as if it were a search hit and exited 0, and --output-format json returned a single hit with every field empty. That is what the two red docs-command tests were catching. vf now calls search_voiceflow and maps each hit to the same {title, link, page, content} it printed before, with page cut from the hit's page URL so it can go straight to vf docs get. A tool error, or a response without structured results, is now an error.

The help check named its flags by hand: agent update's --instructions and --prompt, whose descriptions had backticked words, and transcript search's --version-param. The latest regeneration gave --instructions and --prompt the description "string value". Their two checks kept passing with nothing left to check, and the prose check, which looked for "Backticked 'Name' resolves to", failed. The flags now come from vf --usage, which keeps each description as the spec wrote it. Every flag whose description has a backtick is checked in its command's --help: its description must appear with the backticks turned into quotes. Had pflag lifted a word as the value placeholder, that word would appear bare and the description would not match. A separate case fails if no such flag is found, so the check cannot pass empty. With the neutralization removed from flagDescription, this now reports all 16 flags it affects today: --playbooks renders as "--playbooks workflows", --label as "--label name", --exclude as "--exclude *".

"Leaves Markup-valued flags strict" passed --url 'not json' to mcp-server create and expected a rejection, because --url was []components.Markup. The latest regeneration made that field a plain string and removed Markup from the spec, so the text was accepted and the test went red. The behaviour test now uses transcript search's --filters, a []components.TranscriptFilterUnion: a list of unions, the shape --url had. A list is not a string, so raw text is still an error there. That check cannot see the sharper case the review asked about: a union with a string member, outside a list, which would decode quoted text if the raw-text fallback ever ran on it. A reflection probe over all 90 JSON flags found none with that shape today. So targetsStringValue gets a unit test with its own types: string, *string, a string enum and an optional-nullable string take raw text; a union with a string member, a list, a map and an optional-nullable union do not. Loosening the rule to accept those fails four of its cases.

Copilot

Copilot review overview

🟡 Changes recommended

HOME isolation does not prevent access to real OS keyrings on all supported platforms.

Review effort: Balanced
Findings: 1

Open (1)

Tests can access real credentials despite HOME isolation · New

What changed in this PR

Restores documentation search and improves behavior-suite isolation and reliability.

Changes:

Updates docs search for the new MCP contract.
Isolates tests from the developer’s home directory.
Refreshes flag behavior tests and adds focused Go coverage.

File	Description
`test/setup.ts`	Creates a temporary test HOME.
`test/flag-raw-text.test.ts`	Updates strict union-flag coverage.
`test/flag-errors.test.ts`	Dynamically validates backticked flag descriptions.
`internal/flagutil/stringvalue_test.go`	Tests string-target type detection.
`internal/cli/docs.go`	Implements the new docs search contract.
`internal/cli/docs_test.go`	Tests response parsing, arguments, and page paths.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

An empty HOME isolates ~/.config/vf everywhere, and the login Keychain on macOS, where /usr/bin/security finds it through $HOME. On Linux, vf's keyring (zalando/go-keyring) reaches the Secret Service over the D-Bus session bus, and godbus finds that bus through DBUS_SESSION_BUS_ADDRESS or $XDG_RUNTIME_DIR/bus, never $HOME. So a developer's token in GNOME Keyring or KWallet would still have been read by the no-token tests, and an auth flow could have overwritten it. test/setup.ts now also points DBUS_SESSION_BUS_ADDRESS at a socket that does not exist. godbus uses an address that is set rather than its fallback, and fails at once. go-keyring returns that error, and vf then treats the keyring as unavailable, so it neither reads nor writes it. Checked against the versions vf builds with: go-keyring v0.2.6 and godbus v5.1.0.

The no-token cases ran vf with an empty HOME, which hides the macOS Keychain, but on Linux vf reaches the keyring over the D-Bus session bus. A developer with a token saved there would have had it found, and the cases would have taken a different path. They now point DBUS_SESSION_BUS_ADDRESS at a socket that does not exist, as test/setup.ts does for the whole suite in #37. Copilot raised this in review.

z4o4z

Left 10 inline comments. The ones I'd fix before merging:

flag-raw-text.test.ts:97. The --filters strictness test passes whether or not the raw-text fallback runs on that flag.
setup.ts:7. VF_* variables exported in the developer's shell still override .env.test, so the suite isn't fully hermetic and could use a real token.
docs.go:311. Combined with the existing first-data:-line SSE parsing, the new "no structured results" error makes search fail if the server sends a notification before the result.

The rest are edge cases or test-robustness notes. The no-results prose and --jq comments are about behaviour that predates this PR.

What I checked: the new Go tests pass. I simulated the new help-text check against a locally built vf: it finds all 16 flags with no problems. I couldn't run the live docs calls or the vitest suite in my sandbox.

Generated by Claude Code

The --filters strictness check asserted only "invalid value for --filters". Had the raw-text fallback run on that flag, the text would have been quoted into valid JSON that then failed to decode as a list, and the error starts with the same words, so the check could not fail. Review on #37 caught it. The check now asserts the strict path's own wording ("the value is not valid JSON") and the absence of the fallback's ("valid JSON but not the shape"). With targetsStringValue forced to run the fallback on every flag, it now fails.

dotenv never overrides a variable that is already set, so a VF_TOKEN, VF_WORKSPACE_ID or VF_OUTPUT_FORMAT exported in the developer's shell won over .env.test. With a real token and workspace in the shell, the config counted them as credentials and queued all 16 integration test files, which create and delete projects in that workspace. Every hermetic test's vf also saw the token. Review on #37 caught it. test/env.ts now drops every VF_ variable except VF_SKIP_INTEGRATION_TESTS, the switch CI sets on purpose, before loading .env.test. vitest.config.mts and test/setup.ts both use it. With shell credentials and no .env.test, the suite now stops with its usual "need VF_TOKEN and VF_WORKSPACE_ID" error instead of using them.

Isolation rests on how each platform finds its credential store: the macOS Keychain through $HOME, a Linux keyring over D-Bus. If a platform ever found its store another way, the suite would quietly run against the developer's account again. Review on #37 raised this. test/isolation.test.ts runs vf whoami the way every test runs vf, and requires no token from any source (flag, environment, keyring or config file), no OAuth session, and a config file outside the real home. With VF_TOKEN exported in the shell and the previous commit undone, it fails and shows the token's source as [env].

The docs server answers with server-sent events, and vf read only the first data: line. A stream that sends another message first, such as a progress notification, or that splits the reply across several data: lines, would have made every search fail with "no structured results". Review on #37 caught it. vf now joins each event's data: lines and takes the first message that carries a result or an error; notifications carry neither. A stream with no reply in it is still an error.

Two gaps in reading a search reply, found in review on #37: - A tool error with no text gave "documentation search failed: —", naming no cause. It now says the tool reported an error without saying why. - A hit without a pageUrl got an empty page, and the suggested "vf docs get <page>" then failed. The page now comes from the hit's link, with its #section dropped.

Two problems with vf docs search's structured output, raised in review on #37. Both predate this PR. - With no matches it printed a sentence of prose even under --output-format json, so a caller parsing stdout as JSON failed. It now prints []. - --jq switched the output to JSON but the expression was never applied: --jq '.[0].page' printed the whole list. It now runs through output.ApplyJqFilter, as other commands use it.

Three gaps in the help-text check, found in review on #37: - Each description was matched against the command's whole --help. A flag whose backticks were lifted could pass on another flag's identical text. Each flag is now checked against its own entry: its line and the lines its description wraps onto. - Nothing checked the --flag <type> label itself, though the describe title promises it. The label must now be one of pflag's type names, or absent for a boolean. With neutralization removed, all 16 flags report the lifted word, including --metadata's "{ key, values }". - The --help runs were awaited one after another inside one test, and the KDL schema was decoded with JSON.parse, which throws on a KDL-only escape such as \u{...}. The runs now go in parallel, and KDL escapes are decoded properly.

Bradenream · 2026-10-02T15:20:43Z

Merge activity

Oct 2, 3:20 PM UTC: The merge label 'merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
Oct 2, 3:29 PM UTC: Bradenream added this pull request to the Graphite merge queue.
Oct 2, 3:30 PM UTC: CI is running for this pull request on a draft pull request (#43) due to your merge queue CI optimization settings.
Oct 2, 3:31 PM UTC: Merged by the Graphite merge queue via draft PR: #43.

Bradenream added 4 commits September 29, 2026 14:58

Copilot AI balanced review requested due to automatic review settings October 1, 2026 20:25

Bradenream mentioned this pull request Oct 1, 2026

fix: stop waiting on the silent stdin an agent's shell hands vf #38

Closed

4 of 5 tasks

Copilot started reviewing on behalf of Bradenream October 1, 2026 20:26 View session

Copilot AI reviewed Oct 1, 2026

View reviewed changes

Comment thread test/setup.ts

z4o4z reviewed Oct 2, 2026

View reviewed changes

Bradenream added 7 commits October 2, 2026 10:34

Bradenream added the merge label Oct 2, 2026

effervescentia approved these changes Oct 2, 2026

View reviewed changes

graphite-app Bot closed this Oct 2, 2026

graphite-app Bot deleted the braden/test-isolation/COR-0 branch October 2, 2026 15:31

graphite-app Bot removed the merge label Oct 2, 2026

Bradenream added a commit that referenced this pull request Oct 2, 2026

ci: re-run checks on master now that #37 has fixed its tests

9f7ad76

Bradenream added a commit that referenced this pull request Oct 2, 2026

ci: re-run checks on master now that #37 has fixed its tests

a305b1a

Bradenream added a commit that referenced this pull request Oct 2, 2026

ci: re-run checks on master now that #37 has fixed its tests

2f53766

Bradenream added a commit that referenced this pull request Oct 2, 2026

ci: re-run checks on master now that #37 has fixed its tests

4a8260a

Bradenream added a commit that referenced this pull request Oct 2, 2026

ci: re-run checks on master now that #37 has fixed its tests

ecc41bc

Navigation Menu

fix: restore vf docs search, and make the test suite hermetic and green - #37

fix: restore vf docs search, and make the test suite hermetic and green#37
Bradenream wants to merge 12 commits into
masterfrom
braden/test-isolation/COR-0

Bradenream commented Oct 1, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

z4o4z left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Bradenream commented Oct 2, 2026 •

edited by graphite-app Bot

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

Bradenream commented Oct 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes from review

Before and after

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

z4o4z left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Bradenream commented Oct 2, 2026 • edited by graphite-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Bradenream commented Oct 1, 2026 •

edited

Loading

Bradenream commented Oct 2, 2026 •

edited by graphite-app Bot

Loading