Repository navigation
Conversation
preview-deploy and preview-cleanup extract the workspace ID from the PR
comment with `capture("...(?P<id>...)")` inside `try`. jq 1.8 rejects the
`(?P<name>)` group syntax ("undefined group option"), and `try` turns that
into an empty result, so the comment looked like it held no ID: every push
would create another workspace and closing the PR would skip deleting it.
`(?<name>)` is accepted by both jq 1.7 and 1.8. The tests run the real
step scripts with a mocked gh and tailor, and are wired into CI as
`test:preview`.
…n cleanup A preview workspace whose close-time cleanup never ran (the job failed or was disabled, or the PR stayed open) was never deleted. preview-deploy now takes `ttl`, passed to `workspace create --ttl` so the workspace records an expiry, and preview-cleanup takes `prune-expired`, which runs `workspace prune --expired` after deleting the per-PR workspace. The sweep is scoped to the folder (or, without one, the organization root) the previews are created in, and to names matching `<prefix>-pr-<number>` with the prefix escaped, so other apps sharing the location are untouched. It passes `--limit 0`: prune aborts without deleting anything when more than its default 20 workspaces match, which would fail every later PR close once a backlog that size built up. It is the last step and runs under `!cancelled()`, so a failed deletion still gets swept and a failed sweep cannot undo the deletion or the comment update. With no location known it warns and skips rather than failing every PR close, because `prune --expired` rejects an empty location. Redeploys do not extend the expiry; it counts from creation. Both inputs default to off, so existing workflows behave as before.
The changeset said jq 1.8 rejects `(?P<id>)`. The jq 1.7.1 release binary rejects it too, so name both versions, and rename the file to match.
With `ttl` and `prune-expired`, a PR that stays open past `ttl` has its workspace deleted by the sweep. The PR comment still records the old ID, and preview-deploy reused it without checking, so every later push deployed to a deleted workspace. Closing such a PR also failed in preview-cleanup, because deleting the missing workspace is NotFound. preview-deploy now checks that the recorded workspace exists. If so it restarts the expiry with `workspace ttl set`, so the expiry counts from the last push; a failure to do that only warns. If `workspace get` says not found it creates a new workspace under the same name (names are not unique on the platform) and the comment is updated with the new ID. Any other failure of the check stops the step instead of risking a duplicate. preview-cleanup treats a not-found deletion as already deleted, warns, and still reports the workspace so the comment is updated. NotFound is matched on the CLI output because `workspace get` surfaces the raw Connect error without a stable code.
…, and read the folder env `workspace create --ttl` prints the new workspace as JSON and then exits nonzero with WORKSPACE_TTL_WRITE_FAILED when it cannot confirm the expiry. preview-deploy read the ID through a pipeline under pipefail, so that exit aborted the step before the ID was saved: the workspace was left untracked and the next push created another one. It now keeps the exit status and the output separately, takes the ID from the output, warns, and sets the expiry again with `workspace ttl set`. With no ID in the output it still fails. `workspace create` reads its folder from TAILOR_PLATFORM_FOLDER_ID when no folder is passed, so preview-deploy can place a preview in a folder while preview-cleanup saw no location and skipped the sweep. preview-cleanup now falls back to the same variable, as it already did for the organization.
toiroakr
marked this pull request as ready for review
October 5, 2026 13:14
remiposo
approved these changes
Oct 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A preview workspace whose close-time cleanup never ran (the job failed or was disabled, or the PR stayed open) was never deleted. This lets preview workspaces expire and lets every PR close sweep the expired ones.
preview-deploytakesttl(e.g.7d). The workspace records an expiry when created, and every later push restarts it, so it counts from the last push.preview-cleanuptakesprune-expired,organization-idandfolder-id. After deleting the per-PR workspace it runstailor workspace prune --expired.ttlhas its workspace swept while it is still open. The next push now creates a fresh workspace under the same name instead of deploying to the deleted one, and closing such a PR no longer fails.(?P<id>…)in a jqcapture, which the jq 1.7.1 and 1.8.2 release binaries reject.tryhid the error, so an existing workspace went unrecognized. It is now(?<id>…).Both new inputs are off by default, so existing workflows behave as before. Related to issue #1940 in platform-planning.
Usage
Behavior worth reviewing
folder-id, directly under the organization) whose whole name is{prefix}-pr-{number}, with the prefix escaped. Other apps sharing the location are untouched. The folder wins over the organization, as inpreview-deploy. A workspace without a recorded expiry is never deleted.--limit 0.workspace pruneaborts without deleting anything when more than its default 20 workspaces match. With that, a backlog of 21 would fail every later PR close. The name and location filters keep the sweep narrow.folder-id,organization-id,TAILOR_PLATFORM_FOLDER_IDorTAILOR_PLATFORM_ORGANIZATION_ID(the two variablesworkspace createitself reads), the sweep warns and is skipped instead of failing, becauseprune --expiredrejects an empty location and generated workflows pass an empty string for an unset variable. This is a provisional rule.!cancelled(), so a failed deletion is still swept and a failed sweep cannot undo the deletion or the comment update.preview-deployrunsworkspace geton the recorded ID. Not found means pruned, so it creates a new workspace; the PR comment is updated with the new ID. Any other failure stops the step so a transient error cannot create a duplicate. Ifworkspace create --ttlprints the new workspace and then exits nonzero because it could not confirm the expiry, the ID is still taken from its output and the expiry is set again, so the workspace is not left untracked. Workspace names are not unique on the platform, so reusing the name is fine.not_foundorworkspace … not found), becauseworkspace getsurfaces the raw Connect error without a stable code. A permission loss also reports not found.Verification
pnpm test:previewruns the real step scripts from the action files with a mockedghandtailor, and is added to CI. Existingtest:cli-bin-contractandtest:deploy-outputsstill pass;zizmorandghalintare clean.sedthat escapes the prefix was only run with BSD sed locally. The CI run on ubuntu covers GNU sed.