Skip to content

feat(release): tell tenjin when the MCP Registry pin goes stale - #181

Merged
A1igator merged 2 commits into
mainfrom
A1igator/release-pin-notify
Aug 21, 2026
Merged

feat(release): tell tenjin when the MCP Registry pin goes stale#181
A1igator merged 2 commits into
mainfrom
A1igator/release-pin-notify

Conversation

@A1igator

Copy link
Copy Markdown
Contributor

Scope

A successful changeset publish now files one issue on BackTrackCo/tenjin asking for the MCP Registry pin bump, so a release stops staling that listing in silence. Option 1 (notify) from #98; the auto-PR shape and auto-publishing stay out.

  • scripts/notify-registry-pin.sh reads the published version from changesets/action's publishedPackages, dedupes on an exact open-issue title match, and files an issue naming server.json, package.json, and lib/mcp/metadata.ts (where MCP_SERVER_INFO lives since tenjin#493), plus the rule that the registry publish stays manual and follows a promoted deploy.
  • .github/workflows/release.yml gates the new step on steps.publish.outputs.published == 'true', so a dispatch that shipped nothing announces nothing.
  • scripts/pack-smoke.sh asserts the packed mcpName is blog.tenjin/tenjin, the npm precondition Wire the MCP Registry packages pin into the release pipeline #98 asks for: dropping it breaks tenjin's next registry publish, not anything here.

Risk: the step runs after npm already has the package, so a failure there cannot un-publish. Until the owner action below is done it will fail the release run (loudly, on purpose: an unnoticed miss is the bug being fixed).

Owner action required

Create repo secret RELEASE_CROSSREPO_TOKEN: a fine-grained PAT scoped to BackTrackCo/tenjin with Issues: read and write.

#98 assumed the release-bot token already in that job would do. It will not: the x402r-release-bot installation holds contents: write, pull_requests: write, metadata: read and no Issues permission, and it is installed on this repo (and x402r-sdk), not on tenjin. Widening the App would need both a new permission grant and a new repo in its installation, so the token is parameterized on a named secret instead. RELEASING.md carries this as one-time owner setup step 4.

Testing

  • src/notify-registry-pin.test.ts: 7 cases driving the real script with gh stubbed. Files once per version; files nothing on a no-op publish or when the publish carried no tenjin-cli; skips a same-title open issue; still files when only the previous version's issue is open (the loose --search alone would swallow it); fails and writes nothing without a token; links the release run.
  • pnpm lint, pnpm typecheck, pnpm format:check: clean.
  • pnpm test: 2147 passed, 10 skipped.
  • bash scripts/pack-smoke.sh: PASS, including the new mcpName assertion.
  • Verified live (read-only) that gh's gojq resolves env.TITLE, which the dedupe query depends on.

Closes #98

A successful `changeset publish` now files one issue on BackTrackCo/tenjin
naming the newly published tenjin-cli version and the files that must move
with the pin. The registry rejects version ranges, so the pin is exact and
lives in the other repo, where nothing notices it going stale until the next
registry publish preflight weeks later.

Closes #98

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@A1igator A1igator added the priority: high High priority label Aug 18, 2026

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

@A1igator A1igator added priority: critical Blocks other PRs; merge-priority and removed priority: high High priority labels Aug 21, 2026

@vraspar vraspar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: solid notify-only shape; four minors, none blocking — approve after a look at the silent-skip paths

Reviewed against the diff and live cross-repo checks, verified at fe7da98. Ran locally: pnpm lint, pnpm typecheck, pnpm vitest run src/notify-registry-pin.test.ts (7/7), bash scripts/pack-smoke.sh (PASS incl. the new mcpName assertion).

What's solid:

  • Notify-only is the right scope for #98's option 1: no auto-PR, no auto-publish, and the issue body names all three files that move together with correct paths — every cross-repo reference verified live against BackTrackCo/tenjin main (server.json really pins packages[0] at an exact version; lib/mcp/metadata.ts really holds MCP_SERVER_INFO; docs/MCP-REGISTRY.md exists).
  • The loose-search + exact-jq-predicate dedupe correctly survives a still-open previous-version issue, and that regression has a dedicated test.
  • Secret handling is clean: step-scoped token, masked by Actions, never echoed, mktemp body file trapped away; workflow is dispatch-only on main, so fork PRs can't reach the PAT.
  • The test harness drives the real script and scrubs ambient GH_TOKEN/GITHUB_* before each scenario — it stays honest when run inside Actions.

Minor

  1. CI The trigger wiring can fail back into the exact silence this PR fixes: the whole feature hangs off two changesets/action output names (published, publishedPackages) with no validation anywhere. If an action upgrade renames or drops an output, '' == 'true' skips the step wordlessly and green — "a release nobody was told about" again. Cheap fix: fail loudly if the step ran but published is unset (distinguishes "action didn't report" from "reported false"), or add actionlint to CI so expression/output drift gets caught.

    if: steps.publish.outputs.published == 'true'
    run: bash scripts/notify-registry-pin.sh
    env:
    # Deliberately NOT the release-bot token minted above: that installation
    # holds Contents + Pull requests on this repo alone, so it cannot write
    # an issue on BackTrackCo/tenjin. Owner action: a fine-grained PAT with
    # Issues write on that repo, stored as RELEASE_CROSSREPO_TOKEN. Until it
    # exists this step fails the run, loudly, after npm already has the
    # package: a release nobody was told about is the bug being fixed.
    GH_TOKEN: ${{ secrets.RELEASE_CROSSREPO_TOKEN }}
    PUBLISHED_PACKAGES: ${{ steps.publish.outputs.publishedPackages }}

  2. Testing The gh stub couples to the jq's source text, not its semantics: the stub decides exact-match mode via value('--jq').includes('env.TITLE'). Mutation probe: typo the jq to .numer // empty (substring intact) and the suite stays 7/7 green while real gh returns empty for every lookup — dedupe silently disabled forever. Fix: have the stub apply the predicate minimally, or key the mode off something observable rather than the literal substring.

    const exact = value('--jq').includes('env.TITLE');
    const visible = exact ? issues.filter((i) => i.title === process.env.TITLE) : issues;
    process.stdout.write(visible.length > 0 ? String(visible[0].number) + '\\n' : '');

  3. Hygiene A failed notification is unrecoverable by any rerun, and RELEASING.md doesn't say how to recover: once publish ships X and the notify step fails, "re-run failed jobs" republishes nothing → gate false → no retry path; the missed issue must be filed by hand, but no doc says so. One sentence in RELEASING.md (the env-overridable script supports it directly): run PUBLISHED_PACKAGES='[{"name":"tenjin-cli","version":"X"}]' GH_TOKEN=… bash scripts/notify-registry-pin.sh.

    A publish that actually shipped then files one issue on `BackTrackCo/tenjin`
    (`scripts/notify-registry-pin.sh`): its MCP Registry manifest pins `tenjin-cli`
    by exact version, and nothing else notices that the pin went stale. Deciding
    whether the release is worth advertising, and publishing the manifest, stay
    manual on that side.

  4. Security Dedupe rides best-effort search recall: gh issue list --search is loose full-text over a default 30-result page, and GitHub search indexing lags fresh issues by seconds–minutes — any of those makes existing empty and files a duplicate. Impact is noise, not compromise (the workflow concurrency group already serializes real races). Add --limit 200 and consider searching just "tenjin-cli@$version in:title" so prose words don't dilute recall.

    existing=$(gh issue list --repo "$TARGET_REPO" --state open --search "$TITLE in:title" \
    --json number,title --jq 'map(select(.title == env.TITLE)) | .[0].number // empty')

