Skip to content

Commit cf364e8

Browse files
committed
no-mistakes(review): Rebalance shards and document teardown root cause
1 parent b1d7cae commit cf364e8

10 files changed

Lines changed: 175 additions & 123 deletions

.github/workflows/ci.yml

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,17 +30,17 @@ jobs:
3030
steps:
3131
- uses: actions/checkout@v6
3232
- name: Prove complete, disjoint shard coverage
33-
run: bin/fm-behavior-shards.sh --check 6
33+
run: bin/fm-behavior-shards.sh --check 8
3434

3535
behavior-tests:
36-
name: Behavior tests (shard ${{ matrix.shard }}/6)
36+
name: Behavior tests (shard ${{ matrix.shard }}/8)
3737
needs: behavior-test-plan
3838
runs-on: ubuntu-latest
3939
timeout-minutes: 15
4040
strategy:
4141
fail-fast: false
4242
matrix:
43-
shard: [1, 2, 3, 4, 5, 6]
43+
shard: [1, 2, 3, 4, 5, 6, 7, 8]
4444
steps:
4545
- uses: actions/checkout@v6
4646
with:
@@ -77,11 +77,11 @@ jobs:
7777
echo "TEMP=$shard_root/tmp"
7878
echo "TMUX_TMPDIR=$shard_root/tmux"
7979
} >> "$GITHUB_ENV"
80-
- name: Run behavior shard ${{ matrix.shard }}/6
80+
- name: Run behavior shard ${{ matrix.shard }}/8
8181
run: |
8282
set -eu
8383
manifest="$RUNNER_TEMP/executed-${{ matrix.shard }}.tsv"
84-
bin/fm-behavior-shards.sh --run "${{ matrix.shard }}" 6 "$manifest"
84+
bin/fm-behavior-shards.sh --run "${{ matrix.shard }}" 8 "$manifest"
8585
- name: Upload executed-test manifest
8686
if: always()
8787
uses: actions/upload-artifact@v4
@@ -108,7 +108,7 @@ jobs:
108108
path: ${{ runner.temp }}/behavior-manifests
109109
merge-multiple: true
110110
- name: Verify complete execution union
111-
run: bin/fm-behavior-shards.sh --verify 6 "$RUNNER_TEMP/behavior-manifests"
111+
run: bin/fm-behavior-shards.sh --verify 8 "$RUNNER_TEMP/behavior-manifests"
112112

113113
agent-fleet:
114114
name: Agent Fleet package

docs/ci-behavior-shards.md

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,36 +10,40 @@ The source run is [GitHub Actions run 30648119449](https://github.com/ruby-dlee/
1010

1111
`tests/behavior-test-durations.tsv` records the per-script measurements derived from that run.
1212

13-
The runner contract test added with the sharding implementation is included with a conservative two-second initial estimate, so the completeness guard covers it immediately.
13+
The runner contract test added with the sharding implementation and the newly landed `tests/lavish.test.sh` are included, so the completeness guard covers both immediately.
1414

1515
## Assignment
1616

1717
The planner uses deterministic longest-processing-time assignment.
1818

1919
It sorts by descending recorded duration, uses the test path as the stable secondary key, assigns the next test to the currently lightest shard, and breaks load ties by the lowest shard number.
2020

21-
The six-shard checked-in plan has these estimated serial loads.
21+
The eight-shard checked-in plan has these estimated serial loads.
2222

2323
| Shard | Tests | Estimated load |
2424
|---:|---:|---:|
25-
| 1 | 1 | 673400 ms (11m13s) |
26-
| 2 | 1 | 620746 ms (10m21s) |
27-
| 3 | 1 | 563637 ms (9m24s) |
28-
| 4 | 22 | 528938 ms (8m49s) |
29-
| 5 | 28 | 528929 ms (8m49s) |
30-
| 6 | 28 | 528933 ms (8m49s) |
25+
| 1 | 1 | 563637 ms (9m24s) |
26+
| 2 | 1 | 475500 ms (7m56s) |
27+
| 3 | 13 | 401927 ms (6m42s) |
28+
| 4 | 13 | 401942 ms (6m42s) |
29+
| 5 | 13 | 401927 ms (6m42s) |
30+
| 6 | 10 | 401979 ms (6m42s) |
31+
| 7 | 16 | 401935 ms (6m42s) |
32+
| 8 | 17 | 401936 ms (6m42s) |
3133

32-
The expected healthy critical path is therefore about 11 minutes rather than 57 minutes.
34+
The expected healthy behavior-execution critical path is 9 minutes 24 seconds rather than 57 minutes 23 seconds.
3335

34-
The largest single test file, `tests/fm-account-routing.test.sh`, sets the remaining floor.
36+
The account-routing and report-stack suites keep their original test functions and assertions in shared suite files, while two runner wrappers partition each call list deterministically between isolated runners.
37+
38+
The largest remaining indivisible test file, `tests/fm-teardown.test.sh`, sets the 9-minute-24-second floor.
3539

3640
## Coverage guard
3741

38-
`bin/fm-behavior-shards.sh --check 6` fails when the duration inventory has a missing path, duplicate path, malformed duration, or any difference from the complete `tests/*.test.sh` inventory.
42+
`bin/fm-behavior-shards.sh --check 8` fails when the duration inventory has a missing path, duplicate path, malformed duration, or any difference from the complete `tests/*.test.sh` inventory.
3943

4044
Every matrix runner writes an executed manifest while continuing through all assigned scripts and preserving each exit code.
4145

42-
The final `Behavior tests` job downloads all six manifests and runs `bin/fm-behavior-shards.sh --verify 6 <manifest-dir>`.
46+
The final `Behavior tests` job downloads all eight manifests and runs `bin/fm-behavior-shards.sh --verify 8 <manifest-dir>`.
4347

4448
Verification fails for a missing shard, missing test, duplicate test, wrong shard assignment, malformed row, or recorded test failure.
4549

@@ -55,17 +59,29 @@ The workflow also assigns each shard private mode-0700 `TMPDIR` and `TMUX_TMPDIR
5559

5660
Scripts remain serial within a shard, so no existing test needed weaker assertions, a mock conversion, a skip, a retry, or an added sleep.
5761

58-
Each matrix job is named `Behavior tests (shard N/6)`, and the runner emits explicit begin and end markers containing the test path and exit code.
62+
Each matrix job is named `Behavior tests (shard N/8)`, and the runner emits explicit begin and end markers containing the test path and exit code.
63+
64+
The 15-minute per-shard timeout leaves bounded margin above the 9m24s slowest planned shard while replacing the prior 90-minute blanket.
65+
66+
## Teardown child-endpoint investigation
67+
68+
GitHub Actions run 30649486198 failed at 17:52:54 UTC after `test_forced_secondmate_child_uses_child_home_for_endpoint_verification` correctly retained the Agent Fleet lease, child metadata, and child worktree but its final message assertion expected the endpoint state to be `unknown`.
69+
70+
The Zellij fixture returns a live session, pane, and tab from the child firstmate home, so endpoint discovery deterministically classifies it as `present` before destructive cleanup and emits `managed endpoint ... is still alive`.
71+
72+
The failure was therefore an assertion-ordering mismatch introduced when the fixture became capable of proving presence, not a delayed endpoint shutdown.
73+
74+
Commit `d7db2dcb010cc48f4ca6d34b4386d934e6c9cdde` fixes the assertion to match the proven state while retaining the refusal and all three containment checks.
5975

60-
The 15-minute per-shard timeout leaves bounded margin above the measured 11m13s slowest shard while replacing the prior 90-minute blanket.
76+
GitHub Actions run 30657355610 executed the corrected sequence at 19:51:41 UTC and passed the full behavior job without a retry or added sleep.
6177

6278
## Refreshing timings
6379

6480
Run the complete suite serially in the target environment and atomically replace the duration data only if every test passes.
6581

6682
```sh
6783
bin/fm-behavior-shards.sh --record tests/behavior-test-durations.tsv
68-
bin/fm-behavior-shards.sh --check 6
84+
bin/fm-behavior-shards.sh --check 8
6985
```
7086

7187
Review the new `--check` load summary before committing refreshed timings.

tests/behavior-test-durations.tsv

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
# Recorded from https://github.com/ruby-dlee/firstmate/actions/runs/30648119449 on 2026-07-31.
44
55000 tests/bridge-cutover-python.test.sh
55
71100 tests/fm-account-directory.test.sh
6-
673400 tests/fm-account-routing.test.sh
6+
336700 tests/fm-account-routing-a.test.sh
7+
336700 tests/fm-account-routing-b.test.sh
78
33600 tests/fm-afk-inject-e2e.test.sh
89
20 tests/fm-afk-inject-herdr-e2e.test.sh
910
16500 tests/fm-afk-launch.test.sh
@@ -52,7 +53,8 @@
5253
3170 tests/fm-pi-watch-extension.test.sh
5354
1574 tests/fm-pr-merge.test.sh
5455
802 tests/fm-prompt-exec.test.sh
55-
620746 tests/fm-report-stack.test.sh
56+
310373 tests/fm-report-stack-a.test.sh
57+
310373 tests/fm-report-stack-b.test.sh
5658
495 tests/fm-review-diff.test.sh
5759
26463 tests/fm-secondmate-harness.test.sh
5860
37513 tests/fm-secondmate-lifecycle-e2e.test.sh

tests/fm-account-routing-a.test.sh

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
#!/usr/bin/env bash
2+
set -u
3+
FM_TEST_PART_INDEX=1 FM_TEST_PART_TOTAL=2 . "$(dirname "${BASH_SOURCE[0]}")/fm-account-routing-suite.sh"

tests/fm-account-routing-b.test.sh

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
#!/usr/bin/env bash
2+
set -u
3+
FM_TEST_PART_INDEX=2 FM_TEST_PART_TOTAL=2 . "$(dirname "${BASH_SOURCE[0]}")/fm-account-routing-suite.sh"
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5939,11 +5939,20 @@ run_isolated_test() {
59395939
# a condition context suppresses errexit for the whole test body, so an
59405940
# in-test `set -e` would be silently ignored.
59415941
local status
5942+
FM_TEST_PART_SEQUENCE=$((FM_TEST_PART_SEQUENCE + 1))
5943+
if [ "$FM_TEST_PART_TOTAL" -gt 1 ] \
5944+
&& [ $(((FM_TEST_PART_SEQUENCE - 1) % FM_TEST_PART_TOTAL + 1)) -ne "$FM_TEST_PART_INDEX" ]; then
5945+
return 0
5946+
fi
59425947
( "$@" )
59435948
status=$?
59445949
[ "$status" -eq 0 ] || exit "$status"
59455950
}
59465951

5952+
FM_TEST_PART_INDEX=${FM_TEST_PART_INDEX:-1}
5953+
FM_TEST_PART_TOTAL=${FM_TEST_PART_TOTAL:-1}
5954+
FM_TEST_PART_SEQUENCE=0
5955+
59475956
if [ "${FM_TEST_FOCUSED:-}" = stale-reclaim-generation ]; then
59485957
run_isolated_test test_stale_reclaim_guard_is_owned_before_lock_removal
59495958
exit 0

tests/fm-behavior-shards.test.sh

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ set -u
99
SHARDER="$ROOT/bin/fm-behavior-shards.sh"
1010
DURATIONS="$ROOT/tests/behavior-test-durations.tsv"
1111
CI="$ROOT/.github/workflows/ci.yml"
12-
SHARD_COUNT=6
12+
SHARD_COUNT=8
1313

1414
test_checked_in_plan_is_complete_balanced_and_deterministic() {
1515
local tmp plan_a plan_b inventory planned out
@@ -21,8 +21,8 @@ test_checked_in_plan_is_complete_balanced_and_deterministic() {
2121

2222
out=$("$SHARDER" --check "$SHARD_COUNT") \
2323
|| fail "checked-in behavior shard plan failed its coverage guard"
24-
assert_contains "$out" "FM_BEHAVIOR_PLAN ok tests=82 shards=6" \
25-
"coverage guard did not report the complete 82-test inventory"
24+
assert_contains "$out" "FM_BEHAVIOR_PLAN ok tests=84 shards=8" \
25+
"coverage guard did not report the complete 84-test inventory"
2626
"$SHARDER" --plan "$SHARD_COUNT" > "$plan_a"
2727
"$SHARDER" --plan "$SHARD_COUNT" > "$plan_b"
2828
cmp -s "$plan_a" "$plan_b" || fail "same durations produced different shard plans"
@@ -164,7 +164,7 @@ test_post_run_guard_requires_the_exact_executed_union() {
164164
write_complete_manifests "$plan" "$good"
165165
out=$("$SHARDER" --verify "$SHARD_COUNT" "$good") \
166166
|| fail "post-run guard rejected the exact complete manifest union"
167-
assert_contains "$out" "FM_BEHAVIOR_COMPLETENESS ok tests=82 shards=6" \
167+
assert_contains "$out" "FM_BEHAVIOR_COMPLETENESS ok tests=84 shards=8" \
168168
"post-run guard did not report complete execution"
169169

170170
cp -R "$good" "$missing"
@@ -201,9 +201,9 @@ test_post_run_guard_requires_the_exact_executed_union() {
201201

202202
test_ci_wires_matrix_isolation_timeout_and_union_verification() {
203203
# shellcheck disable=SC2016 # The GitHub expression is a literal YAML needle.
204-
assert_grep 'name: Behavior tests (shard ${{ matrix.shard }}/6)' "$CI" \
204+
assert_grep 'name: Behavior tests (shard ${{ matrix.shard }}/8)' "$CI" \
205205
"CI does not expose the failing shard in the job name"
206-
assert_grep 'shard: [1, 2, 3, 4, 5, 6]' "$CI" \
206+
assert_grep 'shard: [1, 2, 3, 4, 5, 6, 7, 8]' "$CI" \
207207
"CI does not launch every deterministic shard"
208208
[ "$(grep -Fc 'timeout-minutes: 15' "$CI")" -eq 1 ] \
209209
|| fail "behavior matrix must have one 15-minute per-shard timeout"
@@ -214,13 +214,13 @@ test_ci_wires_matrix_isolation_timeout_and_union_verification() {
214214
assert_grep 'echo "TMUX_TMPDIR=$shard_root/tmux"' "$CI" \
215215
"CI does not give each shard a private tmux socket root"
216216
# shellcheck disable=SC2016 # Workflow shell variables must remain literal.
217-
assert_grep 'bin/fm-behavior-shards.sh --verify 6 "$RUNNER_TEMP/behavior-manifests"' "$CI" \
217+
assert_grep 'bin/fm-behavior-shards.sh --verify 8 "$RUNNER_TEMP/behavior-manifests"' "$CI" \
218218
"CI does not verify the union of executed manifests"
219219
assert_grep 'overwrite: true' "$CI" \
220220
"CI cannot refresh a failed shard manifest during a failed-jobs rerun"
221221
assert_no_grep 'for test_script in tests/*.test.sh' "$CI" \
222222
"CI retained the 57-minute serial behavior loop"
223-
pass "CI wires six named isolated shards, a tight timeout, and executed-union verification"
223+
pass "CI wires eight named isolated shards, a tight timeout, and executed-union verification"
224224
}
225225

226226
test_checked_in_plan_is_complete_balanced_and_deterministic

tests/fm-report-stack-a.test.sh

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
#!/usr/bin/env bash
2+
set -u
3+
FM_TEST_PART_INDEX=1 FM_TEST_PART_TOTAL=2 . "$(dirname "${BASH_SOURCE[0]}")/fm-report-stack-suite.sh"

tests/fm-report-stack-b.test.sh

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
#!/usr/bin/env bash
2+
set -u
3+
FM_TEST_PART_INDEX=2 FM_TEST_PART_TOTAL=2 . "$(dirname "${BASH_SOURCE[0]}")/fm-report-stack-suite.sh"

0 commit comments

Comments
 (0)