deer-flow/backend/tests/test_agent_guidance_check.py
spud 906c3d4554
fix(mcp): make durable task claims cancellation-safe (#4966)
* feat(mcp): re-scope to MCP task claim lifecycle only

Keep PR #4966 a small, closed MCP lease/cancellation state-machine change and
move RunJournal and Run lifecycle work into dedicated follow-ups. This branch
contains only the MCP task claim lifecycle:

- mcp task release/snapshot fencing by owner + per-claim lease token
- phase-level single-flight poll/cancel/notification owners with retained handoff
- routine cancellation no longer persisted as a task failure diagnostic
- bounded ordinary release ownership retention past the drain deadline
- 0018_mcp_task_lease_tokens migration + migration/bootstrap head assertions
- wait_for_task_until helper (MCP uses it); worker-specific capture helper moved
  to the run-finalization follow-up

RunJournal (journal.py + test_run_journal.py) and run lifecycle
(manager/worker/store/run sql + run tests) are preserved on
backup/cancellation-safety-full and will be raised as separate follow-ups.

* fix(mcp): unblock claims after ambiguous handoff resolves

A phase-level single-flight owner only guards an ambiguous claim outcome. Once
the claim resolves, the phase owner is released immediately; the handoff may
continue releasing returned rows as bounded, service-owned background work
(transferred to _compensation_tasks on timeout). Per-claim token fencing rejects
a late release against a newer claim generation, so a stuck release no longer
locks the whole phase until process restart.

- README: drop the stale progress-snapshot sentence from the bounded ordinary
  release description.
- service: pop the identity-checked phase owner as soon as the claim outcome is
  known, then release returned rows with the bounded path; carry the release in
  _compensation_tasks if it exceeds the drain deadline.
- mcp/AGENTS.md: document that only an unresolved claim outcome (not the handoff)
  blocks later phase scans, and that returned-row releases may continue in the
  background once the owner is released.
- tests: pin that the phase owner is released before a stuck release finishes
  while the release stays service strong-owned.

* refactor(mcp): remove unused single-record claim wrappers

_poll_one, _cancel_one, and _notify_one are unreachable in production: the
worker always processes claimed records through _run_claimed_batch, so these
wrappers preserved a second, dead single-record lifecycle (state is None)
whose only observable behavior was a wrapper-specific cancellation release.

Remove the three wrappers and migrate the regressions that guarded their
cancel/release invariants to exercise the production _run_claimed_batch path
(operation=_*_one_claimed, release=_release_*_after_cancellation). The single
wrapper-only "state is None" contract (test_poll_release_hang_without_batch)
is deleted; all 11 remaining invariants (CancelledError preservation, repeated
cancellation, poll-only token-fenced lease release, notification claimed vs
dispatched phase release, hung compensation -> service ownership, and
background compensation exactly-once observation) are now covered through the
real batch lifecycle.

* fix(mcp): fence claim-owned mutations against stale generations

The per-claim token check in the ORM release/apply paths was only in the
SELECT; the final write went out by primary key. On SQLite (where
with_for_update() is a no-op) a mutation from an older claim generation
could therefore clear a claim that a newer generation had reclaimed after lease
expiry — the exact distributed lease-fencing failure the per-claim token was
meant to prevent.

Make every claim-owned mutation a single atomic conditional UPDATE with the
owner and per-claim token in the WHERE clause (rowcount 0 => stale, return
False, no mutation):

- release_claim: atomic fence; record the poll-failure event after the fence
  wins (same transaction, holding the write lock).
- apply_snapshot / apply_cancel_snapshot: atomic fence; record the event after.
- finish_notification_run: atomic fence; use a CASE on event_version >>
  dispatch_version to keep a newer event pending for redelivery instead of
  swallowing it as delivered.

Add one regression per path: a stale generation's release/apply/finish after a
same-worker reclaim is rejected and never clears the newer claim.

* test(mcp): pin the migration chain head to the lease-token revision

0026_mcp_task_lease_tokens becomes the alembic head, so the chain-head pin in the 0025 repair test had to move on. Follow the 0023 precedent there (single head plus expected predecessor) instead of pinning a literal head, and give the new revision its own migration test, which owns the pin and covers the nullable claim-token columns on upgrade and their removal on downgrade.

* refactor(mcp): close cancellation cleanup leftovers

* fix(mcp): retain cancelled release diagnostics

* test(mcp): remove obsolete settled compensation case

* test(mcp): cover interleaved lease reclaim races

---------

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
2026-09-20 19:11:42 +08:00

198 lines
8.2 KiB
Python

from __future__ import annotations
import importlib.util
from pathlib import Path, PurePosixPath
import pytest
REPO_ROOT = Path(__file__).resolve().parents[2]
CHECKER_PATH = REPO_ROOT / "scripts" / "check_agent_guidance.py"
EXPECTED_GUIDANCE_PATHS = {
"AGENTS.md",
"backend/AGENTS.md",
"backend/tests/AGENTS.md",
"frontend/AGENTS.md",
"backend/app/gateway/AGENTS.md",
"backend/app/channels/AGENTS.md",
"backend/packages/harness/deerflow/AGENTS.md",
"backend/packages/harness/deerflow/agents/AGENTS.md",
"backend/packages/harness/deerflow/agents/middlewares/AGENTS.md",
"backend/packages/harness/deerflow/agents/memory/AGENTS.md",
"backend/packages/harness/deerflow/community/ragflow/AGENTS.md",
"backend/packages/harness/deerflow/community/tavily/AGENTS.md",
"backend/packages/harness/deerflow/config/AGENTS.md",
"backend/packages/harness/deerflow/extensions/AGENTS.md",
"backend/packages/harness/deerflow/runtime/AGENTS.md",
"backend/packages/harness/deerflow/sandbox/AGENTS.md",
"backend/packages/harness/deerflow/mcp/AGENTS.md",
"backend/packages/harness/deerflow/models/AGENTS.md",
"backend/packages/harness/deerflow/persistence/migrations/AGENTS.md",
"backend/packages/harness/deerflow/persistence/user/AGENTS.md",
"backend/packages/harness/deerflow/reflection/AGENTS.md",
"backend/packages/harness/deerflow/skills/AGENTS.md",
"backend/packages/harness/deerflow/subagents/AGENTS.md",
"backend/packages/harness/deerflow/tools/AGENTS.md",
"backend/packages/harness/deerflow/tracing/AGENTS.md",
"backend/packages/harness/deerflow/tui/AGENTS.md",
"backend/packages/harness/deerflow/utils/AGENTS.md",
"frontend/src/AGENTS.md",
"scripts/AGENTS.md",
}
def _load_checker():
assert CHECKER_PATH.exists(), f"{CHECKER_PATH} must exist"
spec = importlib.util.spec_from_file_location("deerflow_agent_guidance_check", CHECKER_PATH)
assert spec is not None
assert spec.loader is not None
module = importlib.util.module_from_spec(spec)
spec.loader.exec_module(module)
return module
checker = _load_checker()
def _files(**overrides: str) -> dict[PurePosixPath, str]:
files = {PurePosixPath("AGENTS.md"): "# Repository instructions\n"}
files.update({PurePosixPath(path): text for path, text in overrides.items()})
return files
def _codes(findings, severity: str | None = None) -> list[str]:
return [finding.code for finding in findings if severity is None or finding.severity == severity]
def test_normalized_utf8_size_uses_lf_and_counts_non_ascii_bytes() -> None:
assert checker.normalized_utf8_size("a\r\n中\r") == len("a\n中\n".encode())
@pytest.mark.parametrize(
("path", "soft", "hard"),
[
("AGENTS.md", 16 * 1024, 20 * 1024),
("backend/AGENTS.md", 28 * 1024, 32 * 1024),
("backend/app/gateway/AGENTS.md", 40 * 1024, 48 * 1024),
],
)
def test_budget_depends_on_directory_level(path: str, soft: int, hard: int) -> None:
assert checker.agent_budget(PurePosixPath(path)) == (soft, hard)
def test_single_file_over_hard_limit_is_an_error(tmp_path: Path) -> None:
findings = checker.analyze(tmp_path, _files(**{"AGENTS.md": "x" * (20 * 1024 + 1)}))
assert "AG001" in _codes(findings, "error")
def test_effective_ancestor_chain_can_fail_when_each_file_is_valid(tmp_path: Path) -> None:
files = _files(
**{
"AGENTS.md": "r" * (15 * 1024),
"backend/AGENTS.md": "m" * (31 * 1024),
"backend/app/AGENTS.md": "l" * (47 * 1024),
"backend/app/gateway/AGENTS.md": "g" * (47 * 1024),
}
)
findings = checker.analyze(tmp_path, files)
assert "AG001" not in _codes(findings, "error")
assert "AG002" in _codes(findings, "error")
def test_legacy_hard_violation_may_shrink_but_may_not_grow(tmp_path: Path) -> None:
base = _files(**{"AGENTS.md": "x" * (20 * 1024)})
smaller = _files(**{"AGENTS.md": "x" * (20 * 1024 - 1)})
grown = _files(**{"AGENTS.md": "x" * (20 * 1024 + 1)})
smaller_findings = checker.analyze(tmp_path, smaller, base_files=base)
grown_findings = checker.analyze(tmp_path, grown, base_files=base)
assert "AG001" not in _codes(smaller_findings, "error")
assert "AG001" in _codes(grown_findings, "error")
def test_discovery_uses_exact_agents_basename() -> None:
paths = [
PurePosixPath("AGENTS.md"),
PurePosixPath("backend/AGENTS.md"),
PurePosixPath("backend/CLAUDE.md"),
PurePosixPath("backend/docs/GITHUB_AGENTS.md"),
]
assert checker.guidance_paths(paths) == {
PurePosixPath("AGENTS.md"),
PurePosixPath("backend/AGENTS.md"),
}
def test_repository_has_the_approved_scoped_guidance_shape() -> None:
actual = {path.as_posix() for path in checker.guidance_paths(checker._worktree_paths(REPO_ROOT))}
assert actual == EXPECTED_GUIDANCE_PATHS
def test_repository_guidance_stays_below_hard_budgets_and_avoids_doc_indexes() -> None:
for relative_text in EXPECTED_GUIDANCE_PATHS:
relative = PurePosixPath(relative_text)
path = REPO_ROOT / relative_text
assert path.is_file(), relative
_, hard = checker.agent_budget(relative)
text = path.read_text(encoding="utf-8")
assert checker.normalized_utf8_size(text) <= hard, relative
assert "Subsystem Index" not in text
def test_local_guidance_files_contain_the_split_original_sections() -> None:
expected_headings = {
"backend/app/gateway/AGENTS.md": "### Gateway API (`app/gateway/`)",
"backend/app/channels/AGENTS.md": "### IM Channels System (`app/channels/`)",
"backend/packages/harness/deerflow/agents/AGENTS.md": "### Agent System",
"backend/packages/harness/deerflow/agents/middlewares/AGENTS.md": "### Middleware Chain",
"backend/packages/harness/deerflow/agents/memory/AGENTS.md": "### Memory System",
"backend/packages/harness/deerflow/config/AGENTS.md": "### Configuration System",
"backend/packages/harness/deerflow/extensions/AGENTS.md": "### Python Extension System",
"backend/packages/harness/deerflow/runtime/AGENTS.md": "### Checkpoint Channel Modes",
"backend/packages/harness/deerflow/sandbox/AGENTS.md": "### Sandbox System",
"backend/packages/harness/deerflow/mcp/AGENTS.md": "### MCP System",
"backend/packages/harness/deerflow/models/AGENTS.md": "### Model Factory",
"backend/packages/harness/deerflow/persistence/migrations/AGENTS.md": "### Schema Migrations",
"backend/packages/harness/deerflow/reflection/AGENTS.md": "### Reflection System",
"backend/packages/harness/deerflow/skills/AGENTS.md": "### Skills System",
"backend/packages/harness/deerflow/subagents/AGENTS.md": "### Subagent System",
"backend/packages/harness/deerflow/tools/AGENTS.md": "### Tool System",
"backend/packages/harness/deerflow/tracing/AGENTS.md": "### Tracing System",
"backend/packages/harness/deerflow/tui/AGENTS.md": "### Terminal Workbench / TUI",
"frontend/src/AGENTS.md": "### Data Flow",
}
for relative_text, heading in expected_headings.items():
text = (REPO_ROOT / relative_text).read_text(encoding="utf-8")
assert heading in text, relative_text
assert "Before changing files in this directory" not in text, relative_text
def test_mcp_task_lease_token_migration_is_documented() -> None:
guidance = (REPO_ROOT / "backend" / "packages" / "harness" / "deerflow" / "persistence" / "migrations" / "AGENTS.md").read_text(encoding="utf-8")
for required in (
"0026_mcp_task_lease_tokens.py",
"0016_subagent_batches",
"lease_token",
"notification_lease_token",
):
assert required in guidance
def test_repository_exposes_one_local_and_one_ci_entrypoint() -> None:
makefile = (REPO_ROOT / "Makefile").read_text(encoding="utf-8")
workflow = (REPO_ROOT / ".github" / "workflows" / "lint-check.yml").read_text(encoding="utf-8")
assert "check-agent-guidance:" in makefile
assert "scripts/check_agent_guidance.py" in makefile
assert workflow.count("agent-guidance:") == 1
assert "fetch-depth: 0" in workflow
assert "--base-ref" in workflow
assert "--before" in workflow