ci: catch conflict markers and check desktop rust#8030
Conversation
There was a problem hiding this comment.
3 issues found across 2 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
|
Update after first CI run:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 33724761d6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if: steps.changed.outputs.has_desktop_rust == 'true' | ||
| working-directory: desktop/macos/Backend-Rust | ||
| run: | | ||
| cargo fmt --check |
There was a problem hiding this comment.
Format the Rust crate before enabling fmt
Because this PR also sets has_desktop_rust for .github/workflows/lint.yml changes, this new step runs on the PR itself. cargo fmt --help says it formats all bin/lib files of the current crate, and I ran cargo fmt --check in desktop/macos/Backend-Rust; it exits nonzero with rustfmt diffs across existing files such as src/auth.rs and src/services/firestore.rs. As written, the lint job is blocked before it reaches cargo check until the existing crate is formatted or the check is scoped differently.
Useful? React with 👍 / 👎.
| echo "has_personas=$(echo "$FILES" | grep -q '^web/personas-open-source/' && echo true || echo false)" >> $GITHUB_OUTPUT | ||
| # Also run the Rust check when this workflow changes so CI PRs dogfood | ||
| # the new guard instead of only validating YAML syntax. | ||
| echo "has_desktop_rust=$(echo "$FILES" | grep -qE '^desktop/macos/Backend-Rust/.*\.rs$|^\.github/workflows/lint\.yml$' && echo true || echo false)" >> $GITHUB_OUTPUT |
There was a problem hiding this comment.
Include Rust manifests in the backend check
This trigger only matches changed .rs files or this workflow file, so changes to desktop/macos/Backend-Rust/Cargo.toml or Cargo.lock skip the new cargo check --locked step. A dependency/version/feature edit can therefore introduce lockfile drift or a build failure while the Rust CI path stays disabled; include the Rust manifests (and any build script if added) in this predicate.
Useful? React with 👍 / 👎.
|
Addressed the new review comments:
Validation: |
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be6245106f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| run: | | ||
| FILES=$(grep -E '^desktop/macos/Backend-Rust/.*\.rs$' /tmp/changed-files.txt || true) | ||
| if [ -n "$FILES" ]; then | ||
| echo "$FILES" | xargs rustfmt --check |
There was a problem hiding this comment.
Pass the Rust edition to rustfmt
For changed desktop Rust files this invokes rustfmt directly, so it does not read the crate's edition = "2021" from Cargo.toml; rustfmt --help exposes --edition, and locally rustfmt --check desktop/macos/Backend-Rust/src/routes/health.rs fails with async fn is not permitted in Rust 2015 even though the crate compiles as Rust 2021. Any PR touching an async .rs file will therefore fail this lint step before cargo check; pass --edition 2021 here or run formatting through Cargo with the crate manifest.
Useful? React with 👍 / 👎.
|
Addressed the latest review comments:
Validation: |
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/lint.yml">
<violation number="1" location=".github/workflows/lint.yml:142">
P2: The CI rustfmt invocation is not path-safe: `xargs` splits filenames on whitespace, so changed Rust files with spaces in their path will break formatting checks.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
| run: | | ||
| FILES=$(grep -E '^desktop/macos/Backend-Rust/.*\.rs$' /tmp/changed-files.txt || true) | ||
| if [ -n "$FILES" ]; then | ||
| echo "$FILES" | xargs rustfmt --edition 2021 --check |
There was a problem hiding this comment.
P2: The CI rustfmt invocation is not path-safe: xargs splits filenames on whitespace, so changed Rust files with spaces in their path will break formatting checks.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/lint.yml, line 142:
<comment>The CI rustfmt invocation is not path-safe: `xargs` splits filenames on whitespace, so changed Rust files with spaces in their path will break formatting checks.</comment>
<file context>
@@ -139,7 +139,7 @@ jobs:
FILES=$(grep -E '^desktop/macos/Backend-Rust/.*\.rs$' /tmp/changed-files.txt || true)
if [ -n "$FILES" ]; then
- echo "$FILES" | xargs rustfmt --check
+ echo "$FILES" | xargs rustfmt --edition 2021 --check
fi
cd desktop/macos/Backend-Rust
</file context>
| echo "$FILES" | xargs rustfmt --edition 2021 --check | |
| printf '%s\n' "$FILES" | while IFS= read -r file; do rustfmt --edition 2021 --check "$file"; done |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 254c2bc8fe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| conflict_found=0 | ||
| while IFS= read -r file; do | ||
| [ -n "$file" ] || continue | ||
| if git grep --cached -n -I -E '^(<<<<<<<|>>>>>>>)' -- "$file"; then |
There was a problem hiding this comment.
Re-scan conflict markers after staging formatter output
This checks only the cached index before the hook's later formatter blocks mutate the index with git add, so a partially staged firmware file can still commit markers: stage a clean hunk in omi/*.c, leave an unstaged <<<<<<< hunk, and the later clang-format -i/git add path stages the whole file after this scan has already passed (I checked clang-format exits 0 and leaves that marker line intact). Move this scan to the end or rerun it after every formatter has re-staged files.
Useful? React with 👍 / 👎.
Summary
Adds static guards for the failure mode caught on #7951:
<<<<<<</>>>>>>>)rustfmton changed macOS desktop Rust backend filescargo check --lockedfor macOS desktop Rust backend changes.github/workflows/lint.ymlitself changes, so this PR measures the new CI costWhy
The existing lint workflow only covered Dart, backend Python, ARB, firmware C/C++, and selected web packages. Rust files under
desktop/macos/Backend-Rust/**/*.rscould pass PR lint even with syntax errors or conflict markers.Local validation / timing
Result: passed.
Conflict-marker scan over this PR's changed files:
Result:
elapsed=0:00.00 user=0.00 sys=0.00Pre-commit hook on this PR's staged files (no Rust files staged, so conflict scan only):
/usr/bin/time -f 'elapsed=%E user=%U sys=%S' scripts/pre-commitResult:
elapsed=0:00.01 user=0.00 sys=0.01I also verified the new conflict-marker pattern catches the current #7951 markers in
desktop/macos/Backend-Rust/src/routes/chat_completions.rs.CI timing from this PR
This PR intentionally ran the desktop Rust backend guard because
.github/workflows/lint.ymlchanged.cargo fmt --checkacross the whole crate failed in 43s due to existing unrelated rustfmt drift.cargo check --locked.Lint & Format Check: passed in 1m33s.Check desktop Rust backendstep: 48s.The universal conflict-marker CI step completed within the same second as changed-file detection in GitHub's step timestamps.