Skip to content

[Graphite MQ] Draft PR GROUP:spec_8cf666 (PRs 37) - #43

Closed
graphite-app[bot] wants to merge 1 commit into
masterfrom
gtmq_spec_8cf666_1790955024919-6a6a0eba-c38b-47c6-9aad-1e538fcfa299
Closed

graphite-app[bot] wants to merge 1 commit into
masterfrom
gtmq_spec_8cf666_1790955024919-6a6a0eba-c38b-47c6-9aad-1e538fcfa299

Conversation

@graphite-app

@graphite-app graphite-app Bot commented Oct 2, 2026

Copy link
Copy Markdown

This draft PR was created by the Graphite merge queue.
Trunk will be fast forwarded to the HEAD of this PR when CI passes, and the original PRs will be closed.

The following PRs are included in this draft PR:

…en (#37)

## 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
- [x] `gofmt`, `go vet ./...` and `go test ./...` pass; `go.mod` is unchanged.
- [x] 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.
- [x] Full behaviour suite passes 79/79 on current master (`6df4604`). Together with #38–#42 it passes 103/103, with a shell `VF_TOKEN` exported and the decoy HOME unchanged.
- [x] Run against the decoy HOME, which is unchanged afterwards.
- [x] 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.
@graphite-app graphite-app Bot closed this Oct 2, 2026
@graphite-app
graphite-app Bot deleted the gtmq_spec_8cf666_1790955024919-6a6a0eba-c38b-47c6-9aad-1e538fcfa299 branch October 2, 2026 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant