Skip to content

docs(release): stop the release comments misleading the next reader about what fails the build - #1603

Merged
fairchild merged 1 commit into
mainfrom
workspace/claude-1593-stale-copy-comments
Sep 12, 2026
Merged

docs(release): stop the release comments misleading the next reader about what fails the build#1603
fairchild merged 1 commit into
mainfrom
workspace/claude-1593-stale-copy-comments

Conversation

@fairchild

@fairchild fairchild commented Sep 11, 2026

Copy link
Copy Markdown
Owner

This PR changes comments only: 14 lines in, 12 out, in .github/workflows/ci.yml and scripts/verify-release-bundle.sh. The release build's copy step already stops the build when a resource tree is missing, empty or broken, but two comments still said the opposite — that a bad copy was swallowed — and one pointed at a line number that had moved. Anyone reading the release verifier would have been told the wrong thing about what the build catches. Issue #1593 asked for the comments to match the code. Nothing executable changes and the risk is nil; the reason to merge is that the next reader of the release lane gets the truth.

Summary

build-release.sh copies its resource trees through copy_tree_or_fail, which stops the build on a missing, empty, or broken copy. Two comments still described those copies as fail-open, and one cited a build-release.sh line number that has since moved. They now name copy_tree_or_fail and say what the verifier still covers:

  • .github/workflows/ci.yml (the comment on "Verify unsigned bundle structure"): the copies fail closed, but that proves a non-empty tree was copied, not what it held, and an unresolved Ghostty share dir still only warns. The same comment's "until now they only ran after signing" history clause became a present-state reason: the assertions need no secret, so they run on the PR.
  • scripts/verify-release-bundle.sh, header: the same statement, in two sentences.
  • scripts/verify-release-bundle.sh, the terminfo-sentinel comment: the gap is now a share dir holding both directories without the compiled entry, which copy_tree_or_fail copies clean because it only checks that the tree is non-empty. The build-release.sh:492 citation is gone, and the function is named instead.

Comment-only; no executable line changes. Closes #1593

Mergeability

  • Surface: infra / release lane — comments on the unsigned-bundle verify step in ci.yml and in scripts/verify-release-bundle.sh
  • User-facing behavior changed: No — comment-only
  • Non-happy paths considered: The comments describe the two states the verifier still catches after copy_tree_or_fail: an unresolved Ghostty share dir (warns, so the Ghostty and terminfo directories are absent) and a share dir with both directories but no compiled terminfo/78/xterm-ghostty (copies clean). They don't mention the asset compile, because the verifier has no Assets.car assertion.
  • Release/ops preconditions: None (comment-only)
  • Residual risk or follow-up: A third comment with the same stale claim and the build-release.sh:492 citation sits at Tests/WorkspaceManagerAppTests/ReleaseBundleVerificationScriptTests.swift:113-118. It's outside this PR's two-file fence, so I left it and reported it to the steward. The Ghostty-resolve warning itself is tracked in build-release.sh: two steps still succeed after doing nothing — an unresolved Ghostty share tree and a failed asset compile #1592.

Validation

  • bash -n scripts/verify-release-bundle.sh — exit 0, no output
  • actionlint .github/workflows/ci.yml (actionlint 1.7.12) — exit 0, no output
  • uv run --script scripts/validate-release-changes.py --changed-files <json listing the two files>Release change validation passed.
  • uv run --script scripts/tests/test_verify_release_bundle.pyRan 32 tests in 1.639s / OK
  • uv run --script scripts/tests/test_release_workflow.pyRan 6 tests in 0.000s / OK
  • git diff --check — clean
  • No Swift changed, so swift build / swift test don't apply

Performance

  • Not a performance-sensitive change

Evidence

  • Tests named below: the command and the line it printed

The commands and their result lines are listed under Validation.

Evidence Status

  • [complete] Diff: the two comments name copy_tree_or_fail and no longer say the copies are swallowed -- on 76fbb2f, .github/workflows/ci.yml:158 and scripts/verify-release-bundle.sh:14 name copy_tree_or_fail, and rg -n -e fail-open -e swallowed -e 'build-release\.sh:[0-9]' .github/workflows/ci.yml scripts/verify-release-bundle.sh prints nothing
  • [complete] bash -n scripts/verify-release-bundle.sh and the actionlint check pass on the PR head -- on 76fbb2f, bash -n scripts/verify-release-bundle.sh and actionlint .github/workflows/ci.yml (1.7.12) both exit 0 with no output

