Skip to content

test(ready): cover include-ephemeral assigned no-history work#4140

Merged
julianknutsen merged 3 commits into
mainfrom
codex/ready-include-ephemeral-assignee-test-20260523
Jun 3, 2026
Merged

test(ready): cover include-ephemeral assigned no-history work#4140
julianknutsen merged 3 commits into
mainfrom
codex/ready-include-ephemeral-assignee-test-20260523

Conversation

@julianknutsen

Copy link
Copy Markdown
Collaborator

Summary:

  • Add a regression test for assigned ready work when IncludeEphemeral=true.
  • Verify no-history assigned work remains in default ready results while true ephemeral assigned work is only included with IncludeEphemeral.
  • Keep the PR test-only against origin/main; the commit cherry-picked cleanly without the 4107 branch.

Tests:

  • ./scripts/test.sh -run '^TestGetReadyWork_(MetadataSuite|IncludeEphemeralAssigneeIsSuperset)$' ./cmd/bd

@julianknutsen julianknutsen added status/needs-review-auto Request automated PR review workflow status/reviewing PR review workflow is running and removed status/needs-review-auto Request automated PR review workflow labels May 23, 2026
@codecov-commenter

codecov-commenter commented May 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@julianknutsen julianknutsen added status/review-failed PR-review workflow failed before merge-ready status/reviewing PR review workflow is running and removed status/reviewing PR review workflow is running status/review-failed PR-review workflow failed before merge-ready labels Jun 2, 2026
@julianknutsen julianknutsen force-pushed the codex/ready-include-ephemeral-assignee-test-20260523 branch from 22f70d4 to 40926ab Compare June 3, 2026 16:22
@julianknutsen

Copy link
Copy Markdown
Collaborator Author

Maintainer Adoption Review

Thanks for the contribution, @julianknutsen! This PR adds regression coverage for assigned ready work when IncludeEphemeral=true, including the important distinction that no-history assigned work remains visible by default while true ephemeral assigned work only appears when explicitly requested. That matters because ready-work routing depends on these visibility boundaries staying predictable for both normal CLI users and workflow/runtime callers.

This PR was reviewed and adopted with maintainer fixes pushed directly to the PR branch.

Original PR Review

Decision: approve.

Specific gaps fixed:

  • The review found that workflow-only edits to .github/workflows/regression.yml could skip the differential regression job; the final patch keeps the detector sensitive to regression workflow changes while still skipping ordinary test-only CLI edits.
  • The ready-work tests now use newTestStore, matching the branch-per-test helper pattern used by this package instead of the heavier isolated database setup.
  • The final test shape is lower-noise: the adopted patch adds t.Parallel(), removes an unrelated Limit, and reuses issueIDs for the assertions.

Review findings addressed:

  • Release Safety (major, high / all reviewers): workflow-only regression changes could bypass the regression job; the final patch updates the detector and the final head passed the regression workflow.
  • Architectural Consistency (major, high / Claude + DeepSeek): the new ready-work tests used isolated databases where the documented single-store helper was enough; the final patch uses newTestStore.
  • Test Evidence Quality (minor, medium / Claude + DeepSeek): the first pass had avoidable helper/query noise; the final patch simplifies the test and keeps the assertions focused on assigned ready-work visibility.

Remaining non-gating notes:

  • sameStringSet overlaps with the existing requireIssueIDs assertion helper and can be collapsed later if maintainers want one assertion style.
  • The regression detector now has two related regexes that should be kept in sync during future edits.
  • A short local comment near the newTestStore setup could help future debugging, but no remaining gating issue was found.

Maintainer Changes

Maintainer fixes were pushed directly to the PR branch:

  • fix(ci): skip regression for test-only CLI changes (bd-g72vf)
  • fix: address ready-work review findings

Maintainer diff after the contributor test commit: 2 files changed, 12 insertions(+), 14 deletions(-).

Final Review Status

Ready for the merge queue: final head 40926abb2 has passing required GitHub checks, and the merge-ready metadata is in place. I am marking this PR status/merge-ready so the merge queue can pick it up.

CI: PR https://github.com/gastownhall/beads/actions/runs/26898102553; PR Risk https://github.com/gastownhall/beads/actions/runs/26898102546; Regression Tests https://github.com/gastownhall/beads/actions/runs/26898103016; Cross-Version Smoke Tests https://github.com/gastownhall/beads/actions/runs/26898102533

Review Iterations

2 review passes performed. Attempt 1 requested changes for regression workflow coverage and ready-work test structure; the maintainer fixes addressed those findings, and attempt 2 approved the final patch with only non-gating cleanup notes.


Adopted via /adopt-pr workflow. Original contributor commits preserved.

@julianknutsen julianknutsen added status/merge-ready PR is ready for merge workflow status/merge-queued Queued for deterministic PR-review merge and removed status/merge-ready PR is ready for merge workflow status/reviewing PR review workflow is running labels Jun 3, 2026
@julianknutsen julianknutsen merged commit 26630c8 into main Jun 3, 2026
51 checks passed
@julianknutsen julianknutsen removed the status/merge-queued Queued for deterministic PR-review merge label Jun 3, 2026
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.

2 participants