Nits (5), none blocking
  1. Hygiene The bash-3.2 comment states the parse bug more broadly than it exists (scripts/notify-registry-pin.sh:65-67): empirically only a line ending in ) breaks $() + heredoc parsing, not any bare ) mid-line — and the current body has no such line. Reword or drop the version archaeology; keep the temp-file approach.
  2. Hygiene Token rationale triplicated across release.yml:177-182, notify-registry-pin.sh:35-43, RELEASING.md:69-73; keep the full story in RELEASING.md, one-line pointers elsewhere.
  3. Testing Test comment narrates tenjin's internal file history ("moved out of lib/mcp/server.ts", src/notify-registry-pin.test.ts:144-146) — rots on the next tenjin refactor; trim to where the symbol lives now.
  4. Security TARGET_REPO/PACKAGE env overrides are unconstrained in CI (notify-registry-pin.sh:19-20); harmless today (PAT-scoped), but they're pure surface in production — pin them in the workflow env.
  5. Testing Untested boundaries: empty-string GH_TOKEN (what Actions actually produces for an unset secret — same code path as missing, but free to cover), mixed published array (tenjin-cli alongside other packages), GITHUB_SERVER_URL override shaping the run link.
Verified, not issues
  • Injection cleared end-to-end: version comes from npm-enforced semver, reaches gh only as argv / env.TITLE (never re-parsed as a filter); every backtick in the heredoc body escaped, no $(...); GITHUB_* link vars are runner-set constants on a dispatch-only workflow.
  • PAT blast radius contained: fine-grained Issues-only on one repo fails auth elsewhere; token absent from logs and from the test stub log.
  • pack-smoke's mcpName read hits the true packed artifact (tarball installed by path into an empty consumer dir) and runs on every PR via ci.yml, not just releases.
  • Stub's multi-match behavior matches jq's map(...)[0] ordering; ignoring --search in the stub is a conservative superset since the exact-title predicate decides correctness.
  • Malformed PUBLISHED_PACKAGES dies loudly under set -e (raw node trace, cosmetic only); empty-string token caught by [ -z "${GH_TOKEN:-}" ].
  • Red-run-after-successful-publish is documented intent, and workflow_dispatch-only means no required-check/auto-merge machinery misreads it.
Closed as decisions
  • Loud failure marking an otherwise-successful release red until the owner adds RELEASE_CROSSREPO_TOKEN: stated intent in the PR body, and arguably the point. Owner setup step is documented in RELEASING.md.
  • Notify-only, no auto-PR/auto-publish: explicit non-goal of #98 option 1.

Verdict: comments-only — no majors; minors 1–2 are cheap hardening of exactly the silence this PR exists to end, worth taking before merge; 3–4 and the nits are follow-up-grade.

The step gated on `published == 'true'`, so a changesets/action upgrade that
renamed or dropped either output would skip it wordlessly and green: a release
nobody was told about, the bug this feature exists to fix. The gate now runs the
step for anything other than a literal false, and the script fails on an empty
`published` or a claimed publish that names no package.

Dedupe asks for a 200-issue page and searches the version phrase alone, so prose
words cannot dilute recall, and RELEASING.md carries the manual recovery command
for a notification that failed after npm already had the package.

The gh stub now evaluates the script's --jq (predicate field, env var, emitted
field) instead of matching its source text, so a typo like `.numer // empty`
reds instead of passing while real gh returns empty for every lookup.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

@A1igator

Copy link
Copy Markdown
Contributor Author

All four minors and all five nits are in c987d75. Two of them did not land the way the review proposed, with reasons.

Finding 1, the runtime assertion rather than actionlint. actionlint cannot catch this class: renaming steps.publish.outputs.published to a nonexistent output in a scratch copy of release.yml still exits 0, because third-party action outputs are unknown to it. So the gate is now published != 'false', and the script fails loudly on an empty value ("the action did not report") while exiting 0 quietly on any other non-true value ("it reported false"). A second check catches published=true with an empty publishedPackages, which used to exit 0 as "nothing to file" and now fails as "the two outputs disagree".

Inverting that gate is only safe because of what the pinned action actually does. 63a615b (v1.8.0) sets published=false and publishedPackages=[] as defaults before any branch (src/index.ts:63-64), so a genuine nothing-to-publish dispatch still emits a literal false and is skipped. Without that, the inverted gate would have turned every no-op dispatch red.

actionlint was run against the workflows and is clean, but not added to CI: it does not catch this bug, and a workflow-lint job is separate scope.

Finding 2, the stub now applies the predicate. It parses the --jq program into predicate field, env var and emitted field, applies it against the stub issues, and exits 1 on any program shape it does not support rather than falling back to a default mode. Your mutation probe, run against this version:

  • .[0].number to .[0].numer, substring intact: 1 failed, 12 passed. The dedupe case reds, because the stub now returns empty exactly as real gh would.
  • Predicate dropped entirely: 6 failed, 7 passed. The shape is unsupported, the stub exits 1, and the script dies under set -e.

Both probes reverted.

Findings 3 and 4 landed as written: RELEASING.md states that a re-run cannot fix a failed notification and gives the by-hand command including PUBLISHED=true, and the lookup takes --limit 200 narrowed to "$PACKAGE@$version" in:title, with the comment saying plainly that dedupe is best-effort and a miss is noise.

Your five untested boundaries are covered, including the mixed published array with tenjin-cli-plugin@9.9.9 ordered ahead so a prefix match would red. This file goes 7 to 13 tests.

Gates: format, lint, typecheck, build clean; 2153 tests pass; pack-smoke PASS.

@A1igator
A1igator requested a review from vraspar August 21, 2026 19:42

@vraspar vraspar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review: all four minors and five nits resolved at c987d75 — approving

Delta reviewed since fe7da98, both mutation probes independently re-run, gates verified locally (pnpm lint, pnpm typecheck, 13/13 tests, bash scripts/pack-smoke.sh PASS).

Resolved since round 1

  1. Silent-skip trigger wiring → resolved with the runtime assertion rather than actionlint. Verified the load-bearing claim against installed source: changesets/action at the pinned SHA 63a615b sets published=false / publishedPackages=[] unconditionally before any branch (src/index.ts:63-64), so the inverted gate != 'false' cannot turn a genuine no-op dispatch red, while an output rename now lands as an empty PUBLISHED and fails loudly (release.yml:174). The disagreement check (published=true naming zero packages) closes the second silent path (notify-registry-pin.sh:44-49).
  2. Circular stub coupling → resolved. The stub now parses and evaluates the jq program's shape and exits 1 on anything unsupported. Both probes re-run independently: .number.numer typo ⇒ 1 failed (dedupe case reds, exactly as real gh would behave); predicate removed entirely ⇒ 7 failed loudly instead of passing green.
  3. Unrecoverable failed notification → resolved: RELEASING.md documents that a re-run cannot fix it and gives the by-hand command, including the now-required PUBLISHED=true.
  4. Best-effort dedupe recall → resolved: --limit 200, query narrowed to "tenjin-cli@<version>" in:title, miss documented as noise-not-harm, lookup shape asserted by test.

Nits also landed: bash-3.2 archaeology replaced with a neutral rationale, token rationale deduplicated to pointers into RELEASING.md, file-history narration trimmed from the test comment, TARGET_REPO/PACKAGE pinned in workflow env, and the three boundary gaps covered (empty-string token as Actions actually delivers an unset secret, mixed published array ordered to catch prefix matches, non-github.com server URL).

Still open: none.

New: nothing beyond the above; the delta introduces no new issues.

Closed as decisions

  • Dedupe remains best-effort (recall miss files a duplicate issue): stated plainly in code and accepted as noise, not harm.
  • Loud red run after a successful publish until RELEASE_CROSSREPO_TOKEN exists: round-1 decision, unchanged.
  • Notify-only scope, no auto-PR/auto-publish: unchanged non-goal.

Verdict: approve. Clean at c987d75; the only owner action left is creating the RELEASE_CROSSREPO_TOKEN secret before the next release.

@A1igator
A1igator merged commit 0045a07 into main Aug 21, 2026
1 check passed
@A1igator
A1igator deleted the A1igator/release-pin-notify branch August 21, 2026 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: critical Blocks other PRs; merge-priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wire the MCP Registry packages pin into the release pipeline

2 participants