Blockers

  • None

🤖 Generated with Claude Code

https://claude.ai/code/session_01T3CL23nN7Fmu8mE5TB9CdT

Orchestrator note (workspaces)

build-release.sh copies its resource trees through copy_tree_or_fail, which
fails the build on a missing, empty, or failed copy. The comments on the
unsigned-bundle CI step and in verify-release-bundle.sh still described those
copies as fail-open, and one cited a build-release.sh line number that has
since moved. They now name copy_tree_or_fail and say what the verifier still
covers: an unresolved Ghostty share dir only warns, and a clean copy says
nothing about what the tree held.

Closes #1593

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T3CL23nN7Fmu8mE5TB9CdT
@fairchild fairchild added the author:claude-code PRs authored by the claude-code agent (interactive Claude Code sessions) label Sep 11, 2026
@vercel

vercel Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
workspaces Building Building Preview Sep 11, 2026 11:12am UTC

Request Review

@april-clearwater april-clearwater Bot 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.

April Clearwater, Application Lead

✅ Approve

Comment-only fix, and the new wording holds up against the code it describes.

Evidence: ✅ satisfied — both requested items (diff naming copy_tree_or_fail with the stale "swallowed"/build-release.sh:492 phrasing gone; bash -n + actionlint clean) are marked complete and match what the diff actually shows.

Verified the two rewritten comments against build-release.sh, plus the disclosed residual gap

This is a two-file comment change (.github/workflows/ci.yml, scripts/verify-release-bundle.sh), so the right evidence bar is syntax/lint validation plus a diff check that the stale claims are gone — not a build or a screenshot. That's what's provided, and I re-derived it independently rather than taking the author's word:

  • build-release.sh:115-127 (copy_tree_or_fail): fails on missing dir, empty dir, or a failed cp -R. The emptiness check is [[ -n "$(ls -A "$src")" ]] — non-empty, not "what it held." The rewritten comments say exactly this ("proves a non-empty tree was copied, not what it held").
  • build-release.sh:563-568: an unresolved Ghostty share dir hits log_warning only, no fail. Matches "an unresolved Ghostty share dir still only warns" in both rewritten comments.
  • Grepped .github/workflows/ci.yml and scripts/verify-release-bundle.sh for fail-open|swallowed|build-release\.sh:[0-9] post-diff — no matches, confirming the stale phrasing and the moved line citation (:492) are gone.
  • The terminfo-sentinel comment's new claim — a share dir can hold both Ghostty and terminfo directories without the compiled entry, and copy_tree_or_fail copies that clean — follows directly from the non-emptiness-only check above.

Residual risk is disclosed, not hidden: Tests/WorkspaceManagerAppTests/ReleaseBundleVerificationScriptTests.swift:113 still carries the same stale build-release.sh:492 citation. Confirmed it's still there and outside this PR's two-file fence. Reasonable to leave for a follow-up rather than scope-creep this PR, since the author flagged it in Mergeability rather than burying it.

No executable lines changed — confirmed the diff touches only #-prefixed comment lines in both files.

@fairchild

Copy link
Copy Markdown
Owner Author
  • 2026-09-11T11:13:52Z progress | gate on head 76fbb2f (origin/main 5b6bb9d, comment-only in two release-sensitive files)
  1. Bare in the worktree: bash -n scripts/verify-release-bundle.sh clean; actionlint .github/workflows/ci.yml clean; validate-release-changes.py passed for the two files; test_verify_release_bundle.py Ran 32, OK; test_release_workflow.py Ran 6, OK; git diff --check clean.
  2. Diff read: both comments now say the copies fail closed through copy_tree_or_fail, that this proves a non-empty tree and not its contents, and that an unresolved Ghostty share dir still only warns, which is what build-release.sh does since fix(release): resource copies into the bundle fail the build instead of logging success #1591. No PR number in the comment text. The third stale comment the worker found in ReleaseBundleVerificationScriptTests.swift is outside this fence and filed as a follow-up.
  3. Evidence: Two comments still describe the bundle resource copies as fail-open after #1591 #1593's two requested items read [complete] by the factory's own parser; readiness passed on the body with the release fields filled. CI actionlint and test green on this head; build-and-test still running (ci.yml changed, so the lane runs; comment-only diff).
  4. MERGEABLE with headRefOid equal to the worktree HEAD at flip time.

Flipping ready. Merge is the portfolio steward's under B4.

Orchestrator note (workspaces)

@fairchild
fairchild marked this pull request as ready for review September 11, 2026 11:13
@fairchild

Copy link
Copy Markdown
Owner Author

Codex pass — 2026-09-11, head 76fbb2f

The diff changes only comments in the two claimed files and addresses all three sites named by #1593. The new statements match the code: copy_tree_or_fail rejects missing or empty sources and missing destinations, preserves cp stderr, and exits nonzero on copy failure. Both the unconditional SPM copy and the resolved Ghostty copy use it; failure to resolve Ghostty still only warns. Resolution requires ghostty/ and terminfo/, but not the sentinel, while GhosttyResourcesLocator requires terminfo/78/xterm-ghostty, so the described incomplete tree is reachable. The CI job runs for this PR, has no job condition or continue-on-error, references no secrets, and --no-sign bypasses signing configuration and commands. All changed lines are ordinary comments outside YAML block scalars, heredocs, quoted strings, case bodies, and shellcheck directives. No tests depend on their line numbers or old text. git diff --check, bash -n, and actionlint passed. The new prose is accurate and readable.

Findings

  1. Tests/WorkspaceManagerAppTests/ReleaseBundleVerificationScriptTests.swift:113 (minor, non-blocking): the disclosed residual still says the Ghostty copy discards stderr and forces exit zero, and its build-release.sh:492 citation is obsolete. This leaves one contradictory maintainer-facing comment, but it is outside Two comments still describe the bundle resource copies as fail-open after #1591 #1593's three specified sites and cannot affect test behaviour. Compatible with CLEAR.

Not verified

  • No unsigned release build and no GitHub-hosted workflow run, because both write build artifacts. The bundle verification was not reproduced against real Ghostty/SPM outputs.
  • The two Python suites were not rerun, because they create temporary fixtures. Neither validates the changed prose: test_verify_release_bundle.py tests script behaviour and injection anchors, and test_release_workflow.py reads release.yml, not ci.yml. Their green results are therefore not evidence about this diff in either direction.
  • validate-release-changes.py was not rerun. Its RELEASE_PATHS excludes ci.yml, and for this diff it runs the same bash -n check on verify-release-bundle.sh; it does not inspect comment correctness.
  • The gate comment's claims and the current remote CI state were not checked from the worktree. The review used the local origin/main...HEAD diff at head 76fbb2f.

Verdict: CLEAR

Steward note (steward v9)

@april-clearwater

Copy link
Copy Markdown
Contributor

Verdict noted: CLEAR, with one non-blocking finding. The stale comment at Tests/WorkspaceManagerAppTests/ReleaseBundleVerificationScriptTests.swift:113 (wrong stderr/exit-code claim and the obsolete build-release.sh:492 citation) is out of scope for this PR's two-file fence and doesn't affect test behavior, so it's left as-is here. It's already filed as a follow-up per the Mergeability section's residual-risk note — the orchestrator should confirm a tracking issue exists for it, or open one, before this PR merges.

@fairchild fairchild added the steward:look Steward asks Michael to look: passes every leg, waits on his click. Max five. label Sep 12, 2026
@fairchild fairchild changed the title docs(release): the bundle copies fail closed, and the comments say so docs(release): stop the release comments misleading the next reader about what fails the build Sep 12, 2026
@fairchild
fairchild merged commit 84fa6e4 into main Sep 12, 2026
69 of 70 checks passed
@fairchild
fairchild deleted the workspace/claude-1593-stale-copy-comments branch September 12, 2026 18:55
@fairchild fairchild removed the steward:look Steward asks Michael to look: passes every leg, waits on his click. Max five. label Sep 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author:claude-code PRs authored by the claude-code agent (interactive Claude Code sessions) mergeable Agent-approved, ready for owner merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Two comments still describe the bundle resource copies as fail-open after #1591

1 participant