add_file_node discards the duplicate-path signal, so two nodes at one path silently alias #487

Symptom

add_file_node can register two different nodes under one path and silently keep only the last. Nothing reports it, and the losing node becomes unreachable by path while still existing in the graph.

Mechanism

add_file_node interns <parent path>/<name> into a PathId and registers it:

node.path_id = graph.paths.intern(parent_path, node.name);
graph.path_to_node.insert(to_underlying(node.path_id), id);

src/graph/dag.cpp:96-103. The return value is discarded.

SortedPairVec::insert (src/core/sorted_id_vec.cpp:179-185) is not an insert-or-fail - on an existing key it overwrites the value and returns false:

auto const pos = lower_bound(key);
if (pos < size_ && data_[pos].key == key) {
    data_[pos].value = value;
    return false;
}

So the signal that would name a duplicate path is produced and thrown away at the only call site that could act on it. Two nodes with the same parent and the same name leave path_to_node pointing at whichever was added second; the first is orphaned from every path lookup, including ensure_file_node.

Why it is worth closing

Path uniqueness is currently a convention every producer of a node cooperates in, not an invariant. Two live instances of the shared-namespace problem have been found in the last day:

• export or import of TUP_TOOLCHAIN or TUP_TOOL_<name> overwrites putup's own tool node and silently disables tool-change detection #484 (fixed by PR graph: key putup's own tool fingerprints out of the Tupfile's namespace #485): putup's own TUP_TOOLCHAIN / TUP_TOOL_<name> fingerprints shared the env-variable name space with Tupfile export/import. That fix made the key spaces disjoint; it did not make a duplicate insert loud.
• A config variable aliases a same-named source file in the config directory, and edits to that file become invisible #486: a config variable and a same-named source file share a path, and the file becomes invisible.

Both were found by accident. A Result error - or at minimum a diagnostic - on a duplicate path_id would have surfaced each of them at the point of the collision rather than as a stale build days later, and would cover node kinds nobody has audited yet.

What I did not do

I did not construct a repro that reaches the duplicate-insert branch itself. #486 is a lookup collision (ensure_file_node returns the node already registered at that path) rather than a second insert, so it demonstrates the shared namespace but not this branch. The finding here is from reading the two functions above; treat the reachability of the duplicate insert as unverified.

Decisions to make

• Whether add_file_node should return an error on a duplicate path_id, and what its callers do with it.
• Whether SortedPairVec::insert's overwrite-and-return-false contract is the right one for path_to_node specifically, or whether that map wants a distinct insert-or-fail entry point - the same silent-overwrite shape is what made export or import of TUP_TOOLCHAIN or TUP_TOOL_<name> overwrites putup's own tool node and silently disables tool-change detection #484 a stale build rather than an error.