From 03ae235261f268beccdeea45868383d754ac1373 Mon Sep 17 00:00:00 2001 From: Frans Caisar Ramadhan Date: Thu, 6 Aug 2026 13:19:42 +0700 Subject: [PATCH 1/2] fix(remediation): use conventional-commit PR titles and sync existing PRs Remediator PR titles now follow fix(): instead of a git-cascade: prefix. CreateOrUpdatePullRequest now edits the title/body of an already-open PR on the same head branch when they've drifted from the current fix, instead of leaving stale titles/bodies untouched on rerun. --- internal/github/write.go | 21 ++++++-- internal/github/write_test.go | 51 ++++++++++++++++++- internal/remediation/fixes/actions_pinned.go | 2 +- .../fixes/harden_runner_required.go | 2 +- internal/remediation/fixes/npm_ci_required.go | 2 +- internal/remediation/fixes/readme_exists.go | 2 +- 6 files changed, 70 insertions(+), 10 deletions(-) diff --git a/internal/github/write.go b/internal/github/write.go index b258403..2f21ec3 100644 --- a/internal/github/write.go +++ b/internal/github/write.go @@ -91,9 +91,11 @@ func CommitFiles(ctx context.Context, client *github.Client, owner, repo, branch return newCommit.GetSHA(), nil } -// CreateOrUpdatePullRequest opens a PR from head into base, or returns the -// URL of an existing open PR with the same head if one is already there — -// an upsert so re-running remediation doesn't create duplicate PRs. +// CreateOrUpdatePullRequest opens a PR from head into base, or updates the +// title/body of an existing open PR with the same head if one is already +// there — an upsert so re-running remediation doesn't create duplicate PRs +// but still picks up title/body changes (e.g. a remediator's PR title format +// changing between runs). func CreateOrUpdatePullRequest(ctx context.Context, client *github.Client, owner, repo, head, base, title, body string, labels []string, draft bool) (string, error) { existing, _, err := client.PullRequests.List(ctx, owner, repo, &github.PullRequestListOptions{ State: "open", @@ -104,7 +106,18 @@ func CreateOrUpdatePullRequest(ctx context.Context, client *github.Client, owner return "", fmt.Errorf("listing pull requests: %w", err) } if len(existing) > 0 { - return existing[0].GetHTMLURL(), nil + pr := existing[0] + if pr.GetTitle() != title || pr.GetBody() != body { + updated, _, err := client.PullRequests.Edit(ctx, owner, repo, pr.GetNumber(), &github.PullRequest{ + Title: &title, + Body: &body, + }) + if err != nil { + return "", fmt.Errorf("updating pull request: %w", err) + } + return updated.GetHTMLURL(), nil + } + return pr.GetHTMLURL(), nil } pr, _, err := client.PullRequests.Create(ctx, owner, repo, github.CreatePullRequest{ diff --git a/internal/github/write_test.go b/internal/github/write_test.go index 42c2dd7..2c75ebb 100644 --- a/internal/github/write_test.go +++ b/internal/github/write_test.go @@ -300,16 +300,24 @@ func TestCommitFiles_UsesProvidedAuthor(t *testing.T) { // — CreateOrUpdatePullRequest ———————————————————————————————————————————————— -func TestCreateOrUpdatePullRequest_ReturnsExistingWhenOpen(t *testing.T) { +func TestCreateOrUpdatePullRequest_ReturnsExistingWhenOpenAndUnchanged(t *testing.T) { mux := http.NewServeMux() mux.HandleFunc("/api/v3/repos/o/r/pulls", func(w http.ResponseWriter, r *http.Request) { switch r.Method { case http.MethodGet: - writeJSON(w, []*github.PullRequest{{HTMLURL: github.Ptr("https://github.com/o/r/pull/1")}}) + writeJSON(w, []*github.PullRequest{{ + Number: github.Ptr(1), + Title: github.Ptr("title"), + Body: github.Ptr("body"), + HTMLURL: github.Ptr("https://github.com/o/r/pull/1"), + }}) case http.MethodPost: t.Fatal("Create should not be called when an open PR already exists") } }) + mux.HandleFunc("/api/v3/repos/o/r/pulls/1", func(w http.ResponseWriter, r *http.Request) { + t.Fatal("Edit should not be called when title/body are unchanged") + }) client := newTestClient(t, mux) url, err := CreateOrUpdatePullRequest(context.Background(), client, "o", "r", "fix", "main", "title", "body", nil, false) @@ -321,6 +329,45 @@ func TestCreateOrUpdatePullRequest_ReturnsExistingWhenOpen(t *testing.T) { } } +func TestCreateOrUpdatePullRequest_UpdatesTitleWhenChanged(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/api/v3/repos/o/r/pulls", func(w http.ResponseWriter, r *http.Request) { + switch r.Method { + case http.MethodGet: + writeJSON(w, []*github.PullRequest{{ + Number: github.Ptr(1), + Title: github.Ptr("git-cascade: old title"), + Body: github.Ptr("old body"), + HTMLURL: github.Ptr("https://github.com/o/r/pull/1"), + }}) + case http.MethodPost: + t.Fatal("Create should not be called when an open PR already exists") + } + }) + var gotTitle, gotBody string + mux.HandleFunc("/api/v3/repos/o/r/pulls/1", func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPatch { + t.Fatalf("expected PATCH, got %s", r.Method) + } + var body github.PullRequest + json.NewDecoder(r.Body).Decode(&body) + gotTitle, gotBody = body.GetTitle(), body.GetBody() + writeJSON(w, &github.PullRequest{Number: github.Ptr(1), HTMLURL: github.Ptr("https://github.com/o/r/pull/1")}) + }) + client := newTestClient(t, mux) + + url, err := CreateOrUpdatePullRequest(context.Background(), client, "o", "r", "fix", "main", "new title", "new body", nil, false) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if url != "https://github.com/o/r/pull/1" { + t.Errorf("got url=%q", url) + } + if gotTitle != "new title" || gotBody != "new body" { + t.Errorf("got title=%q body=%q", gotTitle, gotBody) + } +} + func TestCreateOrUpdatePullRequest_CreatesAndLabels(t *testing.T) { var labeled []string mux := http.NewServeMux() diff --git a/internal/remediation/fixes/actions_pinned.go b/internal/remediation/fixes/actions_pinned.go index 2157068..486b020 100644 --- a/internal/remediation/fixes/actions_pinned.go +++ b/internal/remediation/fixes/actions_pinned.go @@ -64,7 +64,7 @@ func (f *actionsPinnedFixer) Remediate(ctx context.Context, client *github.Clien return &remediation.Fix{ Files: files, CommitMessage: "fix(security): pin GitHub Actions to commit SHA", - PRTitle: "git-cascade: pin GitHub Actions to commit SHA", + PRTitle: "fix(actions-pinned): pin GitHub Actions to commit SHA", PRBody: fmt.Sprintf( "Automated fix for the `actions-pinned` compliance rule.\n\nResolved and pinned:\n- %s\n\n_Opened automatically by git-cascade auto-remediation._", strings.Join(fixedRefs, "\n- "), diff --git a/internal/remediation/fixes/harden_runner_required.go b/internal/remediation/fixes/harden_runner_required.go index aff7e33..4c6dd6e 100644 --- a/internal/remediation/fixes/harden_runner_required.go +++ b/internal/remediation/fixes/harden_runner_required.go @@ -80,7 +80,7 @@ func (f *hardenRunnerFixer) Remediate(ctx context.Context, client *github.Client return &remediation.Fix{ Files: files, CommitMessage: "fix(security): add step-security/harden-runner as first step", - PRTitle: "git-cascade: add step-security/harden-runner to workflow jobs", + PRTitle: "fix(harden-runner-required): add step-security/harden-runner to workflow jobs", PRBody: fmt.Sprintf( "Automated fix for the `harden-runner-required` compliance rule.\n\nAdded `step-security/harden-runner@%s` as the first step of:\n- %s\n\n_Opened automatically by git-cascade auto-remediation._", hardenRunnerRef, strings.Join(fixedDescs, "\n- "), diff --git a/internal/remediation/fixes/npm_ci_required.go b/internal/remediation/fixes/npm_ci_required.go index 7d8ca8c..23d2dbb 100644 --- a/internal/remediation/fixes/npm_ci_required.go +++ b/internal/remediation/fixes/npm_ci_required.go @@ -75,7 +75,7 @@ func (f *npmCiRequiredFixer) Remediate(ctx context.Context, client *github.Clien return &remediation.Fix{ Files: files, CommitMessage: "fix(ci): use locked install commands in CI workflows", - PRTitle: "git-cascade: use locked Node.js install commands", + PRTitle: "fix(npm-ci-required): use locked Node.js install commands", PRBody: fmt.Sprintf( "Automated fix for the `npm-ci-required` compliance rule.\n\nLocked install commands:\n- %s\n\n_Opened automatically by git-cascade auto-remediation._", strings.Join(fixedDescs, "\n- "), diff --git a/internal/remediation/fixes/readme_exists.go b/internal/remediation/fixes/readme_exists.go index 47b1681..8eaec03 100644 --- a/internal/remediation/fixes/readme_exists.go +++ b/internal/remediation/fixes/readme_exists.go @@ -27,7 +27,7 @@ func (f *readmeExistsFixer) Remediate(ctx context.Context, client *github.Client {Path: "README.md", Content: []byte(content)}, }, CommitMessage: "docs: add README", - PRTitle: "git-cascade: add README.md", + PRTitle: "fix(readme-exists): add README.md", PRBody: "Automated fix for the `readme-exists` compliance rule.\n\nAdded a minimal README.md stub — please expand it with a real project description.\n\n_Opened automatically by git-cascade auto-remediation._", }, nil } From 75d1ca27dd18682d3bc82fd6ab4a593e9a1745da Mon Sep 17 00:00:00 2001 From: Frans Caisar Ramadhan Date: Thu, 6 Aug 2026 14:36:34 +0700 Subject: [PATCH 2/2] docs: document PR upsert title/body sync and PR title convention --- CLAUDE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 1ab8d09..c6d8028 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -52,7 +52,7 @@ Mirrors the checker plugin pattern exactly: each fixable rule ID registers a `Re Not every checker has a paired remediator — only rules where the fix is a deterministic file edit (see the "Auto-Remediation" section in `README.md` for the current list and the excluded-as-too-risky rationale). Detection logic that needs to be shared between a checker and its remediator (so they can never drift) is exported from the checker's file — e.g. `checks.FindUnpinnedActions`, `checks.FindNodeInstallViolations`, `checks.FindJobsMissingHardenRunner` — rather than re-derived from the checker's free-text `Result.Message`. -Git write operations live in `internal/github/write.go` (`EnsureBranch`, `CommitFiles`, `CreateOrUpdatePullRequest`) — separate from the read-only helpers in `repos.go`/`cache.go`. `CommitFiles` batches all of a `Fix`'s file changes into one commit via the Git Data API (blob → tree → commit → ref update) and is a no-op if the resulting tree is unchanged. `CreateOrUpdatePullRequest` upserts against an existing open PR with the same head branch rather than creating duplicates on repeat scans. +Git write operations live in `internal/github/write.go` (`EnsureBranch`, `CommitFiles`, `CreateOrUpdatePullRequest`) — separate from the read-only helpers in `repos.go`/`cache.go`. `CommitFiles` batches all of a `Fix`'s file changes into one commit via the Git Data API (blob → tree → commit → ref update) and is a no-op if the resulting tree is unchanged. `CreateOrUpdatePullRequest` upserts against an existing open PR with the same head branch rather than creating duplicates on repeat scans — if the title/body of that PR differ from the current fix (e.g. a remediator's PR title format changed), it edits them in place rather than leaving the existing PR untouched. Remediator `Fix.PRTitle` values follow Conventional Commits (`fix(): `), matching the repo's commit message convention. Remediation credentials (`--remediate-*` / `GIT_CASCADE_REMEDIATE_*`) have **no fallback** to scan or notify credentials — `resolveRemediateCredentials` errors out if `remediation.enabled` is true and none are set, since this is the one credential set that writes directly to scanned repositories.