Skip to content

fix(declare): keep a stored graph running when a later rule would refuse it - #66

Merged
alice-viola merged 1 commit into
masterfrom
fix/grandfather-stored-documents
Sep 7, 2026
Merged

fix(declare): keep a stored graph running when a later rule would refuse it#66
alice-viola merged 1 commit into
masterfrom
fix/grandfather-stored-documents

Conversation

@albertovincenzi

@albertovincenzi albertovincenzi commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

Problem

restore and reconcile declare through the same path a caller does. A validation refusal there does not keep anybody safe: it unregisters the graph, answers 404 to its pushes, and leaves the ingress queue filling behind a single WARN line. The graph was serving traffic a moment ago, and the rule was added to stop the next declare, not this one.

That makes every rule added since a document was written retroactive. This repository's own production fixture shows the shape at risk: two nodes of the airbnb graph in crates/core/tests/testdata/v1_channel_go_graphs.json (content and messages) carry only conditional budgets. The fixture test itself passes — #34 widens the v1→v2 migration to hand such nodes a passthrough budget, and the test migrates before it validates. But migration runs once per document ever: store.rs parses as GraphDoc first and falls back to v1 only on a parse failure, and lib.rs re-saves the migrated document in v2 shape on the same key. After the first v2 boot the v1 row is gone and migrate.rs is never consulted again, so a v2 document of that shape already in the store is refused on restore and nothing repairs it.

Six open PRs add such a rule: #32, #34, #36, #37, #42, #47.

Fix

  • gate_core::refuses_stored_document names the rules a stored document is still refused for: nodes, paths, application, graph-name, node-name, path-name. There is no plan to build or address in those cases.
  • Every other problem is logged at WARN with its rule names and the graph keeps running.
  • A caller's declare (from_caller == true) is unchanged and still refused for everything.

Verification

  • cargo test --workspace -- --include-ignored against queen 1.0.5 + postgres 16
  • new live regression a_stored_document_that_breaks_a_later_rule_keeps_running, which fails on master (the graph is never registered) and passes here
  • new unit test pinning both halves of the rule list
  • cargo clippy --workspace --all-targets -- -D warnings, cargo fmt --all --check

Review notes

This is the base the six rule PRs should sit on. It changes no rule and no caller-facing behaviour; it changes only what happens to a document that is already in the store.

https://claude.ai/code/session_012K8u7BEJyd6nDNMCQAgH3z

…use it

`restore` and `reconcile` declare through the same path a caller does, so a
validation refusal there does not keep anybody safe. It unregisters the graph,
answers 404 to its pushes and leaves its ingress queue filling behind one WARN
line — strictly worse than the condition the rule describes, because the graph
was serving traffic a moment ago and the rule exists to stop the NEXT declare.

Every rule added since a document was written is therefore retroactive today.
The repository's own production fixture already shows it: two nodes of the
`airbnb` graph in `v1_channel_go_graphs.json` carry only conditional budgets,
and the migration remedy runs once per document ever — after the first v2 boot
the v1 row is gone and `migrate.rs` is never consulted again.

- `refuses_stored_document` names the rules a stored document is still refused
  for: no nodes, no paths, or a name that cannot become a queue name and a kv
  key. There is no plan to run in those cases.
- everything else is logged at WARN with the rule names and kept running. A
  caller's declare of the same document is refused exactly as before.

Claude-Session: https://claude.ai/code/session_012K8u7BEJyd6nDNMCQAgH3z
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.

2 participants