docs(process): a mutation's node ID is checkable before the write (closes #1916) - #1922

Closes #1916.

What changed

AGENTS.md > PR Workflow > step 8 covers a mutation reporting the wrong thing - a mergePullRequest that says "not mergeable" on a merge that landed - and not a mutation addressing the wrong thing, which is the direction with no undo.

A mutation names its subject by node ID and by nothing else: createIssue takes a repositoryId, not an owner and a name. So a well-formed ID that is wrong does not fail - it succeeds against whatever object it does name. A repositoryId carried over from an earlier response rather than queried, R_kgDOD1WOFw for R_kgDORUMiZg, created issue #1 in an unrelated third-party repository and returned success. The only clue was the url in the response, and deleteIssue needs admin on the target, so it could not be undone.

This lands as the last bullet of step 8, whose subject is already verifying a mutation by reading it back.

The issue's prescribed location does not exist

#1916 proposed recording the rule "next to the existing use_github guidance". There is none - AGENTS.md has no use_github, node-ID or GraphQL passage at all:

grep -n "use_github\|node ID\|GraphQL\|repositoryId" AGENTS.md   ->  0 hits

Worth stating because the absence is why nothing caught the write: the rule was not weakly documented, it was undocumented.

The premise the incident was written up under is measurably false

The write-up says IDs are "unreadable by design". They are not. A node ID is <TypePrefix>_<urlsafe-base64(msgpack array)>, where a repository is [0, databaseId] and anything a repository owns is [0, repository databaseId, own databaseId] - so the type and the target repository are readable with no network call, before the write:

Every right-hand column is what repository(owner: "strands-labs", name: "robots") returns beside the ID, so the decoder is checked against GitHub's own answer rather than against a reimplementation of itself.

That matters because it changes the remedy. "Always query the ID" is advice a stale value defeats - which is exactly what happened three times in one run. A check runnable on the value in hand does not depend on remembering.

One finding the write-up did not have

All three guessed IDs in that run carried one wrong repository:

So a single stale value contaminated every mutation, and the two that failed did so only because their own databaseId happened not to exist under that repository. Failing closed was luck about the guess, not a property of the API - and the guess that got lucky the other way is the one that wrote.

Test surface

tests/test_graphql_node_id_targeting.py, 15 tests, two classes with different jobs.

TestTheNodeIdEnvelopeIsCheckableOffline (10) executes the claim. A pin that only asserted AGENTS.md says the ID is checkable would keep passing against a future ID format that had stopped being checkable, leaving the guidance reading plausibly while advising something impossible. This decodes the three real IDs above against their published databaseIds, asserts the stray ID is distinguishable from the intended one, and asserts both incident IDs resolve to the same wrong repository.

TestTheGuidanceNamesTheDecodableEnvelope (5) pins the prose, because the prose is the deliverable - an agent reads AGENTS.md, not this module. It asserts adjacency rather than vocabulary: the fail-open property, the decodable envelope, the missing undo and the response-url check have to stay in the same breath as the rule, since each alone is unactionable. A future edit tightening the passage back to "resolve IDs with a query" is exactly the regression, it looks like an improvement, and nothing else in the tree would notice. Same structural reason tests/test_merge_gate_viewer_scope.py and tests/test_codeql_query_filters.py exist, and the text-assertion shape follows those modules.

Negative control - with origin/main's AGENTS.md restored, all 5 guidance tests fail and all 10 offline-decoding tests pass unchanged:

5 failed, 10 passed        # every failure in TestTheGuidanceNamesTheDecodableEnvelope

The split is the point: the envelope is a property of GitHub's IDs, not of this change, so only the guidance half is new.

Writing the decoder surfaced a second permissiveness

base64.urlsafe_b64decode discards bytes outside the alphabet and everything after the padding, so "kgDORUMiZg==extra" decodes to this repository's id - a corrupted ID yielding a plausible answer, in a value used to decide whether a write is safe. The trailing-garbage case caught it. The decoder now re-encodes and compares rather than the test being relaxed.

Verification

MUJOCO_GL=egl python -m pytest tests -q --no-cov -p no:randomly
  18485 passed, 257 skipped in 585.34s

ruff check strands_robots tests tests_integ          All checks passed!
ruff format --check strands_robots tests tests_integ  1046 files already formatted
mypy strands_robots tests tests_integ                Success: no issues found in 1043 source files

python3 scripts/assemble_changelog.py --check         changelog fragments OK (146 pending)
python3 scripts/check_merge_base_overlap.py           No overlap (base 2f2f1911, unmoved)

Scope

Documentation and test only; no production code changes. Nothing here touches a policy, simulation behaviour, rendering, recording or a robot asset, so there is no visual artifact to attach - the measurements above are the evidence.

The stray issue itself cannot be cleaned up: deleteIssue needs admin on the target repository. It is closed as NOT_PLANNED with an apology, which is the whole reason this is a check-before rather than a verify-after.