Skip to content

fix(bonedigger): cleanup trap returns 0 so report exits cleanly - #889

Open
Xarianne wants to merge 1 commit into
projectbluefin:mainfrom
Xarianne:fix/bonedigger-cleanup-exit-code
Open

fix(bonedigger): cleanup trap returns 0 so report exits cleanly#889
Xarianne wants to merge 1 commit into
projectbluefin:mainfrom
Xarianne:fix/bonedigger-cleanup-exit-code

Conversation

@Xarianne

@Xarianne Xarianne commented Jul 29, 2026

Copy link
Copy Markdown

bluefin-common PR

What does this change?

Appends return 0 to the cleanup() EXIT trap in system_files/bluefin/usr/libexec/bonedigger-report so ujust report exits 0 on success instead of 1.

Why?

ujust report fully succeeds (gist uploaded, browser opened) but prints recipe 'report' failed with exit code 1 because the trap's last statement — [[ -n "$OTEL_STDERR" ]] && rm -f ... — returns 1 on an always-empty variable, and the happy path has no trap - EXIT to neutralize it.

Triggered by dakota#940. This supersedes bonedigger#25, which correctly fixed this in just/report.just in the bonedigger repo on 2026-06-13. Twelve hours later, bonedigger#26 centralized the report recipe into common as system_files/bluefin/usr/libexec/bonedigger-report and deleted the bonedigger copy — but the fix was not re-applied to the new common file, regressing the bug. common#655 stayed closed while the bug returned.

PR pipeline

opened ──▶ review ──▶ approved ──▶ merged
                    [lgtm]      auto-merge
                                when CI green

Add do-not-merge at any time to block automation.
/approve or lgtm from a maintainer triggers merge queue.

Checklist

  • PR title follows Conventional Commits (fix:, feat:, docs:, ci:, refactor:, etc.)
  • just check passes
  • pre-commit run --all-files passes
  • Skill doc updated if the change affects agent-facing conventions or behavior (see docs/skills/skill-improvement.md)
  • AGENTS.md / docs/SKILL.md / docs/skills/ links remain valid
  • CI is green after push: gh run list --repo projectbluefin/common --limit 5

AI attribution

Assisted-by: GLM 5.2 via opencode

@Xarianne
Xarianne force-pushed the fix/bonedigger-cleanup-exit-code branch 2 times, most recently from 8050eeb to 2af8006 Compare July 29, 2026 19:50
The cleanup() EXIT trap's last statement tested an always-empty OTEL_STDERR,
returning 1 and making just report 'recipe report failed with exit code 1'
even though ujust report fully succeeds. Append return 0 so the trap can
never override a successful exit.

Supersedes bonedigger#25, which fixed a dead just/report.just that never
propagated to the shipped bonedigger-report. Wires the existing bats test
into CI and adds a regression test.

Refs: common#655, bonedigger#25, dakota#940

Assisted-by: GLM 5.2 via opencode
@Xarianne
Xarianne force-pushed the fix/bonedigger-cleanup-exit-code branch from 2af8006 to 74f9590 Compare July 29, 2026 19:58
@Xarianne
Xarianne marked this pull request as ready for review July 29, 2026 20:05
@Xarianne
Xarianne requested review from a team, ahmedadan, castrojo, hanthor and repires as code owners July 29, 2026 20:05
@Xarianne

Copy link
Copy Markdown
Author

Soery for the amends, my agent decided to have an identity crisis and called itself CoPilot and went from following agents.md TO THE LETTER to ignoring it, so was trying to tame it before I posted the PR.

@hanthor hanthor left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approving. Correct fix — an EXIT trap handler's return value becoming the script's exit status is exactly the failure mode, and return 0 is the right guard rather than relying on every exit path remembering trap - EXIT. Regression test is well-targeted (extracts cleanup() and asserts status 0), and it's wired into both the Justfile and CI.

The note about the fix having previously landed in the bonedigger repo's stale just/report.just instead of the shipped common script is a genuinely useful post-mortem.


Generated by Claude Code

@Xarianne

Copy link
Copy Markdown
Author

Just saw #895 rewrote bonedigger-report and removed the cleanup trap entirely. Does the exit-code-1 issue still reproduce on main? Happy to close this if superseded.

@castrojo

castrojo commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Sorry I totally stepped on this while revising the report format. It should be much smoother for users though, once we get past this backlog it won't be so hectic, thanks!

@Xarianne

Copy link
Copy Markdown
Author

That's not a problem at all. Would you like me to close this? :)

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