Take consensus out of the source dependency cycle

[Amaury Chamayou (achamayou)]

Motivation

Partially addresses #3517. This implements the next step from the latest analysis.

Since #8463, the source dependency cycle has been {consensus, endpoints, js, node}. consensus is in it only because raft.h includes five node headers, and Raft needs very little from them. This PR inverts those dependencies, so consensus leaves the cycle and gets a dependency policy.

	Before	After
Direct internal edges	84	83
Components in the cycle	consensus, endpoints, js, node	endpoints, js, node
Acyclic components, all with policies	21	22

The diagram shows every direct dependency among the four components that formed the cycle, each labelled with the number of #include directives behind it. The dashed red edge is the one this PR removes.

graph LR
  subgraph core["remaining cycle"]
    endpoints -- 6 --> node
    node -- 4 --> endpoints
    js -- 9 --> node
    node -- 14 --> js
    js -- 7 --> endpoints
  end
  node -- 11 --> consensus
  consensus -.->|5, removed| node
  linkStyle 6 stroke:#cf222e,stroke-width:3px

Loading

Implementation summary

Raft keeps the same behaviour, but reaches the node through types owned by consensus. No node code moves:

• aft::ConsensusChannels (consensus/aft/consensus_channels.h) holds the part of ccf::NodeToNode that Raft uses: associate_node_address, recv_authenticated<T>, DroppedMessageException, and a new send_consensus_message. NodeToNode derives from it and implements send_consensus_message as send_authenticated(to, NodeMsgType::consensus_msg, ...). The channel manager does not change, NodeMsgType stays in node, and ccf::NodeToNode::DroppedMessageException still names the same type.
• aft::CommitObserver (consensus/aft/commit_observer.h) has a single method, on_commit(TxID, const ViewHistory&). CommitCallbackSubsystem implements it; on_commit is the renamed trigger_callbacks. set_consensus() and its pointer are removed: the pointer was only used to throw if trigger_callbacks() ran before set_consensus(), and Raft was the only caller of both.
• Retired node cleanup: Aft takes a std::function<void()> instead of a std::shared_ptr<NodeClient>. NodeState::setup_consensus() builds the RetiredNodeCleanup from its HTTPNodeClient and passes a lambda that owns it, so it still lives as long as Aft.
• Tests: aft::ChannelStubProxy only implements ConsensusChannels, which drops 11 no-op overrides. raft_test now includes commit_callback_subsystem.h itself; test files are not covered by the dependency policies.
• source-dependencies.json gains a consensus policy (ccf-api, crypto, ds, kv, service; removing any entry makes the check fail).
• Housekeeping: the policies no longer allow two dependencies that the code stopped using: tcp -> ds and tls -> ccf-api.

Other open PRs affected:

• Fluentd exporter for observability of the raft tracing #8386 adds consensus -> msgpack and consensus -> tracing, which the new consensus policy rejects. Allowing them is fine as long as tracing does not depend on node. As written, fluentd_sink.h includes ccf/node/startup_config.h, which would put consensus back in the cycle. Having FluentdSink take a tracing-owned endpoint would avoid that.
• Remove ringbuffer usage for node-to-node traffic #8446 and Remove ringbuffer usage for ledger mutations and range reads #8405 edit an adjacent line in the same make_shared<RaftType> statement, so their conflicts will be mechanical.

Safety and compatibility

Production behaviour does not change:

• Consensus messages go through the same channel manager with the same NodeMsgType, and receive-side authentication is identical.
• Commit callbacks run from the same point in Aft::commit, under the same locks.
• Retired node cleanup is queued from the same two call sites.

The only behaviour change is test-only. Unit tests construct Aft with a null NodeClient, so Raft used to queue a cleanup task that would have dereferenced it if it had run. Now it queues nothing.

There are no public API changes (nothing under include/), no wire, ledger or KV format changes, and no effect on mixed-version operation or recovery. Nothing is user-facing, so there is no CHANGELOG.md entry.

[Copilot]

Copilot review overview

🔵 Needs a closer look

The production Raft channel, commit notification, and cleanup paths are consensus-critical and warrant final human review.

Review effort: Balanced
Findings: None

What changed in this PR

This PR advances #3517 by removing consensus from CCF’s source dependency cycle while keeping Raft’s node-facing behavior behind consensus-owned interfaces.

Changes:

• Replace Raft’s direct node dependencies with channel, commit-observer, and cleanup interfaces.
• Simplify the Raft test channel stub.
• Add a consensus dependency policy and make the checker reject new or stale cycles.

Custom instructions used: .github/copilot-instructions.md, .github/instructions/reviewing.instructions.md

File	Description
src/​node/​node_to_node.h	Implements consensus message sending through the new interface.
src/​node/​node_state.h	Supplies Raft with a cleanup callback.
src/​node/​commit_callback_subsystem.h	Implements the commit observer.
src/​consensus/​aft/​test/​main.cpp	Includes the node callback implementation explicitly.
src/​consensus/​aft/​test/​logging_stub.h	Narrows the test channel stub to Raft’s interface.
src/​consensus/​aft/​raft.h	Uses the new interfaces instead of node types.
src/​consensus/​aft/​consensus_channels.h	Defines Raft’s channel interface.
src/​consensus/​aft/​commit_observer.h	Defines the commit notification interface.
scripts/​source-dependencies.json	Records the consensus policy and remaining cycle.
scripts/​check-source-dependencies.py	Detects dependency cycles and missing policies.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.