Skip to content

ci(cmux-next): a target the manifest declares passes graph coverage until the base autofix adds it - #23364

Merged
teamleaderleo merged 2 commits into
feat-cmux-nextfrom
orphan/route-graph-pending-targets
Oct 21, 2026
Merged

teamleaderleo merged 2 commits into
feat-cmux-nextfrom
orphan/route-graph-pending-targets

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator

The target graph Packages/macOS/CmuxNext/ci-target-graph.json is bot-only (#21946). A pull request can't commit it, and the base autofix regenerates it after the merge. But GraphCoversThePackage.test_every_source_and_test_directory_is_a_target requires every target directory to already be in the graph. So every PR that adds a SwiftPM target fails cmux-next checks (Tier routing) with a red it can't fix. #21818 is one example: it adds CmuxNextAgentIntegrations and CmuxNextAgentIntegrationsTests.

What changes:

  • A target directory that Package.swift declares but the graph doesn't list yet now passes. The docs already say a manifest change routes every tier, so such a PR isn't under-tested in the meantime.
  • A Swift directory that the manifest doesn't declare still fails, as before.
  • New test test_the_manifest_reader_finds_the_graph_targets: the manifest reader must find every graph target whose directory exists. This keeps the allowance from quietly letting directories through if the regex breaks.

Only tests/test_cmux_next_route.py changes. I didn't run it on cosmic-gray; CI's cmux-next checks runs it. I checked by hand that the regex finds 121 of the graph's 122 targets. The one it misses is the binary target CCmuxAppFFI, which has no path and is skipped.

No capture: CI test only, no UI.

Agent session: cosmic-gray

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.


Summary by cubic

Fixes cmux-next Tier routing so PRs that add a SwiftPM target to Package.swift no longer fail the graph coverage check. ci-target-graph.json is bot-only (#21946): a PR can't commit it, and the base autofix regenerates it after merge, so every manifest-changing PR like #21818 went red with no way to fix it. A target the manifest declares now passes until the autofix adds it, while a directory the manifest doesn't declare still fails. The allowance maps Tests/<name> to test targets and Sources/<name> to others, so a test directory can't pass for a regular target or vice versa. A companion test verifies the manifest reader finds every graph target whose directory exists, keeping the allowance from quietly letting stray directories through.

Written for commit 33cd7d9. Summary will update on new commits.

View guided diff Turn on auto-fix

…eclares until the base autofix adds it

The target graph is bot-only (#21946), so a pull request that adds a SwiftPM
target cannot add it to ci-target-graph.json. GraphCoversThePackage then failed
Tier routing on every such PR (e.g. #21818, CmuxNextAgentIntegrations). A target
directory the manifest declares now passes until the base autofix regenerates the
graph; a directory the manifest does not declare still fails. A second test keeps
the manifest reader honest against the graph.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15cfc-663f-7650-b9f7-64d198c6b88b&host=cosmic-gray
@teamleaderleo teamleaderleo added the risk:low Risk tier (docs/ci/risk-tiers.md) label Oct 21, 2026
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Owned by orphan-sweep on cosmic-gray · session

@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

cmux status (one comment, edited in place; each line is updated by the job that owns it)

Review 33cd7d9: HQ feed card · gallery before/after runs · checks

App of this head (app.zip, kept 1 day)

gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr23364 -f ref=orphan/route-graph-pending-targets -f platform=macos -f backend_mode=local

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (claude-opus review subagent): SHIP at 3c14380

I read the diff, docs/ci/cmux-next-tiers.md, scripts/ci/cmux_next_route.py, .github/workflows/cmux-next-base-autofix.yml and scripts/cmux-next/base-autofix*.sh, and checked the regex against this checkout's Package.swift. I didn't run the test suite.

1. The reasoning holds.

  • Package.swift is in FULL_INPUTS, so a PR that changes it calls result.everything(...). That sets every tier, and swift_targets becomes all (the filter is empty), so the new test target runs even though the graph doesn't list it.
  • Without a manifest change, a file under an undeclared target directory still routes every tier ("in the CmuxNext package but in no target").
  • The docs already say this: the graph is bot-only (Stop parallel feat-cmux-next PRs from conflicting in generated files and registries #21946), a manifest change routes every tier, and no check compares the graph with the manifest. The comment on graph checks in cmux-next.yml (around line 3032) agrees. The old assertion conflicted with that rule; this change makes them agree.

2. It doesn't hide a real failure on base.

  • The base autofix regenerates the graph whenever anything under Packages/ changes (autofix-areas.sh: area graph 'Packages/|...'). It runs swift package dump-package and then ci-target-graph.py, under set -euo pipefail, so a failed dump or generation fails the run.
  • The graph isn't in base-autofix-frozen.txt.
  • The tip already has a bot commit (chore(next): base autofix at 32e14804c397), so the push isn't stuck in dry-run.
  • One side effect: a stale graph used to turn Tier routing red on every PR. After this change, the failed autofix run is the only signal. base autofix: make the CI target graph on its own Linux job, never behind the Mac exports #23354's body describes exactly that case (Tier routing red on every PR from 08:38Z to 10:09Z on 10-21). This PR stops that failure, which is what the docs intend. While the graph is stale, an edit inside the new target still routes every tier. The only gap is a dependency-only change: it won't select the new test target until the autofix lands. That is fine.

3. The regex is correct for this manifest.

  • It finds 121 names, and they are exactly the graph's targets minus CCmuxAppFFI. That one is declared as let appFFI: Target = .binaryTarget(...), has no path in the graph, and is skipped on both sides.
  • No target in the graph uses a custom path:. The only path: arguments in Package.swift are on .package(path:) dependencies.
  • Every Swift directory under Sources/ and Tests/ is in the graph today.
  • The other name: matches (products, package dependencies) are correctly not picked up.
  • \s* covers the multi-line .target(\n name: form, and SwiftPM requires name: as the first argument.

4. The test is correct and in the file's style. It uses unittest with subTest and module-level helpers like tiers(), and needs no new registration, because the file is already in test-execution.toml and the Tier routing step. The companion test is a good guard: if the regex stops matching, it fails on the current graph instead of letting directories through silently.

Non-blocking nits:

  • The allowance matches the directory name only, not its kind. A stray Tests/Foo would pass when the manifest declares .target(name: "Foo"), which lives in Sources/Foo. To close this, map testTarget to Tests and the other kinds to Sources, and check f"{kind}/{name}".
  • A commented-out // .target(name: "X" would count as declared. It's unlikely to matter.
  • The docstring says "Every target uses its default path", but nothing enforces it. A future target with a custom path: wouldn't be caught by the companion test. The main assertion still holds through the graph's paths, so this is only a wording issue.

5. It doesn't duplicate an open PR. #23354 moves the graph regeneration to its own Linux job in the base autofix, which shortens the stale window. It doesn't touch this test, so the two PRs complement each other. I found no other open PR that changes GraphCoversThePackage.

…target

Review nit: Tests/Foo passed when Foo was a regular target. The manifest reader now
returns each target's default path, Tests/<name> for a test target and
Sources/<name> otherwise, and both tests compare paths.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15cfc-663f-7650-b9f7-64d198c6b88b&host=cosmic-gray
@blacksmith-sh

blacksmith-sh Bot commented Oct 21, 2026

Copy link
Copy Markdown

Found 5 test failures on Blacksmith runners:

Failures

Test View Logs
/tmp/tmp3dojsg8j/summary-missing.json (live):/summary-missing.json (live): View Logs
/tmp/tmpsj0zo57z/summary-fail.json (live):/summary-fail.json (live): View Logs
test_a_pull_request_that_changes_the_shortcut_inputs_still_rewrites_them (main.Sett
ingsOnlyArtifactLeavesTheActionFollowers.test_a_pull_request_that_changes_the_shortcut_
inputs_still_rewrites_them) (job='autofix')/
test_a_pull_request_that_changes_the_shortcut_inputs_still_rewrites_them (main.Sett
ingsOnlyArtifactLeavesTheActionFollowers.test_a_pull_request_that_changes_the_shortcut_
inputs_still_rewrites_them) (job='autofix')
View Logs
test_regenerated_action_exports_bring_the_followers (main.SettingsOnlyArtifactLeave
sTheActionFollowers.test_regenerated_action_exports_bring_the_followers) (job='autofix'
)/
test_regenerated_action_exports_bring_the_followers (main.SettingsOnlyArtifactLeave
sTheActionFollowers.test_regenerated_action_exports_bring_the_followers) (job='autofix'
)
View Logs
test_settings_exports_alone (main.SettingsOnlyArtifactLeavesTheActionFollowers.test
_settings_exports_alone) (job='autofix')/
test_settings_exports_alone (main.SettingsOnlyArtifactLeavesTheActionFollowers.test
_settings_exports_alone) (job='autofix')
View Logs

Fix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (claude-opus review subagent): SHIP at 33cd7d9

I reviewed the diff since 3c14380. Only tests/test_cmux_next_route.py changed.

  • manifest_target_paths() maps testTarget to Tests/<name>, and target and executableTarget to Sources/<name>. These are SwiftPM's default paths. A stray Tests/Foo next to a regular target Foo now fails, so my first nit is fixed.
  • I ran an independent check on this checkout: the reader gives 121 paths, and they are exactly the graph's 121 non-null paths, with nothing extra or missing.
  • The companion test now asserts each graph target's path, not its name. A future target with a custom path: would fail it, so the docstring's "No target in the manifest sets its own path" is now enforced. That covers my third nit too.
  • The allowance still applies only to a directory that is both missing from the graph and declared at that exact path, so an undeclared directory still fails.

Still open, and non-blocking: a commented-out .target(name: in Package.swift would count as declared. Everything in my earlier review at 3c14380 (routing, base autofix, no duplicate PR) still holds.

@teamleaderleo
teamleaderleo merged commit f32bde6 into feat-cmux-next Oct 21, 2026
@teamleaderleo
teamleaderleo deleted the orphan/route-graph-pending-targets branch October 21, 2026 12:00
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Bead cx-059o6 created for this PR by the cmux coordinator.

@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:low Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant