test: `deployed_allowlist_parses` asserts nothing about the deployment, and had already rotted in one of four #7

New issue

Closed

opened 2026-08-25 02:38:11 +00:00 by jlxq0 · 0 comments

jlxq0 commented

2026-08-25 02:38:11 +00:00

Owner

A test named after the running system, that takes no input from the running system

All four of caldav-mcp, carddav-mcp, jmap-mcp and typst-mcp carry this, copy-pasted alongside oauth_redirect.rs:

/// The exact set the deployment ships, parsed as one env value.
#[test]
fn deployed_allowlist_parses() {
    let raw = "https://claude.ai/api/mcp/auth_callback,\
               ... nine entries ...";
    let allowed = parse_allowlist(raw, ENV_OAUTH_REDIRECT_URIS).unwrap();
    assert_eq!(allowed.len(), 9);
}

raw is a literal in the test. allowed.len() counts what raw parsed to. The deployment is not an input, so the assertion cannot fail when the deployment changes. The doc comment makes a claim about production that nothing checks, and the test passes forever regardless of whether the claim is still true.

It had already rotted in one of the four

Found by the typst-mcp worker, then verified across the set against the live cluster rather than assumed:

repo	fixture	live	match
`caldav-mcp`	9 entries	9 entries	yes
`carddav-mcp`	9 entries	9 entries	yes
`jmap-mcp`	9 entries	9 entries	yes
`typst-mcp`	9 entries	7 entries	no

typst-mcp's deployment is missing the three claude:// / cowork:// entries the fixture lists, and carries http://localhost:8787/oauth/callback, which the fixture does not. Three were accurate by luck and one was not, and no test could have told you which was which — they all passed.

Why this is worth a change rather than a shrug

A green test named deployed_allowlist_parses reads as evidence that the deployed allowlist is what you think it is. It is not evidence of anything except that a string literal parses. That is the same failure the AGENTS.md rule about status files describes: something that ages into a confident description of a system that has moved, while looking healthy.

It also nearly cost real work. typst-mcp's two deployed loopback entries share a scheme, host and port and differ only by path, so a port-relaxation that also relaxed the path would silently merge them in production. The fixture would have said nothing, because it did not know they existed.

The repair, already done in typst-mcp

Split it in two:

deployed_allowlist_parses — the actual live set, with the date it was verified in the doc comment. Still cannot fail on drift, but now it is a dated claim rather than an undated one, and the date tells the next reader how much to trust it.
allowlist_accepts_the_shapes_we_support — the same parsing assertions with no claim about production at all.

The general rule, worth putting in each repo's AGENTS.md under Known Pitfalls: a test named after the deployment is a claim about the running system, and it rots silently. Either feed it the running system or do not name it after it.

Optional, and probably better

A CI step that reads the live allowlist and diffs it against the fixture would make drift loud. It needs cluster access from the runner, which is a bigger change than this issue is asking for. Noted rather than proposed.

Related: #3 — same "five copies, and they diverge" shape.

## A test named after the running system, that takes no input from the running system All four of `caldav-mcp`, `carddav-mcp`, `jmap-mcp` and `typst-mcp` carry this, copy-pasted alongside `oauth_redirect.rs`: ```rust /// The exact set the deployment ships, parsed as one env value. #[test] fn deployed_allowlist_parses() { let raw = "https://claude.ai/api/mcp/auth_callback,\ ... nine entries ..."; let allowed = parse_allowlist(raw, ENV_OAUTH_REDIRECT_URIS).unwrap(); assert_eq!(allowed.len(), 9); } ``` `raw` is a literal in the test. `allowed.len()` counts what `raw` parsed to. **The deployment is not an input, so the assertion cannot fail when the deployment changes.** The doc comment makes a claim about production that nothing checks, and the test passes forever regardless of whether the claim is still true. ## It had already rotted in one of the four Found by the `typst-mcp` worker, then verified across the set against the live cluster rather than assumed: | repo | fixture | live | match | |---|---|---|---| | `caldav-mcp` | 9 entries | 9 entries | yes | | `carddav-mcp` | 9 entries | 9 entries | yes | | `jmap-mcp` | 9 entries | 9 entries | yes | | `typst-mcp` | 9 entries | **7 entries** | **no** | `typst-mcp`'s deployment is missing the three `claude://` / `cowork://` entries the fixture lists, and carries `http://localhost:8787/oauth/callback`, which the fixture does not. Three were accurate by luck and one was not, and **no test could have told you which was which** — they all passed. ## Why this is worth a change rather than a shrug A green test named `deployed_allowlist_parses` reads as evidence that the deployed allowlist is what you think it is. It is not evidence of anything except that a string literal parses. That is the same failure the `AGENTS.md` rule about status files describes: something that ages into a confident description of a system that has moved, while looking healthy. It also nearly cost real work. `typst-mcp`'s two deployed loopback entries share a scheme, host and port and differ only by path, so a port-relaxation that also relaxed the path would silently merge them in production. The fixture would have said nothing, because it did not know they existed. ## The repair, already done in typst-mcp Split it in two: - `deployed_allowlist_parses` — the actual live set, with the date it was verified in the doc comment. Still cannot fail on drift, but now it is a dated claim rather than an undated one, and the date tells the next reader how much to trust it. - `allowlist_accepts_the_shapes_we_support` — the same parsing assertions with no claim about production at all. The general rule, worth putting in each repo's `AGENTS.md` under Known Pitfalls: **a test named after the deployment is a claim about the running system, and it rots silently.** Either feed it the running system or do not name it after it. ## Optional, and probably better A CI step that reads the live allowlist and diffs it against the fixture would make drift loud. It needs cluster access from the runner, which is a bigger change than this issue is asking for. Noted rather than proposed. Related: https://forge.oddie.app/jlxq0/caldav-mcp/issues/3 — same "five copies, and they diverge" shape.

jlxq0 referenced this issue

2026-08-25 02:48:15 +00:00

test(auth): carry the shared redirect-URI case table in this repo #3

jlxq0 referenced this issue from a commit

2026-08-26 04:59:00 +00:00

test(auth): date the deployed allowlist snapshot and assert per entry

jlxq0 referenced this issue from a pull request that will close it,

2026-08-26 04:59:22 +00:00

test(auth): date the deployed allowlist snapshot and assert per entry #11

jlxq0 closed this issue

2026-08-26 05:09:48 +00:00