Skip to content

Show the Claude runtime regeneration command - #225

Merged
n3wth merged 3 commits into
google:mainfrom
Aaravkhanal:codex/packaging-failure-command
Oct 30, 2026
Merged

n3wth merged 3 commits into
google:mainfrom
Aaravkhanal:codex/packaging-failure-command

Conversation

@Aaravkhanal

@Aaravkhanal Aaravkhanal commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Refs #219

Contribution Type

  • Bug Fix / Minor Patch
  • Feature / Architectural Change -- Stage 1: Design Doc PR
  • Feature / Architectural Change -- Stage 2: Implementation PR

Summary of Key Changes

  • Replace the vague Claude runtime freshness failure hint with the complete command: python3 scripts/pack_plugin.py --target claude --output <output-dir>.
  • Explain that the generated egm-claude folder should replace distributions/claude, preventing stale files from remaining.
  • This is the separate failure-message follow-up requested in Require passing unit tests before merging #219. It does not change packaging behavior.

Suggested label: ci.

Human Accountability Affirmation

  • I am a human contributor. I have personally reviewed, understood, and can explain every line of this change in review (using AI tools to write code is welcome, but human understanding of the submitted change is required).

Verification

  • For Design Doc PRs (Stage 1): Not applicable.

  • For Bug Fix / Implementation PRs (Stage 2): Executed unit tests; details below.

  • For Harness Support PRs: Not applicable.

  • pytest tests/unit -k 'not published_claude_runtime_is_current' passes on current main.

  • The full suite passes when the generated Claude README sync from fix(packaging): sync the packaged readme #209 is applied temporarily. fix(packaging): sync the packaged readme #209 is still open, so the unchanged test_published_claude_runtime_is_current failure remains on current main until that fix lands; the focused run confirms this PR now prints the full regeneration command and replacement guidance.

  • git diff --check passes.

@n3wth n3wth left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed da7649e. The command is correct: --target and --output are both required by pack_plugin.py, and the builder writes <output>/egm-claude. One wording fix inline, non-blocking.

The PR only changes the assertion message, so I did not run the tests. The new text is reachable only while test_published_claude_runtime_is_current fails, which it does on main until #209 lands, and I did not trigger that failure to print the message. cla/google and the scope/template check pass; the Tests workflow has not run on this head.

Comment thread tests/unit/test_packaging.py Outdated

@n3wth n3wth left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Re-reviewed at 67ab9de. The only change since da7649e is the wording fix from my last round: the message now uses python3 and says to replace distributions/claude with the generated egm-claude folder, which matches docs/directory.md and avoids leaving stale files behind.

I ran tests/unit locally at this head. The one failure is test_published_claude_runtime_is_current, from the README.md drift that #209 fixes, and it prints the new message as written. The Tests workflow has not run on this head; cla/google and the scope/template check pass.

Nothing blocking.

Comment thread tests/unit/test_packaging.py

@recursivethinker recursivethinker left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@n3wth
n3wth merged commit 9fb3548 into google:main Oct 30, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants