Skip to content

feat: let every environment toggle default-branch tracking from its comment - #242

Merged
vigneshrajsb merged 3 commits into
mainfrom
vb/expose-branch-tracking-in-comment
Aug 31, 2026
Merged

feat: let every environment toggle default-branch tracking from its comment#242
vigneshrajsb merged 3 commits into
mainfrom
vb/expose-branch-tracking-in-comment

Conversation

@vigneshrajsb

@vigneshrajsb vigneshrajsb commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

What

The Redeploy on pushes to default branches option now appears as a checkbox in the environment comment for every active environment.

Previously it was only rendered once already enabled, so it could be switched off from the comment but never on. In practice that meant it could only be set from the CLI.

Behaviour

  • The option applies only to services with no branch override. A service pinned to a branch already rebuilds on pushes to that branch, and is unaffected.
  • Setting it queues a redeploy, as it did before.
  • The option is hidden once an environment is torn down.
  • A comment that does not contain the option is left alone rather than treated as a request to disable it.

Also

trackDefaultBranches is added to the build API schema. The endpoint already returned it; only the schema was missing it, so clients could not type it.

Testing

Parser coverage for the checked, unchecked and absent cases, and for bodies that should not match. Ordering coverage for comment edits that request several things at once.

…omment

The "Redeploy on pushes to default branches" option was only rendered once it
was already enabled, so it could be turned off from the pull request comment
but never on. Render it for every active environment instead.

Because an absent option line previously meant "off", parsing now returns
undefined when the line is missing so a stale or hand-trimmed comment body no
longer clears the stored value.

Comment overrides are applied before a redeploy request is honoured, so a single
edit that changes options and ticks Redeploy keeps both, and a body that cannot
be parsed no longer swallows the redeploy.

Also hides the option on torn down environments and adds trackDefaultBranches to
the build API schema.
Reading the option used two separate whole-body scans, so a quoted copy of the
line could be read as checked while the real list item was unchecked. It is now
a single match with a capture group, and quoted lines no longer count because
GitHub does not render those as checkboxes.

Moving overrides ahead of the redeploy also let a cache purge request take
precedence over a redeploy request in the same edit, which dropped the redeploy.
Redeploy comes first again.

The option is now hidden only for torn down environments, matching the actions
block, instead of also hiding while tearing down.

The test covering the absent-option guard passed even with the guard removed,
because an undefined value is ignored when matching call arguments. It now
asserts the key is absent, and fails without the guard.
@vigneshrajsb
vigneshrajsb marked this pull request as ready for review August 29, 2026 04:40
@vigneshrajsb
vigneshrajsb requested a review from a team as a code owner August 29, 2026 04:40
@vigneshrajsb

Copy link
Copy Markdown
Contributor Author

Raising a finding from review that I have deliberately not changed, because it touches a design decision made in #241 rather than anything this PR introduced.

The stale-comment window

applyBuildConfigPatch refreshes the comment only when no redeploy was queued:

const queued = await this.enqueueRedeployIfEnabled(updatedBuild, pullRequest, runUuid);
if (!queued) {
  await refreshMissionControlComment(this, updatedBuild, pullRequest);
}

So when a redeploy is queued — the common case — the comment keeps showing the previous value until a build or deploy status transition rebuilds it. During that window, anyone editing the comment for an unrelated reason submits a body carrying the old value, and it gets written back.

This PR makes that window matter more than it did. Previously the option line rendered only when the setting was enabled, so a stale body preserved true and the effect was benign in the enable direction. Now the line always renders, so a stale body carries a concrete false and silently reverts an enable.

The absent-line guard added here does not help: it returns undefined only when the line is missing, which no longer happens on an active environment.

Why I left it alone

Skipping the refresh when a redeploy is queued is deliberate and covered by two tests (does not duplicate the comment refresh when the config change already queues a redeploy, and the service-overrides equivalent). Reversing it makes those tests fail by design, and reintroduces the duplicate refresh #241 set out to remove. That is a call for whoever owns that decision, not something to flip as a side effect of this change.

Options, roughly in order of size

  1. Refresh on the queued path as well, and update the two tests — closes the window, at the cost of the duplicate refresh.
  2. Compare-and-set on the write, in the manner of applyApiEnvironmentPatch, which already takes Build.transact + forUpdate() — narrower, and fixes the lost update rather than the staleness.
  3. Leave it, and accept that a comment edit during a deploy can revert a value set elsewhere.

Happy to implement whichever you prefer, in this PR or a follow-up.

@vigneshrajsb
vigneshrajsb merged commit 1eac182 into main Aug 31, 2026
5 checks passed
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.

1 participant