Repository navigation
Relative selectors resolve and unknown ones error instead of running cwd - #293
Open
calvin-archastro wants to merge 1 commit into
Open
calvin-archastro wants to merge 1 commit into
calvin-archastro wants to merge 1 commit into
Conversation
Contributor
Author
|
ArchDev is watching this pull request. State: watching (cycle 12 of 12 for head 0c2bcf2).
Paused by the |
calvin-archastro
force-pushed
the
factory/tsk_0UyNNBodV8CtLwmDNWVYiK-e1d2d9d5-3768-428b-b126-
branch
4 times, most recently
from
November 1, 2026 12:46
52c28fc to
51b1c02
Compare
calvin-archastro
force-pushed
the
factory/tsk_0UyNNBodV8CtLwmDNWVYiK-e1d2d9d5-3768-428b-b126-
branch
from
November 1, 2026 16:51
51b1c02 to
0c2bcf2
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Review on ArchCode
Epic: Aster bug bash fixes
Task: Relative selectors resolve and unknown ones error instead of running cwd
Problem and author intent
aster <target>project selection only reliably handled//addresses (viaProjectSelector) and a narrow./...prefix. Other documented forms were dropped or mis-resolved, and an empty selector list fell back to “the project containing cwd.”Reported failure modes (issue 267):
services/apiandservices/*were not kept as selectors, so selection failed withNo projects specified...../...was retained but prefix matching reportedNo projects found matching: ./....//services/api/was compared literally and reportedProject not found.totally-bogus,../web, and../...were also dropped. Withprojectsempty, cwd detection selected the containing project and exited 0, so a typo or a sibling path silently ran the cwd project (for example//services/api:test).Intent: keep non-flag selectors, resolve cwd-relative directories and the agreed glob forms to workspace-relative project paths, treat
.as the project containing cwd, accept a trailing slash on//addresses, and fail closed on anything unmatched or outside the workspace.//address grammar and run-target addresses stay unchanged. README was left unchanged because the documented forms are implemented rather than dropped.What changed
Selection now classifies each selector before matching.
services/api,services/*,../web) are stored../and../are both relative forms. A trailing/is stripped from//addresses and exclusions (//itself is left alone), so//services/api/is the same address as//services/api..still means cwd detection, not a path../..are collapsed logically (logical_normalize, no filesystem canonicalize). The result must stay under the workspace root. Escape fails withselector <s> resolves outside the workspace <root>./...recursive prefix (./...,../...,path/..., empty prefix = all projects);*= one path segment;**= any depth. Other glob metacharacters (?,[, a*that is not its own segment,..inside a glob) are rejected.//forms still go throughProjectSelector.//...still means all.unrecognised project selector '<s>'; use //path, //path/..., ./..., or .. Cwd fallback runs only when no selector was given, or when./use_cwdis explicit. That fallback is the longest discovered directory that is a prefix of cwd (a root project only when cwd is the workspace root), not every project under cwd.Scope
src/cli/run.rs— parse, resolve, glob match,select_projects, exclusions, and unit tests.tests/integration.rs— binary-level selector coverage.Risk assessment
High. This path runs for every target command; a wrong match runs or skips the wrong projects, including in CI. Normalization is logical, not
canonicalize, so the workspace root string in outside-workspace errors depends on the root the CLI already resolved (the integration test compares against a canonicalized temp root; unit tests use a literal/workspace). Relative exclusions swallow resolution errors via.ok(), so a bad exclusion can silently exclude nothing. An exact//address that is present in the graph or address map but does not match a discovered project still does not get a new error (same shape as the old “in graph but unmatched” branch).User impact
services/api,./path,../web,./...,../...,path/...,services/*, andservices/**select projects against cwd instead of being ignored or falling through to the cwd project.*does not match nested projects (services/*isservices/apiandservices/web, notservices/api/v2).**does.services/api*is unrecognised.aster test //services/api/behaves like//services/api..still runs only the project containing cwd.Testing
Patch evidence adds these tests. Presence in the diff is not proof they passed.
src/cli/run.rs):test_parse_run_args_keeps_relative_selectors,test_parse_run_args_strips_trailing_address_slash,test_select_projects_relative_directory_and_globs,test_select_projects_parent_selectors_resolve_against_cwd,test_select_projects_unknown_selector_does_not_fall_back_to_cwd,test_select_projects_outside_workspace_names_selector_and_root,test_select_projects_trailing_slash_address_matches_exact_project,test_select_projects_unsupported_glob_is_unrecognised.tests/integration.rs::relative_and_glob_selectors_select_expected_projects. It builds a temp workspace (services/api,services/web,services/api/v2,libs/core), runs theasterbinary, and asserts PASS lines, non-matches, exit status, the unrecognised-selector text, and the outside-workspace text forservices/api,services/*,services/**,./...,//services/api/,bogus,../web,../api,../..., and...Advisory pipeline outcomes (all reported succeeded, exit 0): format, lint, test, fix, reformat, verify-lint, verify-test, commit. The fix note cites
cargo clippy --locked --workspace --all-targets --all-features -- -D warningsandcargo test --locked --workspace --all-targets --all-features, both exit 0, and commit reported no remediation files. Verification gap: upstream check logs are truncated. The supplied test excerpts name unrelated cases (migrations, server, startup, ui, upgrade) and do not namerelative_and_glob_selectors_select_expected_projectsor the newcli::runtests. This review does not treat CI as having shown those cases pass. Bare.and../*are not in the integration excerpt (../*is covered by the parent-selector unit test).Follow-ups and known issues
*/**only and the new error strings.cargo test --lib cli::runandcargo test --test integration relative_and_glob_selectors_select_expected_projectsif those names must be confirmed in CI output.