Skip to content

feat: list uncommitted files for git-dirty-check#2669

Merged
comatory merged 4 commits intomainfrom
ondrej/eng-9246-git-dirty-check-list-dirty-files-in-pr-comment
Mar 20, 2026
Merged

feat: list uncommitted files for git-dirty-check#2669
comatory merged 4 commits intomainfrom
ondrej/eng-9246-git-dirty-check-list-dirty-files-in-pr-comment

Conversation

@comatory
Copy link
Copy Markdown
Contributor

@comatory comatory commented Mar 19, 2026

Summary by CodeRabbit

  • Chores
    • Enhanced CI workflow to display modified files in pull request comments when git checks fail
    • Updated workflow trigger to include changes in GitHub Actions configuration files

Checklist

List dirty files in the PR comment
Screenshot 2026-03-19 at 16 11 47

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 19, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a79353eb-b8eb-4e3b-8f4e-6e6c66f074ed

📥 Commits

Reviewing files that changed from the base of the PR and between 81c0e8d and a817334.

📒 Files selected for processing (1)
  • .github/workflows/dummy-ci.yaml

Walkthrough

The git-dirty-check composite action is enhanced to collect and display changed files in a collapsible PR comment section when uncommitted changes are detected. Additionally, the workflow trigger is updated to watch for changes in the composite actions directory.

Changes

Cohort / File(s) Summary
Git Dirty Check Enhancement
.github/actions/git-dirty-check/action.yaml
Added a new dirty-files-list bash step that executes on failure, collecting changed filenames via git diff --name-only, formatting them as a bullet list, and outputting to GITHUB_OUTPUT. Extended the failure-path PR comment with a collapsible HTML <details> section displaying the dirty files list.
Workflow Trigger Update
.github/workflows/dummy-ci.yaml
Expanded pull_request.paths filter to include .github/actions/**, triggering the workflow when composite action files are modified in addition to existing scripts/**, .github/workflows/image-release.yml, and docs-website/** paths.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding functionality to list uncommitted files when git-dirty-check fails, which is the primary modification across the changeset.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

📝 Coding Plan
  • Generate coding plan for human review comments

Comment @coderabbitai help to get the list of available commands and usage tips.

@comatory comatory force-pushed the ondrej/eng-9246-git-dirty-check-list-dirty-files-in-pr-comment branch from 81c0e8d to 482a5cc Compare March 19, 2026 15:14
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
proto/wg/cosmo/common/common.proto (1)

25-25: Reconsider the enum naming for consistency and clarity.

The enum value name ERR_TEST_DIRTY_CHECK includes "TEST" which breaks consistency with all other error codes in the EnumStatusCode enum. Every other error code uses the pattern ERR_<DESCRIPTION> (e.g., ERR_DEPLOYMENT_FAILED, ERR_SCHEMA_MISMATCH_WITH_APPROVED_PROPOSAL, ERR_BAD_REQUEST). Since this is for a production git-dirty-check feature, consider ERR_DIRTY_CHECK or ERR_GIT_DIRTY_CHECK to align with the existing enum pattern and avoid ambiguity about whether this is testing-only code.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@proto/wg/cosmo/common/common.proto` at line 25, Rename the enum value
ERR_TEST_DIRTY_CHECK in EnumStatusCode to a production-appropriate name (e.g.,
ERR_DIRTY_CHECK or ERR_GIT_DIRTY_CHECK) to match the ERR_<DESCRIPTION> pattern;
update the enum declaration in common.proto and then search & replace all
references to ERR_TEST_DIRTY_CHECK across the codebase (including any generated
code, switch/case statements, tests, and serialization/deserialization usage) to
the new symbol to preserve compatibility.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/actions/git-dirty-check/action.yaml:
- Around line 15-19: The heredoc in the run step uses the static marker "EOF"
which can collide with file names; change the multiline GITHUB_OUTPUT write to
use a unique delimiter (e.g., generate a GUID or use a fixed unlikely token)
instead of "EOF": capture the output in FILES as before, create a unique
delimiter variable (DELIM), then write "files<<$DELIM" and terminate with
"$DELIM" when echoing to $GITHUB_OUTPUT so that FILES (and names containing EOF)
are preserved; update the run block that defines FILES and writes to
$GITHUB_OUTPUT accordingly.

In `@controlplane/src/core/bufservices/namespace/getNamespace.ts`:
- Around line 29-33: The snippet in getNamespace.ts has inconsistent indentation
for the lines calling namespaceRepo.byId and namespaceRepo.byName (they use 8
spaces instead of the project's 2-space style) and appears unrelated to the
git-dirty-check feature; fix by normalizing indentation to the project's 2-space
style around the if (req.id) block (adjust the lines calling
namespaceRepo.byId(req.id) and namespaceRepo.byName(req.name)), and if this
formatting change was unintentional, revert it so the PR only contains changes
relevant to adding uncommitted file listing to git-dirty-check; run the
repository's formatter/linter or the git-dirty-check locally before committing
to ensure no stray formatting changes remain.

---

Nitpick comments:
In `@proto/wg/cosmo/common/common.proto`:
- Line 25: Rename the enum value ERR_TEST_DIRTY_CHECK in EnumStatusCode to a
production-appropriate name (e.g., ERR_DIRTY_CHECK or ERR_GIT_DIRTY_CHECK) to
match the ERR_<DESCRIPTION> pattern; update the enum declaration in common.proto
and then search & replace all references to ERR_TEST_DIRTY_CHECK across the
codebase (including any generated code, switch/case statements, tests, and
serialization/deserialization usage) to the new symbol to preserve
compatibility.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b694fdec-0d10-477e-8e9b-962fc49c6642

📥 Commits

Reviewing files that changed from the base of the PR and between 0e5fa81 and 81c0e8d.

📒 Files selected for processing (3)
  • .github/actions/git-dirty-check/action.yaml
  • controlplane/src/core/bufservices/namespace/getNamespace.ts
  • proto/wg/cosmo/common/common.proto

Comment thread .github/actions/git-dirty-check/action.yaml
Comment thread controlplane/src/core/bufservices/namespace/getNamespace.ts Outdated
@wundergraph wundergraph deleted a comment from github-actions Bot Mar 19, 2026
@wundergraph wundergraph deleted a comment from github-actions Bot Mar 19, 2026
@comatory comatory marked this pull request as ready for review March 19, 2026 15:17
@comatory comatory requested review from a team as code owners March 19, 2026 15:17
@comatory comatory enabled auto-merge (squash) March 19, 2026 15:19
@comatory comatory merged commit f779fcc into main Mar 20, 2026
7 of 8 checks passed
@comatory comatory deleted the ondrej/eng-9246-git-dirty-check-list-dirty-files-in-pr-comment branch March 20, 2026 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants