fix: the heartbeat guard grepped its own explanation (4295)
CI and images / lint (push) Successful in 3s
CI and images / extension-version (push) Successful in 3s
CI and images / frontend-build (push) Successful in 24s
CI and images / backend-lint-and-test (push) Successful in 32s
CI and images / integration (push) Successful in 2m9s
CI and images / sign-extension (push) Successful in 3s
CI and images / build-web (push) Successful in 1m48s
CI and images / smoke-web (push) Successful in 55s
CI and images / build-agent (push) Successful in 8m56s
CI and images / promote (push) Skipped
CI and images / lint (push) Successful in 3s
CI and images / extension-version (push) Successful in 3s
CI and images / frontend-build (push) Successful in 24s
CI and images / backend-lint-and-test (push) Successful in 32s
CI and images / integration (push) Successful in 2m9s
CI and images / sign-extension (push) Successful in 3s
CI and images / build-web (push) Successful in 1m48s
CI and images / smoke-web (push) Successful in 55s
CI and images / build-agent (push) Successful in 8m56s
CI and images / promote (push) Skipped
Run 7375: the test that asserts the agent's heartbeat is not gated on holding
leases failed — on the docstring of the fix, which quotes the construct the
fix removed, because that is what a docstring explaining a fix does.
assert "if ids:" not in loop
A source-TEXT assertion cannot tell code from prose about code. Parsed now:
the function's AST body, unparsed with the docstring node dropped, so the
guard reads only what executes.
Worth stating as the general shape, since this repo writes long explanatory
comments on purpose: any check that greps source for the absence of a pattern
is in tension with documenting why that pattern is gone. Either it excludes
the prose, or the next person to explain the fix breaks the guard that
protects it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR
This commit is contained in:
+20
-6
@@ -362,19 +362,33 @@ def test_the_agent_heartbeats_whether_or_not_it_holds_a_lease():
|
|||||||
|
|
||||||
Read from the source rather than by running the loop: it is a `while True`
|
Read from the source rather than by running the loop: it is a `while True`
|
||||||
with a sleep, so exercising it means threads and timing, and the property
|
with a sleep, so exercising it means threads and timing, and the property
|
||||||
is simply that the call is not behind a `if ids:`.
|
is simply that the call is not behind a guard on `ids`.
|
||||||
|
|
||||||
|
Parsed rather than grepped, and that is not fussiness — the first cut
|
||||||
|
searched the raw function text and failed on its own explanation. The
|
||||||
|
docstring of the fix quotes the construct the fix removed, because that is
|
||||||
|
what a docstring explaining a fix DOES. A source-text assertion cannot
|
||||||
|
tell the code from the prose about the code; the AST can, so the body is
|
||||||
|
unparsed with its docstring dropped.
|
||||||
"""
|
"""
|
||||||
|
import ast
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
src = (
|
src = (
|
||||||
Path(__file__).resolve().parents[1] / "agent" / "fc_agent" / "worker.py"
|
Path(__file__).resolve().parents[1] / "agent" / "fc_agent" / "worker.py"
|
||||||
).read_text()
|
).read_text()
|
||||||
loop = src[src.index("def _heartbeat_loop"):]
|
fn = next(
|
||||||
loop = loop[:loop.index("\n def ")]
|
n for n in ast.walk(ast.parse(src))
|
||||||
|
if isinstance(n, ast.FunctionDef) and n.name == "_heartbeat_loop"
|
||||||
|
)
|
||||||
|
code = "\n".join(
|
||||||
|
ast.unparse(n) for n in fn.body
|
||||||
|
if not (isinstance(n, ast.Expr) and isinstance(n.value, ast.Constant))
|
||||||
|
)
|
||||||
|
|
||||||
assert "self.client.heartbeat(ids)" in loop
|
assert "self.client.heartbeat(ids)" in code
|
||||||
assert "if ids:" not in loop, (
|
assert "if ids" not in code, (
|
||||||
"the heartbeat is gated on holding leases again; an idle agent then "
|
"the heartbeat is gated on holding leases again; an idle agent then "
|
||||||
"reads as stopped after 300s while sleep mode backs its lease poll "
|
"reads as stopped after 300s while sleep mode backs its lease poll "
|
||||||
"off to 900s"
|
f"off to 900s\n\n{code}"
|
||||||
)
|
)
|
||||||
|
|||||||
Reference in New Issue
Block a user