Skip to content

watch.sh consumes a message for a role claimed between the lock read and the fetch #983

Description

@fujibee

scripts/watch.sh can consume and mark read a message addressed to a role another session has already claimed.

scripts/watch.sh reads the actas lock and fetches messages at two different points of the same poll turn:

:631  pair_state="$(actas_lock_state "$pair_team" "$pair_agent" "$SESSION_ID" ...)"
        ... 62 lines, including storage_store_exists and storage_read_cursor_get ...
:693  OUT="$(storage_watch_after "$READ_CURSOR" "$pair_team:$pair_agent" ...)"

If the role is claimed by another session between those two lines, the watcher has already decided the pair is still its own, and goes on to fetch — and mark read — a message addressed to a role it no longer owns. It notices on the next turn and exits with a message on stderr, which is why the failure looks intermittent rather than systematic.

That stderr line is evidence that the watcher eventually noticed. It is not evidence that it did not consume. Waiting for it does not close this.

Reproduction

One line inserted at watch.sh:632, between the two points above — nothing else changed:

sleep "${AGMSG_PROBE_WINDOW:-0}"

Then bats -f "claimed by another session" tests/test_actas_integration.bats:

AGMSG_PROBE_WINDOW result
0 ok
2 not ok[[ "$output" != *"after the handover"* ]] failed, i.e. the message was consumed

The delay is the only difference between the two runs, so the window is real and entering it is sufficient to lose a message.

Without the probe the test passes 20/20 on this machine, which is why it reads as a flake. It is not timing noise in the test — it is a window in the product that the test only sometimes lands in.

Why this cannot be fixed in the test

The test is outside the watcher and cannot observe which of :631 / :693 has happened. There is no moment it can choose that is known to be safe. Any test-side fix narrows the window rather than closing it.

Not proposing the fix here

The two candidates I can see are re-checking the lock immediately before the fetch, or making "check the lock, fetch, mark read" one indivisible step. The first only narrows the window again. I have not read enough of the poll loop to argue for the second, so this issue is the observation and the reproduction, not the design.

Related: #828 tracks the remaining sleep-based decisions in the same test file; this is the product side rather than the test side.

A note on the test's own reference

tests/test_actas_integration.bats:203 names this behaviour (#683), but #683 is about a per-team migration copying a read cursor past the destination high-water — an unrelated defect. I followed that number before reading the issue, reopened #683 and commented there in error; both have been retracted. The stale reference in the test name is worth correcting alongside this.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions