fix(watch): fold sentinel '-' into the empty session-id path - #857
Merged
Conversation
…#855) Callers still pass "${GROK_SESSION_ID:--}" from fujibee#477, but watch.sh stopped folding the literal dash, so monitor re-eval wrote watch.-.pid. Restore the one-line fold and the bats case that locked it. Co-authored-by: Cursor <cursoragent@cursor.com>
…old case The restored bats locks the fujibee#477 fold, not the later regression ticket. Co-authored-by: Cursor <cursoragent@cursor.com>
6 tasks
Non-last `! cmd` is silent on bash 3.2, so the new fold case raised the enforced-assertions baseline. refute fails the test on every platform. Co-authored-by: Cursor <cursoragent@cursor.com>
This was referenced Aug 17, 2026
Owner
|
Thanks for this, and sorry it sat. This was red for a reason that had nothing to do with your change. One macOS Every ubuntu shard, every Windows leg and the other three macOS shards were The cap is being hit because a handful of tests are slow: eight cases account for Nothing is needed from you. Review to follow. |
4 tasks
fujibee
added a commit
that referenced
this pull request
Aug 19, 2026
…timeout with real margin (#885) * fix(ci): pin count-invisible heavy files apart, and raise the shard cap with real margin (#847, #848) @test count is a loose proxy for wall-clock cost in general, and blind for a specific shape of file: one whose cost is almost entirely waiting rather than how many @test blocks it contains. Measured 2026-08-19 (run 32193147987): test_remote_engine_start_refusal.bats (9 tests, 722s, ~80s/test) and test_remote_status_liveness.bats (31 tests, 380s, ~12s/test) both sit near the bottom of the count-weighted sort while carrying two of the largest absolute durations in the suite -- invisible to the weight the greedy partition uses, so nothing stops an unrelated test-count change elsewhere from repacking them onto the same shard (#847's demonstrated failure mode). shard-tests.sh now seeds these two into distinct shards before the ordinary count-weighted pass runs, so their placement no longer depends on any other file's test count. tests.yml's bats-shard timeout-minutes moves from 25 to 30, with the arithmetic behind that number in a comment, and its comment claiming a timeout is always a genuine hang is corrected (#848) -- #857 hit exactly the opposite: 335/335 tests ok, cancelled 12s after its last one, at the cap. Files that are merely large by test count (e.g. a 179-test file at an ordinary ~1.2s/test) were checked and deliberately left unpinned: count already weights them correctly. * address co1's static review P2s (#847, #848) - test name now says >= 2 (total=1 trivially wraps both pins into the same slot, same as every other file) instead of claiming a property that does not hold there - shard-tests.sh's pin comment now names a concrete follow-up: re-run the same seconds-per-test measurement once #876 et al. land, since they change these two files' real cost and can make the pin list stale or incomplete
fujibee
added a commit
that referenced
this pull request
Sep 11, 2026
…artup A helper that a test backgrounds and that runs until killed holds every descriptor it inherits for as long as it lives -- including, under a parallel bats run, the harness's own high pipes. #575 fixed this class for the sync engine (scripts/lib/close-fds.sh); the mock was not covered. It now closes every descriptor at or above 3 before it opens the listen socket, so a survivor holds nothing the harness opened; 0/1/2 stay (the port is printed on stdout). This is HYGIENE, not a fix for the macOS shard cancellations tracked in #1107. Reading the hang-samples as a time series shows the shard still starting new tests right up to the cut-off: it is a wall-clock cap being reached by a still-progressing shard (the workflow's own #848/#857 note), not a descriptor-inheritance hang. Kept separate so the review and the ledger stay accurate; the mock/fd was a bystander caught in a snapshot, not the cause. test_engine_inherited_fds.bats grows the matching regression (baseline comparison, mutation-checked); test_team_list.bats reaps its mock in teardown.
This was referenced Sep 11, 2026
fujibee
added a commit
that referenced
this pull request
Sep 11, 2026
…artup A helper that a test backgrounds and that runs until killed holds every descriptor it inherits for as long as it lives -- including, under a parallel bats run, the harness's own high pipes. #575 fixed this class for the sync engine (scripts/lib/close-fds.sh); the mock was not covered. It now closes every descriptor at or above 3 before it opens the listen socket, so a survivor holds nothing the harness opened; 0/1/2 stay (the port is printed on stdout). This is HYGIENE, not a fix for the macOS shard cancellations tracked in #1107. Reading the hang-samples as a time series shows the shard still starting new tests right up to the cut-off: it is a wall-clock cap being reached by a still-progressing shard (the workflow's own #848/#857 note), not a descriptor-inheritance hang. Kept separate so the review and the ledger stay accurate; the mock/fd was a bystander caught in a snapshot, not the cause. test_engine_inherited_fds.bats grows the matching regression (baseline comparison, mutation-checked); test_team_list.bats reaps its mock in teardown.
fujibee
added a commit
that referenced
this pull request
Sep 12, 2026
…artup (#1156) A helper that a test backgrounds and that runs until killed holds every descriptor it inherits for as long as it lives -- including, under a parallel bats run, the harness's own high pipes. #575 fixed this class for the sync engine (scripts/lib/close-fds.sh); the mock was not covered. It now closes every descriptor at or above 3 before it opens the listen socket, so a survivor holds nothing the harness opened; 0/1/2 stay (the port is printed on stdout). This is HYGIENE, not a fix for the macOS shard cancellations tracked in #1107. Reading the hang-samples as a time series shows the shard still starting new tests right up to the cut-off: it is a wall-clock cap being reached by a still-progressing shard (the workflow's own #848/#857 note), not a descriptor-inheritance hang. Kept separate so the review and the ledger stay accurate; the mock/fd was a bystander caught in a snapshot, not the cause. test_engine_inherited_fds.bats grows the matching regression (baseline comparison, mutation-checked); test_team_list.bats reaps its mock in teardown.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #855.
Restores the #477 one-line fold of sentinel
-inwatch.sh, and the bats case that locked it. Callers that pass"${GROK_SESSION_ID:--}"are unchanged. Problem, repro, and why this is not a caller fix are on the issue.Independently mergeable. #858 no longer contains these commits (unstacked). Cursor monitor still needs this fold at runtime (
"${CURSOR_CONVERSATION_ID:--}"), so please land this before #858. #858 stays a draft until then.Test plan
bats tests/test_watch.bats --filter "sentinel '-' session_id".github/scripts/check-enforced-assertions.shat baseline