Skip to content

Commit b024403

Browse files
Wilson421claude
andcommitted
Fix manual-run tag propagation, and make the human gate real
FOUR FINDINGS FROM REVIEW OF d0fa235, ALL CONFIRMED AGAINST THE FILE. 1. BLOCKING. `github-release` used `github.ref_name` for the release tag and title. On a workflow_dispatch run that is the BRANCH the workflow was launched from, not the tag typed into the form - so a manual release of v0.6.0 packaged 0.6.0 correctly and then created the release against `main`. The build job already resolves and validates the tag; everything downstream now uses that answer instead of re-deriving a different one from the context. The manual input is also validated against ^v[0-9]+\.[0-9]+\.[0-9]+(-.+)?$ before anything acts on it, because a typo used to travel silently into a checkout and a filename. 2. Packaging discarded the determinism it had just established. `npm ci` installs the lockfile's vsce - 3.6.0 - and the next line ran `npm install -g @vscode/vsce`, fetching whatever `latest` was that morning, in the step that builds the artifact users install. Meanwhile the publish step was being scrupulously pinned. Now `npm exec -- vsce package`, so the lockfile governs the artifact and the exact pin covers only authentication and upload. 3. Open VSX was passed `--pre-release` for no effect. ovsx warns "Ignoring option '--pre-release' for prepackaged extension" and continues: the channel is in the VSIX manifest from package time. The flag implied the upload decided something it does not. Removed, documented, and ovsx pinned to 1.1.1. 4. The `environment: marketplace` comment still explained Entra federated credential subjects, from the discarded design. Removed before it sent somebody back into the Klein bottle in six months. THE HUMAN GATE IS NOW THE ENVIRONMENT, NOT THE DRAFT. The guide said nothing reaches users until you edit the draft and press publish. That stops being true the moment MARKETPLACE_PUBLISH is enabled - publishing the GitHub draft and publishing to the Marketplace are independent acts, and only the first was ever gated by a human. So `marketplace` becomes a protected environment requiring a reviewer. A protected environment does not pause a running job, it stops the job STARTING, which means the OIDC token is never minted: the capability to publish does not exist until a person approves it. That is a better property than a job that can always publish and is trusted not to. The variable and the approval do different jobs and both stay. MARKETPLACE_PUBLISH decides whether the job is reachable at all - unset, it is skipped, so testing with real tags raises no approval prompts to dismiss. The approval decides whether a reachable job runs. Documented too: do not enable "prevent self-review" on that environment. With one maintainer it deadlocks the pipeline permanently. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent d0fa235 commit b024403

3 files changed

Lines changed: 84 additions & 17 deletions

File tree

.github/workflows/release.yml

Lines changed: 42 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ jobs:
6464
base: ${{ steps.meta.outputs.base }}
6565
is_prerelease: ${{ steps.meta.outputs.is_prerelease }}
6666
channel: ${{ steps.meta.outputs.channel }}
67+
tag: ${{ steps.meta.outputs.tag }}
6768
steps:
6869
# On a manual run GITHUB_REF is a BRANCH, so the tag has to be named and checked out
6970
# explicitly. Without this the workflow advertised manual publishing and could not do it: the
@@ -110,6 +111,14 @@ jobs:
110111
# A manual run has no tag ref, so take the one that was asked for.
111112
TAG="${{ inputs.tag }}"
112113
if [[ -z "$TAG" ]]; then TAG="${GITHUB_REF#refs/tags/}"; fi
114+
115+
# Validate before anything acts on it. A typo in the manual form used to travel silently
116+
# into a checkout, a package name, and a release title.
117+
if [[ ! "$TAG" =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-.+)?$ ]]; then
118+
echo "::error::'$TAG' is not a release tag. Expected vX.Y.Z or vX.Y.Z-suffix."
119+
exit 1
120+
fi
121+
113122
VERSION="${TAG#v}"
114123
PKG=$(node -p "require('./package.json').version")
115124
@@ -183,8 +192,12 @@ jobs:
183192
FLAG=""
184193
if [[ "${{ steps.meta.outputs.channel }}" == "prerelease" ]]; then FLAG="--pre-release"; fi
185194
186-
npm install -g @vscode/vsce
187-
vsce package $FLAG --out "excel-power-query-editor-${{ steps.meta.outputs.base }}.vsix"
195+
# THE LOCKFILE'S vsce, not whatever `latest` is this morning.
196+
#
197+
# `npm install -g @vscode/vsce` threw away the determinism `npm ci` had just established -
198+
# and it did so in the step that builds the artifact users install, while the publish step
199+
# was being scrupulously pinned. `npm exec` uses the copy npm ci placed in node_modules.
200+
npm exec -- vsce package $FLAG --out "excel-power-query-editor-${{ steps.meta.outputs.base }}.vsix"
188201
189202
- name: Check the package contains what it must
190203
run: |
@@ -220,11 +233,17 @@ jobs:
220233
with:
221234
name: vsix
222235

