Graph accessors answer an unresolved id with a value a real node can have, because INVALID_NODE_ID is the source root #496

Summary

The id-keyed graph accessors answer an id that resolves to no node with a default that is also a legitimate answer. So a caller cannot tell a failed lookup from a real value.

The root of it is include/pup/core/types.hpp:

inline constexpr auto INVALID_NODE_ID = NodeId { 0 };
inline constexpr auto SOURCE_ROOT_ID = INVALID_NODE_ID;

"No such node" and "the source root" are the same value.

Where the default reads as a real value (src/graph/dag.cpp)

• get_parent_dir returns NodeId { 0 } for an unresolved id, which is also the parent of every top-level node. test/unit/test_graph.cpp pins this with CHECK(get_parent_dir(g, *cmd) == 0);.
• get<NodeType> goes through read_file_field(graph, id, &FileNode::type, NodeType::File), so an unresolved id reports as a source file. A command-tagged id answers Command before any lookup, even if no such command was minted.
• get<NodeFlags> returns NodeFlags::None.
• get<OutputAction> returns Normal.
• get<Name>, get<Display>, get<SourceDir> and get<InstructionPattern> return StringId::Empty.
• The views return an empty list.
• is_stdout_capture returns false.
• get_full_path returns "", the same as the source or build root.
• compute_command_key and compute_command_signature hash an empty expansion, so two unresolved ids share one key.
• find_by_dir_name strips the tag from parent_dir, so a command-, condition- or phi-tagged id reads the file slot with the same index.

get_parent_command is not ambiguous, because a command id is never 0.

Reachability

None found from production. The callers traced (for example serialize_graph_nodes in src/cli/cmd_build.cpp) take ids from all_nodes, nodes_of_type or validated edge endpoints, and all of those yield live ids only. Not every caller was traced: get<NodeType> at cmd_build.cpp (the path_to_node lookup), config_commands.cpp, scheduler.cpp and dag.cpp's compute_command_signature were not.

The invariant "callers pass live ids" is conventional, not structural. The defaults hide any break in it, the way add_file_node hid #492 until a test hit it as an out-of-bounds write.

Found while fixing

PR #494 (#492) fixed the mutating member of this class, add_file_node. It deleted resolve_phi_node, which had no callers. It left this family alone, because changing one accessor's contract would leave it out of step with the others.

Decision to make

• Give "no node" a value distinct from the source root, so that INVALID_NODE_ID != SOURCE_ROOT_ID. This touches every == 0 / != 0 test on node ids.
• Or make the accessors' precondition checked: an assert in read_file_field / read_command_field and in get_parent_dir. Callers that pass a non-file id on purpose would need auditing first.
• Or have iterating callers resolve the node once (get_file_node) and read its fields, so that the id-keyed defaults are never consulted on those paths.