Skip to content

fix(graph): reject physical queue feedback and source aliases - #32

Merged
alice-viola merged 2 commits into
masterfrom
fix/reject-queue-feedback
Sep 7, 2026
Merged

fix(graph): reject physical queue feedback and source aliases#32
alice-viola merged 2 commits into
masterfrom
fix/reject-queue-feedback

Conversation

@albertovincenzi

Copy link
Copy Markdown
Collaborator

Problem

The graph validator only treated explicitly declared ingress queues as owned inputs. The compiled plan also contains Gate-owned interior source queues, so two unsafe physical topologies passed validation even when the logical node DAG looked acyclic:

  • another node or graph could declare a Gate interior queue as its ingress, creating a second consumer group that receives and forwards every message;
  • a terminal egress could point back to any source in the same graph, including the one-node ingress=egress case, making admitted messages circulate forever and repay budgets on every turn.

Fix

  • derive queue ownership from every compiled stage source, including interior queues;
  • apply that ownership consistently within one document, against the local registry, and against graphs loaded from the durable store;
  • reject a terminal destination that aliases any source in its own graph with a dedicated queue-cycle validation problem;
  • add unit coverage for local alias/feedback cases and a broker-backed regression for cross-graph interior ownership.

Notes for Alice

Normal graph-to-graph chaining remains legal: graph A may write to a queue that graph B owns as its ingress. The producer does not claim its terminal egress as a source. The new refusals only prevent multiple Gate consumers of the same physical source and feedback from a graph's terminal output into one of its own sources.

The durable-store collision check is still a read/check operation, as before; this PR strengthens which queues it checks but does not claim to solve a simultaneous cross-replica declaration race.

Verification

  • cargo fmt --all -- --check
  • cargo test --workspace
  • cargo clippy --workspace --all-targets -- -D warnings

`queue-cycle` asked whether a terminal destination appears in the set of this
graph's sources. Set membership cannot tell a loop from a chain: two paths
joined through one queue — `app.in` to `app.mid` on one, `app.mid` to `app.out`
on the other — make `app.mid` a terminal destination and a source, and messages
pass through it once and stop. That graph is legal on master, and the refusal
told it that work "would circulate for ever", which is not true of it.

The check now walks the queue graph forward from the destination and asks
whether it can arrive back. The case the rule is for — a one-node target whose
ingress and egress are the same name — still answers yes on the first hop.

It matters beyond the false 422: a rule that refuses a stored document also
stops it from restarting, so a graph that has been running for months would not
have come back after the next deploy.

Claude-Session: https://claude.ai/code/session_012K8u7BEJyd6nDNMCQAgH3z
@albertovincenzi

Copy link
Copy Markdown
Collaborator Author

This rule is retroactive on documents that are already in the store.

restore and reconcile both go through declare_from_storedeclare_locked(.., from_caller = false)validate_with, with no exception. A single Problem therefore means the graph is not registered on the next restart: its pushes answer 404 and its ingress queue fills behind one WARN line.

#66 draws the line — a stored document is refused only for nodes, paths or a name that cannot become a queue name and a kv key, and everything else is logged and kept running. Suggest basing this PR on it, or merging it first.

For #34 specifically this is not hypothetical: airbnb/content and airbnb/messages in crates/core/tests/testdata/v1_channel_go_graphs.json carry only conditional budgets, and the passthrough remedy in migrate.rs never runs again once a document has been re-saved in v2 shape.

https://claude.ai/code/session_012K8u7BEJyd6nDNMCQAgH3z

@alice-viola
alice-viola merged commit 10dd887 into master Sep 7, 2026
1 check passed
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