236+
# USE THE TAG THE BUILD RESOLVED, NOT `github.ref_name`.
237+
#
238+
# On a manual run `github.ref_name` is the BRANCH the workflow was launched from, so a
239+
# dispatch for v0.6.0 packaged 0.6.0 correctly and then created the release against `main`.
240+
# The build job already does the work of resolving and validating the tag; everything
241+
# downstream must use its answer rather than re-deriving a different one from the context.
223242
- name: Create the draft release
224243
uses: softprops/action-gh-release@v2
225244
with:
226-
tag_name: ${{ github.ref_name }}
227-
name: Excel Power Query Editor ${{ github.ref_name }}
245+
tag_name: ${{ needs.build.outputs.tag }}
246+
name: Excel Power Query Editor ${{ needs.build.outputs.tag }}
228247
# DRAFT deliberately: nothing becomes visible until a human reads it and clicks publish.
229248
draft: true
230249
prerelease: ${{ needs.build.outputs.is_prerelease }}
@@ -254,11 +273,20 @@ jobs:
254273
needs.build.outputs.channel != 'none' &&
255274
(vars.MARKETPLACE_PUBLISH == 'enabled' || inputs.publish_marketplace == true)
256275
257-
# The identity lives on the environment, not on the branch or tag.
276+
# THE HUMAN GATE. Make this a protected environment requiring a reviewer.
258277
#
259-
# A federated credential is matched by SUBJECT, and a subject built from a ref would need one
260-
# credential per tag - or a wildcard, which is the opposite of the point. `environment:marketplace`
261-
# is one stable subject that covers tag pushes and manual runs alike.
278+
# A protected environment does not merely pause the job - the job does not START, so the OIDC
279+
# token is never minted. The capability to publish does not exist until a person approves it,
280+
# which is a stronger property than checking a box in a form and hoping.
281+
#
282+
# It is also what keeps "nothing reaches users without a human" true once MARKETPLACE_PUBLISH is
283+
# enabled. Publishing the draft GitHub release and publishing to the Marketplace are independent
284+
# acts, and only the first one was ever gated by a human editing a draft.
285+
#
286+
# The Marketplace trust policy may also name this environment.
287+
#
288+
# (Nothing to do with Entra any more. An earlier design used this name as a federated-credential
289+
# subject; trusted publishing has no subject, and that reasoning is gone.)
262290
environment: marketplace
263291

264292
permissions:
@@ -315,15 +343,17 @@ jobs:
315343
- name: Publish
316344
env:
317345
OVSX_PAT: ${{ secrets.OVSX_PAT }}
346+
OVSX_VERSION: 1.1.1
318347
run: |
319348
if [[ -z "$OVSX_PAT" ]]; then
320349
echo "::warning::OVSX_PAT is not configured - skipping Open VSX."
321350
exit 0
322351
fi
323-
FLAG=""
324-
if [[ "${{ needs.build.outputs.channel }}" == "prerelease" ]]; then FLAG="--pre-release"; fi
325352
326-
npx --yes ovsx publish --packagePath *.vsix $FLAG -p "$OVSX_PAT"
353+
# NO --pre-release HERE, DELIBERATELY. ovsx warns "Ignoring option '--pre-release' for
354+
# prepackaged extension" and moves on: the channel is already baked into the VSIX manifest
355+
# at package time. Passing it would look like it decided something.
356+
npx --yes "ovsx@${OVSX_VERSION}" publish --packagePath *.vsix -p "$OVSX_PAT"
327357
328358
summary:
329359
name: Summary
@@ -335,7 +365,7 @@ jobs:
335365
run: |
336366
CHANNEL="${{ needs.build.outputs.channel }}"
337367
{
338-
echo "### ${{ github.ref_name }}"
368+
echo "### ${{ needs.build.outputs.tag }}"
339369
echo ""
340370
echo "| Step | Result |"
341371
echo "| --- | --- |"

docs/PUBLISHING_GUIDE.md

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,11 @@ git tag v0.6.0
99
git push origin v0.6.0
1010
```
1111

12-
That builds, packages, and creates a **draft** GitHub release with the `.vsix` attached. Nothing is
13-
published to the Marketplace and nothing is visible to users until you edit the draft and press
14-
publish.
12+
That builds, tests, packages, and creates a **draft** GitHub release with the `.vsix` attached.
13+
14+
**Publishing the draft and publishing to the Marketplace are two independent acts.** Editing the
15+
draft and pressing publish makes the GitHub release visible; it does nothing to the Marketplace. The
16+
Marketplace gate is the `marketplace` environment approval described below.
1517

1618
## Marketplace publishing is OFF
1719

@@ -88,10 +90,40 @@ repository's workflow, and it replaces every step an Entra route would have need
8890

8991
| Kind | Name | Value |
9092
| --- | --- | --- |
91-
| Environment | `marketplace` | must exist — the publish job declares it |
93+
| Environment | `marketplace` | must exist, **with a required reviewer** — see below |
9294
| Variable | `MARKETPLACE_PUBLISH` | `enabled`, when you want tags to publish |
9395
| Secret | `OVSX_PAT` | Open VSX token — optional, see below |
9496

97+
### The human gate is the environment, not the draft
98+
99+
**Make `marketplace` a protected environment requiring a reviewer.** Settings → Environments →
100+
`marketplace` → Required reviewers → add yourself.
101+
102+
This is stronger than it looks. A protected environment does not pause a running job — **the job
103+
never starts**, so the OIDC token is never minted. The *capability* to publish does not exist until
104+
a person approves it, rather than existing continuously and being politely unused.
105+
106+
```text
107+
push v0.6.0
108+
├─ build, test, package
109+
├─ draft GitHub release (visible to nobody until you publish it)
110+
111+
marketplace environment
112+
├─ REQUIRED HUMAN APPROVAL ◄── the gate
113+
114+
OIDC token minted → Marketplace publish → Open VSX
115+
```
116+
117+
**Do not enable "prevent self-review."** With a single maintainer it deadlocks the pipeline
118+
permanently: the only person who can approve is the person who triggered it.
119+
120+
**Two gates, doing different jobs.** `MARKETPLACE_PUBLISH` decides whether the job is *reachable*
121+
unset, it is skipped entirely, so testing the pipeline with real tags produces no approval prompts
122+
to dismiss. The environment approval decides whether a reachable job *runs*. Keep both.
123+
124+
An approval request expires after 30 days and the run fails, which is the correct outcome for a
125+
release nobody remembered to approve.
126+
95127
**No `AZURE_CLIENT_ID`, no `AZURE_TENANT_ID`, no secrets for the Marketplace at all.** The
96128
`id-token: write` permission on the job is what lets `vsce` request the token; without it the error
97129
is explicit about the missing permission.
@@ -141,6 +173,11 @@ access token stored as the **`OVSX_PAT`** secret.
141173
a token must not turn a successful Marketplace publish into a red release. Without `OVSX_PAT` it
142174
warns and skips.
143175

176+
**Open VSX is not passed `--pre-release`, on purpose.** Given an already-packaged VSIX it warns
177+
*"Ignoring option '--pre-release' for prepackaged extension"* and carries on — the channel is baked
178+
into the manifest at package time. Passing the flag would suggest the upload decides something it
179+
does not. `ovsx` is pinned to `1.1.1` for the same reason `vsce` is pinned.
180+
144181
## Why the pipeline was rebuilt
145182

146183
Worth knowing, because the failure was invisible for a year.

docs/project/EPQE_Development_Roadmap.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ The two write paths currently disagree about deletion, and nobody chose that. Se
129129
| PQ-29 | ✅ done | USER_GUIDE.md has no mention of live sync | S | [Live_Sync](../Live_Sync.md) | feature doc written; User_Guide now carries a section pointing into it |
130130
| PQ-30 | ✅ done | Retire the RELEASE_SUMMARY pattern | S || removed; CHANGELOG and the release body are the two homes |
131131
| PQ-32 | ✅ done | docs: link and orphan checking in CI | S | [Overview](../Overview.md) | `npm run docs:links` found 11 dead links and 1 unreachable doc on its first run |
132-
| PQ-34 | 🟡 wired | Marketplace pre-release channel, and PAT-free auth before PATs die | M | [PQ-34][pq-34] | pipeline done 2026-08-17: **trusted publishing** (`--oidc`, pinned to 3.9.3-5), odd/even channel derivation with an intent assertion, Open VSX non-blocking. Blocked on one Marketplace trust policy; no Entra tenant needed after all. `--oidc` is absent from 3.9.2 but shipped on `next` 2026-08-11 |
132+
| PQ-34 | 🟡 wired | Marketplace pre-release channel, and PAT-free auth before PATs die | M | [PQ-34][pq-34] | pipeline done 2026-08-17: **trusted publishing** (`--oidc`, pinned to 3.9.3-5), odd/even channel derivation with an intent assertion, Open VSX non-blocking. Blocked on one Marketplace trust policy; no Entra tenant needed after all. `--oidc` is absent from 3.9.2 but shipped on `next` 2026-08-11 Human gate is a **protected `marketplace` environment**, so the OIDC token cannot be minted without an approval. |
133133
| PQ-31 | 🟡 partial | bump-version: drop commit analysis, sync the README badge | S | [PUBLISHING_GUIDE](../PUBLISHING_GUIDE.md) | badge sync DONE by docs-tools `values`; commit analysis still there, and the `npm version` tag hazard is now documented rather than fixed |
134134

135135
### Data safety — the thing that must never break

0 commit comments

Comments
 (0)