Skip to content

Read activestorage's own block in the hosted gem leg - #1946

Merged
Mikola Lysenko (mikolalysenko) merged 1 commit into
mainfrom
ci-janitor/gem-leg-own-block
Nov 2, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 1 commit into
mainfrom
ci-janitor/gem-leg-own-block

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Nov 1, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

hosted-e2e is required through ci-ok. Since about 17:12 UTC on 2026-11-01 it has failed on every non-draft PR. Examples: 41330638545 and 41330161127, each red on 2 attempts with 3 internal retries. Tracking issue: #1945.

There are two causes, both on the production side:

  1. bun: patch.socket.dev returns 403 to bun for the hosted minimist artifact. That has to be fixed in production, or bypassed with HOSTED_E2E_DISABLED. This PR does not fix it.
  2. gem: production now publishes patches for actionpack and activesupport 6.0.3, which are dependencies of activestorage. The hosted Gemfile therefore holds several patch-registry blocks. wired_gem_registry_uuid read the first block, so the leg checked actionpack's patch (f86d5caa…) against GEM_UUIDS. It then failed with a misleading "verify the new patch and extend GEM_UUIDS" message. In some retries the leg passed, depending on which block came first.

Root cause

The test parser assumed the Gemfile holds exactly one Socket source block.

Fix

wired_gem_registry_uuid(gemfile, name) now returns the UUID only from the block whose body declares gem "<name>". If activestorage is not wired at all, which is what the failing tries show, the leg now fails with "the hosted scan did not redirect activestorage itself". It no longer blames a neighbour's patch. A new non-ignored unit test, wired_gem_registry_uuid_reads_the_named_gems_block, covers:

  • blocks in any order
  • the unwired-activestorage case
  • a gem whose name starts with activestorage

No assertion was weakened, and no test was removed or moved.

Proof

  • cargo test -p socket-patch-cli --test e2e_hosted_production wired_gem passes.
  • rustfmt was run on the touched file only.
  • I could not run the live leg: this sandbox's proxy blocks the production API.

Generated by Claude Code


Note

Low Risk
Test-only changes to hosted e2e Gemfile parsing; no runtime CLI or redirect behavior is modified.

Overview
Fixes flaky/misleading failures in the hosted production gem e2e leg after production started wiring patch-registry blocks for activestorage transitive gems (actionpack, activesupport).

wired_gem_registry_uuid now takes a gem name and scans each Socket source … do block until it finds one whose body declares gem "<name>", then reads that block’s patch UUID. It no longer returns the first registry block, which could belong to a dependency and be checked against GEM_UUIDS for activestorage.

The gem install proof calls the helper with GEM_NAME and uses a clearer panic when activestorage itself was never redirected. A new unit test covers block ordering, unwired activestorage beside wired transitives, and non-prefix gem name matching.

Reviewed by Cursor Bugbot for commit 75b6f65. Configure here.


Generated by Claude Code

Production now publishes patches for activestorage's dependencies
(actionpack, activesupport 6.0.3), so the hosted Gemfile rewrite can
hold several patch-registry blocks. wired_gem_registry_uuid took the
first one, so the leg checked actionpack's UUID against GEM_UUIDS and
failed with a misleading "extend GEM_UUIDS" message whenever a
transitive block came first or activestorage was not wired at all
(hosted-e2e, 2026-11-01, runs 41330638545 and 41330161127).

Match the block whose body declares gem "activestorage". When
activestorage is not wired, the leg now says so instead of naming a
neighbour's patch. Add a unit test for the parser.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011f9aeRg9Pkq38o8d4ttAvb
@mikolalysenko Mikola Lysenko (mikolalysenko) added the ci-janitor Opened by the CI janitor routine (flakes, redundant tests, CI perf) label Nov 1, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


Generated by Claude Code

@cursor cursor 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.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 75b6f65. Configure here.

@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Nov 1, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[burn-down agent] Labeled Ready for review at 75b6f65.


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final review brief

What it does. Test-only fix for the hosted-e2e gem leg. Production now publishes patches for activestorage's dependencies too, so the hosted Gemfile can hold several Socket source ... do blocks. The helper used to read the first block, which could be a neighbour's patch. It now reads only the block that declares gem "activestorage". If that block is missing, the leg fails with a clear "did not redirect activestorage itself" message. The separate bun 403 is not fixed here (#1945).

Risk: low. One test file. No CLI or product code. The new unit test runs on every PR.

Look here

  • e2e_hosted_production.rs#L720-L739:`` the block scan. The marker matches the rewriter's source "<idx>" do format (`upstream/gem.rs`).
  • #L730: the body ends at the first line that starts with end. An indented end would let the body run on into the next block. The rewriter writes end at column 0, so this does not matter today.
  • #L742-L776:``` unit test. It covers block order, an unwired activestorage beside wired transitive gems, and a gem whose name starts with activestorage.
  • #L2251:```` call site.

Verified

  • Read the full diff. CHANGELOG.md is untouched. No debug code.
  • CRLF Gemfiles are handled: lines() strips the \r, and "\nend" still matches.
  • Ran cargo test -p socket-patch-cli --test e2e_hosted_production wired_gem on the head: 1 passed. I used the stable toolchain because this container's pinned 1.93.1 toolchain has no cargo.
  • Without the fix, the old first-block read returns u-actionpack for the first fixture, so the new test catches the old behaviour.
  • CI: 31 checks green, 19 skipped, 0 failing. ci-ok and clippy are green. Bugbot reviewed this head and rated it Low Risk with no findings.

Changes I made: none.

Open questions: none.

Auto-merge is armed, so approving sends this PR straight to the merge queue.


Generated by Claude Code

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.

[agent] Reviewed 75b6f65395b0db0f97635da7f4e3ebbff41db6fc. Checked named-gem block selection, UUID extraction, missing-target and prefix regressions. No major correctness issues found. Validation: source and regression-test review plus the available exact-head CI checks; no local test pass is claimed.

@mikolalysenko
Mikola Lysenko (mikolalysenko) added this pull request to the merge queue Nov 2, 2026
Merged via the queue into main with commit 2e3e30e Nov 2, 2026
92 of 94 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the ci-janitor/gem-leg-own-block branch November 2, 2026 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-janitor Opened by the CI janitor routine (flakes, redundant tests, CI perf) Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants