Two open scaffolder bugs, fixed — and what I read on the way there

I started a contribution session with two open scaffolder bugs on backstage/backstage and a vague plan to “just land a couple of PRs.” Three days later I’d read more of the monorepo than I expected, fixed a latent bug that wasn’t in the issue tracker, and learned exactly which CI job fails when you forget to regenerate the public API report.

This is the field report. Two issues, two PRs, three lessons.

The two bugs

#34098 — gitlab:repo:push scaffolder action returns 400 Bad request from GitLab when there are no file changes to commit (e.g. re-running a template against an up-to-date branch). The error message blames the user’s commitAction config or claims files don’t exist remotely. Neither is true.

#33531 — a recent security fix stripped all secrets from scaffolder dry-run executions, including user-supplied task secrets that integration test suites depend on. The fix was correct in spirit, too aggressive in scope.

Both are area:scaffolder, both type:bug, both shippable as small PRs.

Fix #1: the empty-actions short-circuit (PR #34480)

When the GitLab action computes actions for Commits.create, it filters out files where the file action resolves to skip. If the workspace is unchanged, that filter empties the array. GitLab then rejects:

400 Bad request - Provide at least one action, or set allow_empty to true

The reporter’s suggested fix was reasonable: detect the empty list, log a warning, skip the API call. Implementation:

if (actions.length === 0 && !allowEmpty) {
  ctx.logger.warn(
    `No file changes to commit to ${repoID} on branch '${branchName}'; ` +
    `skipping commit. Set 'allowEmpty: true' to create an empty commit.`,
  );
  ctx.output('projectid', repoID);
  ctx.output('projectPath', repoID);
  return;
}

The output schema needed commitHash to become optional — no commit means no hash. That’s a public-API change, which sets up the first CI surprise (below).

Fix #2: keep env secrets blocked, let task secrets through (PR #34481)

The commit that introduced the bug (4f5ed06d) is two lines in NunjucksWorkflowRunner.ts:

// before
secrets: this.environment?.secrets ?? {},
secrets: task.secrets ?? {},

// after the security fix
secrets: task.isDryRun ? {} : this.environment?.secrets ?? {},  // ok
secrets: task.isDryRun ? {} : task.secrets ?? {},               // too much

Environment secrets are server-configured — those should be stripped during dry-run because the dry-run endpoint can be called by anyone who can submit a template. Task secrets are caller-supplied — if you can post them, you already had them. Stripping them breaks the integration-test pattern described in the issue.

The fix is one character (well, ~ten characters) — drop the ternary on line 403:

secrets: task.secrets ?? {},

And the test renames from “should not pass environment secrets or task secrets” to “should strip environment secrets but pass user-supplied task secrets.” The test rename documents the security boundary so the next person doesn’t undo this in good faith.

The latent bug that wasn’t on the tracker

While writing tests for fix #1, the existing gitlabRepoPush.examples.test.ts started failing. The “specific source and target path” example had been passing before my change but now hit my empty-actions short-circuit.

Investigation: the example declares sourcePath: 'src' but the test fixture sets up a folder called source/. With sourcePath: 'src', the action walks a non-existent directory, gets zero files, produces zero actions. Before my fix, the action then called GitLab with [] and got a mock response (mocks always succeed in tests). The test asserted that Commits.create had been called with a non-empty actions array — something that, after a beforeEach that didn’t jest.clearAllMocks(), matched a call from the previous test in the same file.

In other words: the test had been quietly green for who knows how long while asserting against the wrong mock invocation.

Fix: rename the fixture folder to src/, correct the filePath assertion to dest/abcd.txt (since targetPath: 'dest'), and move on. But it was a good reminder that mock-call persistence across tests is the kind of bug that hides until someone introduces an early-return path. If you don’t trust your beforeEach, sprinkle expect(mock).toHaveBeenCalledTimes(N) liberally — it would have caught this.

The CI surprise

I pushed fix #1, waited, and got a red Verify 22.x / Verify 24.x on the PR. Logs:

*************************************************************************************
* You have uncommitted changes to the public API or reports of a package.           *
* To solve this, run `yarn build:api-reports` and commit all md file changes.       *
*************************************************************************************
Error: API Extractor completed with 0 errors and 1 warnings

Changing commitHash: string → commitHash?: string | undefined is a public-API change. Backstage tracks every package’s exported types in report.api.md via API Extractor. The Verify job re-runs the extractor and fails if the report differs from the checked-in version.

Fix in principle: run yarn build:api-reports and commit the regen.

Fix in practice: the local regen produced lots of drift — union-type field orderings flipped, an “ae-undocumented” warnings block appeared at the bottom of the file — none of it caused by my change. The cause: my local TypeScript / api-extractor version doesn’t exactly match what CI runs. Committing the noisy regen would have made the diff impossible to review.

The right move turned out to be:

git checkout -- plugins/scaffolder-backend-module-gitlab/report.api.md
# then edit by hand:
#   commitHash: string;
# becomes
#   commitHash?: string | undefined;
# and nothing else

Re-pushed. Verify went green.

I saved this as a memory note for future contributions: when changing exported types in a Backstage package, the report.api.md change should be just your real diff, nothing more. Discard local-only drift.

Here’s the full feedback loop, since this is the kind of CI cycle you’ll hit any time a Backstage PR changes an exported type:

Lessons

1. Latent bugs hide behind mock-call persistence. Either always clearAllMocks() in beforeEach, or assert toHaveBeenCalledTimes(N) so a false-positive match doesn’t slip through.
2. report.api.md failures = stale public-API report. Cherry-pick the one line that reflects your real change; ignore environment-driven drift.
3. Backstage’s contribution loop is heavier than typical OSS — claim, branch, DCO sign-off, changeset, CI, API report. That weight is the thing that keeps the public API stable; you stop fighting it once you’ve been through it twice.

Both PRs are live:

• #34480 — fix(scaffolder-backend-module-gitlab)
• #34481 — fix(scaffolder-backend)

Next post in this series: building a new scaffolder module — scaffolder-backend-module-mcp — that lets Backstage templates invoke any Model Context Protocol server as a step. Stay tuned.