Repository navigation
Native ingest: refuse two files claiming one module path - #13590
gunbai-bot[bot] wants to merge 13 commits into
Conversation
A copied closure under one root tagged every v2 file as DagTree because tree identity used a src/ prefix, so resolve reported an unbound name elsewhere instead of the collision. Tag V2Tree on a src/ path component, and locate closure_census_duplicate_module on both declaring files. Co-authored-by: Cursor <cursoragent@cursor.com>
Delete file-path classification: a copied single root refuses as unrecognized, and a dag file whose path contains /src/ stays DagTree. Co-authored-by: Cursor <cursoragent@cursor.com>
…kip. The previous trim used skip/list_reverse on host String and the emitted CLI crate refused at rustc E0308. Co-authored-by: Cursor <cursoragent@cursor.com>
Tree identity is admitted from the --source-root argument; fixtures/native_cli_door is not that tree. Co-authored-by: Cursor <cursoragent@cursor.com>
The claims still supply ingest at v2_cli_run; deleting a fixture root now fails the instrument walk that actually executes those trees. Co-authored-by: Cursor <cursoragent@cursor.com>
review 78075The first finding is already gone on this head ( The dangling-fixture finding was true of the earlier head: |
…nd refuse undeclared lens roots. source_reference_repoint now stamps origin on SourceRootWalkTask and reads SourceRootWalkedPath.path. The module-graph lens matches each declared file to a pool root and admits that argument; it no longer defaults unknown paths to DagTree. Co-authored-by: Cursor <cursoragent@cursor.com>
review 78082Both findings were true of the previous head; they are fixed on
|
…ed tree tag. The sentinel is not a walked source; source_ref_read_failed_diagnostic only needed the path for its locus. Co-authored-by: Cursor <cursoragent@cursor.com>
review 78086The first finding was already gone on The second finding was true: the internal-impossible arm constructed |
The Bool scan could only be true when a first declaring file existed, so the one-file fallback was unreachable from the census step. Co-authored-by: Cursor <cursoragent@cursor.com>
The eval-driver ingest now admits tree identity from the --source-root argument, so a scratch directory that is not dag or src/v2 exited 2 before tokenize could refuse the specimen. Co-authored-by: Cursor <cursoragent@cursor.com>
review 78104The advisory on — sent from clever-tern-634 |
An if whose condition used || after a parenthesized equality was typed as String vs Optional, so the floor refused module_graph. Co-authored-by: Cursor <cursoragent@cursor.com>
…owth row. Ctrl asked for a checkable hand-Rust receipt; the walk extends walk_cli_door and dissolves with that row's existing v1-hand-queue-drain trigger. Co-authored-by: Cursor <cursoragent@cursor.com>
review 78111Receipt for — sent from clever-tern-634 |
The inner if still typed Present as String versus Absent as Optional, so the floor kept refusing module_graph. Co-authored-by: Cursor <cursoragent@cursor.com>
review 78126PR body updated to the walk-origin cutover ( — sent from clever-tern-634 |
Duplicate-module both-files already lives at the census fold; two-root and collision inhabitance is walk_native_ingest_layout_controls. Each v2_cli_run enrolment was 150k–634k eval steps against a 72k ceiling. Co-authored-by: Cursor <cursoragent@cursor.com>
briansrls
left a comment
There was a problem hiding this comment.
APPROVE at exact head 323bf36d98bb71c244ec12e73ac3a3bba515eb02. No blocking finding in the root-origin repair, duplicate-module refusal, or the replacement coverage for the two deleted expensive CLI claims.
(a) The surviving discriminators execute on required PR CI
The body understates the evidence: the native filesystem/layout controls are NOT skipped on this PR's required emit-build route. In exact-head workflow 37843915123, emit-build job 113539891208 runs the v2-native-cli instrument. Its log at 2026-10-08T22:01:57.4210025Z reports:
v2-native-cli: native-ingest layout controls held (unrecognized copied root, /src/ under dag, two roots, both-files duplicate)
The log then records successful door execution and instrument completion. This is not just compilation of a Rust test or the author's BuildBuddy dispatch. walk_cli_door unconditionally calls walk_native_ingest_layout_controls(binary, workspace)?; a failed layout control returns an error to the required instrument. Rust-unit-tests being skipped does not skip this call.
The four cases actually launch the emitted executable. They require an unsupported copied root to produce the usage-refusal exit, empty stdout and a diagnostic naming that root; a src subdirectory inside a declared dag root to emit successfully; two admitted roots to emit successfully with nonempty output; and duplicate modules to produce the refusal exit plus the duplicate reason and both collision filenames. Missing fixture directories refuse rather than skip.
The deleted two_files_declaring_one_module_refuse_as_a_census_collision_holds is covered at the native CLI exit boundary by that executed duplicate case, and at the supplied emission boundary by v2.test.claim.self_host.closure_emission::two_files_declaring_one_module_refuse_the_emission_at_the_census_holds. The latter requires Rejected, the exact closure_census_duplicate_module reason, and both ce_duplicate_a_read and ce_duplicate_b_read in the located diagnostic chain; Accepted is false.
The deleted distinct_module_paths_across_two_roots_do_not_refuse_as_a_collision_holds accepted even certain non-collision refusals. The surviving native two-root control instead requires successful emission with nonempty output, while the small source_root_tagging claims check the actual DagTree/V2Tree values. This does not weaken that positive.
I downloaded the exact-head floor artifact 11582556943 and verified its ZIP SHA256 against the current GitHub artifact metadata: 773ab03b676caf76aaa7e17ee7f39799c28c6043193b39f05da634c20b54c525. Its TSV records all six source_root_tagging claims and the duplicate-module closure-emission claim passing, with verdict_reached=true and observed costs. The duplicate claim is 453 eval steps. Thus the small boundary checks and the real native route both remain executing. No restoration of the two expensive claims, extra cost exemption, or new test lane is requested.
This verifies current passing regression controls, including their refusal cases. I did not independently replay a pre-fix compiler or a mutation; the author's BuildBuddy run 4a2b27c6 remains separate author-run evidence.
(b) Root naming is an explicit supported-layout restriction, not eliminated path dependence
The production walker seeds each task with the root argument as origin, preserves that origin while descending, and transfers it through SourceRootWalkedPath to SourceRootFile. source_root_ingest_from_files reads file.origin. source_root_for_storage_path is deleted; the file's own path no longer chooses its tree.
admit_source_root_argument remains a name-based admission of the two supported layouts: dag and src/v2, including the explicitly handled relative/absolute and trailing-slash forms. Any other discovered origin returns SourceRootArgumentUnrecognized, then SourceRootIngestFromFilesUnrecognizedRoot carrying the origin and file, and the CLI maps that refusal to CliUsageRefused. There is no catch-all DagTree and no search for an interior /src/ segment in a file path.
I accept this as the declared CLI root-layout contract at this head, not a semantic detector of arbitrary filesystem contents. The RFM and source comment name that bound and preserve the copied-root rejection. A directory merely renamed dag is treated as an asserted dag root; this PR does not certify its historical provenance or support arbitrary renamed or mixed-root dumps. Likewise this review does not claim that every unused/empty extra root is independently validated by this per-file admission. The guarantee is faithful tagging from a supported discovering root, not elimination of all naming conventions. The old per-file heuristic is not retained as a fallback.
The RFM distinguishes the original mis-tagging incident from an actual duplicate declared path. Its ceiling remains acceptance-time refusal/valid root tagging, not a claim that duplicate files are unwritable or that all native unbound-name failures are fixed. No additional root-introspection project or new root-role flag is required for this bounded repair.
(c) Duplicate modules block emission
The both-files diagnostic is consumed by the rejecting closure-emission outcome and by the CLI refusal path. It is not a census-only warning. The retained floor claim explicitly rejects an Accepted emission, and the required native control checks the process refusal exit with the duplicate diagnostic and both file names. Distinct declarations remain the successful native control. The repair adds the first locus without replacing the second or changing duplicate membership into a nonblocking report.
The origin-field adaptation in the walker/readers and emitted realization preserves errors rather than defaulting an unknown root. Exact-head generated CI passed all-target lint and the one-emission stage0 mirror check. I found no additional blocking fail-open arm in the reviewed changes.
Non-blocking PR-body correction and verification scope
Correct the test-plan sentence saying native-CLI filesystem controls are skipped on PR CI, and remove the two deleted v2_native_cli_test claims from its active test inventory. The code and exact-head required log already provide stronger evidence than that sentence says. Do not conflate any separately skipped full fault/restoration rebuild with these executing layout controls.
Workflow 37843915123 passed seed, generated, floor, emit-build and witnesses; Rust unit tests were skipped. Local execution here was artifact hashing/TSV inspection only. I did not run a local compiler, author mutation, remote dispatch or live infrastructure operation. No merge or enqueue performed. Land through the normal required checks on the composed revision.
|
Superseded by #13641 (v1 closeout): this head is an ancestor of integration/v1-closeout. |
Summary
std.algebra.dag/std/algebra.dagismodule std.algebra;src/v2/std/algebra.dagismodule v2.std.algebra. A copied single root used to guessDagTreefrom the file path, so resolve later reportedresolve_unbound_name_is_declared_elsewhere.--source-root(SourceRootFile.originfromgunbc.source_root_read) admitted byadmit_source_root_argument: adagorsrc/v2directory, otherwise a typed refuse (SourceRootArgumentUnrecognized). There is noDagTreedefault and no/src/path-component classifier.src/folder under adagroot still emits; two admitted roots emit; a duplicate declared module names both files (closure_census_duplicate_module, same grain as the seed'smodule_path_collision_panic_message).gunbc.recurring_failure_mode.two_source_files_claiming_one_module_path_misdiagnosed. Native inhabitance iswalk_native_ingest_layout_controlson the existingwalk_cli_doorinstrument, accounted ongunbc.source_root_eval_driver_seed_growth.Operator merge only.
Test plan
dag-named scratch root)./src/under dag, two roots, both-files duplicate) viagunbc test //gunbc/instruments:v2-native-cli/walk_native_ingest_layout_controls.source_root_tagging_testandclosure_emission_testcensus collision. The twov2_native_cli_testCLI emit claims were deleted in 323bf36 because they exceeded the new-witness step budget; the native walk covers them.The native-CLI filesystem controls DO run on PR CI: emit-build runs
//gunbc/instruments:v2-native-cliand prints "native-ingest layout controls held". Only therust-unit-testslane is skipped on pull_request. To reproduce locally, use this exact remote command (BuildBuddy linux/amd64,ctrl-build --remote, cgroupmemory.maxrequired orHostBudgetUnreadable):