[PR #5022] fix(publish-mcp): retry and classify registry read failures #6740

Open
opened 2026-11-01 21:42:43 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5022
Author: @Rintu62
Created: 10/18/2026
Status: 🔄 Open

Base: main ← Head: fix/registry-read-hardening


📝 Commits (5)

  • f27df51 fix(publish-mcp): retry and classify registry read failures
  • fb8419a Merge remote-tracking branch 'upstream/main' into fix/registry-read-hardening
  • b806d3e fix(publish-mcp): cancel failed response bodies and keep summary best-effort
  • 917569f test(publish-mcp): use the shared temp directory helper
  • 9c7b2ea Merge remote-tracking branch 'upstream/main' into fix/registry-read-hardening

📊 Changes

3 files changed (+227 additions, -10 deletions)

View changed files

📝 .github/actions/publish-mcp/registry-state.d.mts (+14 -0)
📝 .github/actions/publish-mcp/registry-state.mjs (+76 -9)
📝 tests/boundary/filesystem/mcpRegistryPublication.test.ts (+137 -1)

📄 Description

Summary

Fixes #4998.

Before publishing, the workflow checks whether this version already exists in the MCP Registry. That check (registry-state.mjs) gave up on the first timeout, so one slow response blocked the whole publish. It now retries twice for temporary failures. If it still fails, the job fails exactly like before, and the failure type is written to the job summary.

Problem and expected behavior

In #4998 the registry's /v0.1/ping returned 200, but the listing request timed out after 30 s and the job stopped before authentication. A short blip shouldn't be enough to stop a release, and the job output should say whether it was a timeout, a 5xx or a 404.

The safety rule stays the same: if the lookup can't finish, the result is unknown and the job fails. A failed read is never treated as "not published".

Change and scope

  • Added a small readPage helper around the existing request and JSON parse. It retries twice (waits 5 s, then 15 s) on timeouts, network errors and 5xx. It does not retry 404 or other 4xx.
  • Before a retry, the failed response body is cancelled so the connection is not left busy.
  • If the last retry fails, the original error is thrown again, so registryPublicationState can't return "absent" unless every page was read.
  • Added classifyReadFailure. When the script runs as the action step, a failure writes the kind (timeout, network_error, server_error, not_found, http_error, other) to GITHUB_STEP_SUMMARY and then rethrows. Writing the summary is best effort: if that write fails, the original registry error is still the one that is thrown.
  • registryPublicationState takes an optional third argument (retryDelaysMs, sleep) so tests don't wait for real delays. Types are updated in registry-state.d.mts.

I didn't change action.yml, authentication, the publish step, the 30 s timeout or the metadata comparison. This doesn't fix the registry problem itself, it only makes the job handle it better.

This is a routine fix inside existing boundaries, so a maintainer scope decision doesn't apply. See the contribution scope.

Contract and boundary impact

  • Semantic owner and earliest changed stage: .github/actions/publish-mcp/registry-state.mjs, the version lookup before authentication.
  • Contribution scope fit and linked maintainer decision: routine fix, not applicable.
  • CLI and MCP/tool-catalog contract: none, this is a release workflow helper.
  • Provider, bridge, target-format, or platform compatibility: none.
  • Evidence, artifact, provenance, or reconstruction contract: not applicable.
  • Target selection, authority, upstream behavior, and cleanup details: not applicable, no target is touched. It only reads the public registry listing.
  • Generated metadata, package, or installation impact: none, no generated files changed.

Expected effects:

  • Read existing files or captured data.
  • Launch or attach to a process.
  • Inject instrumentation or run scripts/code.
  • Change process memory, registers, files, or configuration.
  • Contact network services.
  • Recover protected code or work around protections.
  • Documentation or usability only; no target interaction.
  • Not sure yet; investigation or maintainer guidance needed.

The script reads server.json as before. The network call is the existing registry GET, now retried. On failure it appends a short note to the GITHUB_STEP_SUMMARY file.

Evidence and regression coverage

  • Tests added or updated: 6 new tests in tests/boundary/filesystem/mcpRegistryPublication.test.ts (retry then success with delays 5000 and 15000, all attempts time out and the error is rethrown after 3 calls, no retry on 404, failure classification, failed response body is cancelled before the retry, original error is kept when the job summary can't be written). The last one runs the script in a temp directory from createTestTempDirectory. The existing 503 test now passes { sleep: noSleep } so it doesn't wait for real delays.
  • Base reproduction or other evidence: I didn't run the new tests against the base revision. Reading the base code, it makes one request per page and throws on the first failure, so the retry tests would not pass there.
  • User-visible CLI/MCP output: none. The only visible change is the failure note in the CI job summary, which I haven't seen in a real run.
  • Remaining proof gaps: no real registry run and no real timeout reproduction (the tests use a mocked request). I ran the tests locally on Windows only. I didn't run npm run check or npm run verify:test-temp-hygiene, and 23 tests in releaseWorkflow.test.ts are skipped on my machine. I haven't checked whether mcpRegistryPublication.test.ts ran in the CI shards.

Validation performed

  • npx vitest run tests/boundary/filesystem/mcpRegistryPublication.test.ts — 9 passed, exit 0
  • npx vitest run tests/boundary/filesystem/releaseRecovery.test.ts tests/boundary/filesystem/releaseWorkflow.test.ts — 17 passed, 23 skipped, exit 0 (same result without my changes)
  • npx turbo run typecheck lint --force — 2 successful, exit 0
  • npm run test:focused -- <test file> — didn't run: exits 1 on Windows because native/windows/build/manifest.json is missing, before any test starts. I used npx vitest run directly instead.
  • GitHub Actions CI run #12934 on this PR: Static checks, Source tests, Build package, the native package baselines and the Windows capability lane passed. Portable tests failed: Test shard (3/5) has 2 failing tests in tests/boundary/mcp/nativeUiFinalizationMcp.test.ts ("retains native UI reports over MCP when its deadline expires during close", both cleanup variants), where cancelled / cleanup_incomplete was expected and provider_timeout was received. This PR only changes the three files listed above, and nothing in the diff touches native UI finalization code. I did not run that test locally, so I can't say for certain that the failure is unrelated. I can't re-run workflows from my fork.

Compatibility, safety, and release

  • Breaking changes or migration steps: none. The existing error message (Registry lookup failed: HTTP <status>) is unchanged and the new parameter is optional.
  • Real Hopper/Ghidra, browser, or OS coverage: not applicable, I only ran checks on Windows.
  • Package or release metadata impact: none expected.
  • Security, privacy, process, or containment review: only the GET read is retried. Authentication and publish are untouched, and a persistent failure still fails the job before them. Worst case per page is about 110 s (3 x 30 s + 5 s + 15 s).

Author readiness

Human owner: @Rintu62

  • I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
  • I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
  • I declare this PR ready for initial maintainer review.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/5022 **Author:** [@Rintu62](https://github.com/Rintu62) **Created:** 10/18/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/registry-read-hardening` --- ### 📝 Commits (5) - [`f27df51`](https://github.com/morluto/rea/commit/f27df51f323a07fb00d88895a9e2b5438a48d488) fix(publish-mcp): retry and classify registry read failures - [`fb8419a`](https://github.com/morluto/rea/commit/fb8419a4da7b4e2cc09909dcd4118a78838b4000) Merge remote-tracking branch 'upstream/main' into fix/registry-read-hardening - [`b806d3e`](https://github.com/morluto/rea/commit/b806d3e689a062eb2f81c6eb94646b5eadb99fe0) fix(publish-mcp): cancel failed response bodies and keep summary best-effort - [`917569f`](https://github.com/morluto/rea/commit/917569fd04b2cd68af9a66b21de6193f14d8aa49) test(publish-mcp): use the shared temp directory helper - [`9c7b2ea`](https://github.com/morluto/rea/commit/9c7b2ea971b955da4419f0c17e4e38c3af251367) Merge remote-tracking branch 'upstream/main' into fix/registry-read-hardening ### 📊 Changes **3 files changed** (+227 additions, -10 deletions) <details> <summary>View changed files</summary> 📝 `.github/actions/publish-mcp/registry-state.d.mts` (+14 -0) 📝 `.github/actions/publish-mcp/registry-state.mjs` (+76 -9) 📝 `tests/boundary/filesystem/mcpRegistryPublication.test.ts` (+137 -1) </details> ### 📄 Description ## Summary Fixes #4998. Before publishing, the workflow checks whether this version already exists in the MCP Registry. That check (`registry-state.mjs`) gave up on the first timeout, so one slow response blocked the whole publish. It now retries twice for temporary failures. If it still fails, the job fails exactly like before, and the failure type is written to the job summary. ## Problem and expected behavior In #4998 the registry's `/v0.1/ping` returned 200, but the listing request timed out after 30 s and the job stopped before authentication. A short blip shouldn't be enough to stop a release, and the job output should say whether it was a timeout, a 5xx or a 404. The safety rule stays the same: if the lookup can't finish, the result is unknown and the job fails. A failed read is never treated as "not published". ## Change and scope - Added a small `readPage` helper around the existing request and JSON parse. It retries twice (waits 5 s, then 15 s) on timeouts, network errors and 5xx. It does not retry 404 or other 4xx. - Before a retry, the failed response body is cancelled so the connection is not left busy. - If the last retry fails, the original error is thrown again, so `registryPublicationState` can't return `"absent"` unless every page was read. - Added `classifyReadFailure`. When the script runs as the action step, a failure writes the kind (`timeout`, `network_error`, `server_error`, `not_found`, `http_error`, `other`) to `GITHUB_STEP_SUMMARY` and then rethrows. Writing the summary is best effort: if that write fails, the original registry error is still the one that is thrown. - `registryPublicationState` takes an optional third argument (`retryDelaysMs`, `sleep`) so tests don't wait for real delays. Types are updated in `registry-state.d.mts`. I didn't change `action.yml`, authentication, the publish step, the 30 s timeout or the metadata comparison. This doesn't fix the registry problem itself, it only makes the job handle it better. This is a routine fix inside existing boundaries, so a maintainer scope decision doesn't apply. See the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). ## Contract and boundary impact - Semantic owner and earliest changed stage: `.github/actions/publish-mcp/registry-state.mjs`, the version lookup before authentication. - Contribution scope fit and linked maintainer decision: routine fix, not applicable. - CLI and MCP/tool-catalog contract: none, this is a release workflow helper. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: not applicable. - Target selection, authority, upstream behavior, and cleanup details: not applicable, no target is touched. It only reads the public registry listing. - Generated metadata, package, or installation impact: none, no generated files changed. Expected effects: - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [x] Change process memory, registers, files, or configuration. - [x] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. The script reads `server.json` as before. The network call is the existing registry GET, now retried. On failure it appends a short note to the `GITHUB_STEP_SUMMARY` file. ## Evidence and regression coverage - Tests added or updated: 6 new tests in `tests/boundary/filesystem/mcpRegistryPublication.test.ts` (retry then success with delays 5000 and 15000, all attempts time out and the error is rethrown after 3 calls, no retry on 404, failure classification, failed response body is cancelled before the retry, original error is kept when the job summary can't be written). The last one runs the script in a temp directory from `createTestTempDirectory`. The existing 503 test now passes `{ sleep: noSleep }` so it doesn't wait for real delays. - Base reproduction or other evidence: I didn't run the new tests against the base revision. Reading the base code, it makes one request per page and throws on the first failure, so the retry tests would not pass there. - User-visible CLI/MCP output: none. The only visible change is the failure note in the CI job summary, which I haven't seen in a real run. - Remaining proof gaps: no real registry run and no real timeout reproduction (the tests use a mocked `request`). I ran the tests locally on Windows only. I didn't run `npm run check` or `npm run verify:test-temp-hygiene`, and 23 tests in `releaseWorkflow.test.ts` are skipped on my machine. I haven't checked whether `mcpRegistryPublication.test.ts` ran in the CI shards. ## Validation performed - `npx vitest run tests/boundary/filesystem/mcpRegistryPublication.test.ts` — 9 passed, exit 0 - `npx vitest run tests/boundary/filesystem/releaseRecovery.test.ts tests/boundary/filesystem/releaseWorkflow.test.ts` — 17 passed, 23 skipped, exit 0 (same result without my changes) - `npx turbo run typecheck lint --force` — 2 successful, exit 0 - `npm run test:focused -- <test file>` — didn't run: exits 1 on Windows because `native/windows/build/manifest.json` is missing, before any test starts. I used `npx vitest run` directly instead. - GitHub Actions CI run #12934 on this PR: Static checks, Source tests, Build package, the native package baselines and the Windows capability lane passed. Portable tests failed: `Test shard (3/5)` has 2 failing tests in `tests/boundary/mcp/nativeUiFinalizationMcp.test.ts` ("retains native UI reports over MCP when its deadline expires during close", both cleanup variants), where `cancelled` / `cleanup_incomplete` was expected and `provider_timeout` was received. This PR only changes the three files listed above, and nothing in the diff touches native UI finalization code. I did not run that test locally, so I can't say for certain that the failure is unrelated. I can't re-run workflows from my fork. ## Compatibility, safety, and release - Breaking changes or migration steps: none. The existing error message (`Registry lookup failed: HTTP <status>`) is unchanged and the new parameter is optional. - Real Hopper/Ghidra, browser, or OS coverage: not applicable, I only ran checks on Windows. - Package or release metadata impact: none expected. - Security, privacy, process, or containment review: only the GET read is retried. Authentication and publish are untouched, and a persistent failure still fails the job before them. Worst case per page is about 110 s (3 x 30 s + 5 s + 15 s). ## Author readiness Human owner: @Rintu62 - [ ] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [ ] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [ ] I declare this PR ready for initial maintainer review. ## Before merge - [ ] This PR addresses a concrete problem or an agreed enhancement. - [ ] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [ ] The PR has one focused outcome and the title follows `type(scope): outcome`. - [ ] Related issue is linked, or the reason for not linking one is stated above. - [ ] Tests cover changed observable behavior and meaningful failure paths. - [ ] Owning docs, contracts, and generated metadata are updated where needed. - [ ] User-visible CLI/MCP changes include representative output. - [ ] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
morluto/rea#6740
No description provided.