diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 2fa8f6cd..04519712 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -12,7 +12,7 @@ "name": "PACT", "source": "./pact-plugin", "description": "Orchestration harness that turns Claude Code into a coordinated team of specialist AI agents", - "version": "4.0.0", + "version": "4.0.1", "author": { "name": "Synaptic-Labs-AI" }, diff --git a/README.md b/README.md index ed6897de..5670220a 100644 --- a/README.md +++ b/README.md @@ -504,7 +504,7 @@ When installed as a plugin, PACT lives in your plugin cache: │ └── cache/ │ └── pact-plugin/ │ └── PACT/ -│ └── 4.0.0/ # Plugin version +│ └── 4.0.1/ # Plugin version │ ├── agents/ │ ├── commands/ │ ├── skills/ diff --git a/pact-plugin/.claude-plugin/plugin.json b/pact-plugin/.claude-plugin/plugin.json index 75e06345..5f8a7dcc 100644 --- a/pact-plugin/.claude-plugin/plugin.json +++ b/pact-plugin/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "PACT", - "version": "4.0.0", + "version": "4.0.1", "description": "Orchestration harness that turns Claude Code into a coordinated team of specialist AI agents", "author": { "name": "Synaptic-Labs-AI", diff --git a/pact-plugin/README.md b/pact-plugin/README.md index ea96a26e..3f055a1c 100644 --- a/pact-plugin/README.md +++ b/pact-plugin/README.md @@ -1,6 +1,6 @@ # PACT — Orchestration Harness for Claude Code -> **Version**: 4.0.0 +> **Version**: 4.0.1 Turn a single Claude Code session into a managed team of specialist AI agents that prepare, design, build, and test your code systematically. diff --git a/pact-plugin/hooks/wake_lifecycle_emitter.py b/pact-plugin/hooks/wake_lifecycle_emitter.py index ce5dec5c..f9126eb8 100644 --- a/pact-plugin/hooks/wake_lifecycle_emitter.py +++ b/pact-plugin/hooks/wake_lifecycle_emitter.py @@ -147,21 +147,60 @@ def _extract_task_id(input_data: dict[str, Any]) -> str | None: Pull the task_id out of the PostToolUse payload. PostToolUse stdin shape carries the original tool_input under - "tool_input" and the tool's response under "tool_response". Both - TaskCreate and TaskUpdate accept/return a task with an `id` field. - Defensively probe both paths; return None if neither yields a - string id. + "tool_input" and the tool's response under "tool_response". + TaskCreate's tool_response is nested — the created task is wrapped + under a "task" key (`tool_response.task.id`) — while TaskUpdate's + tool_response is flat (`tool_response.id`). Probe in precedence + order, returning the first match whose value is a string that is + non-empty after `.strip()`: + + 1. tool_input.taskId + 2. tool_input.task_id + 3. tool_response.task.id + 4. tool_response.task.taskId + 5. tool_response.task.task_id + 6. tool_response.id + 7. tool_response.taskId + 8. tool_response.task_id + + WHY the nested `tool_response.task.*` probes precede the flat + `tool_response.*` probes: production-typical TaskCreate payloads + are nested (per #612 logging-shim capture from session + pact-56ce3a2a on 2026-05-02). Placing nested probes first means + the production-common case hits the first matching probe; the + flat probes remain as fallback for TaskUpdate and for legacy/test + fixture shapes. + + Returned values are guaranteed non-empty after strip — a + whitespace-only id (e.g. `" "`) would propagate to a TaskStop + call with a syntactically-valid-but-semantically-empty id and + fail downstream; rejecting at the source is cheaper. Returns + None if no probe matches. """ tool_input = input_data.get("tool_input") or {} if isinstance(tool_input, dict): tid = tool_input.get("taskId") or tool_input.get("task_id") - if isinstance(tid, str) and tid: + if isinstance(tid, str) and tid.strip(): return tid tool_response = input_data.get("tool_response") or {} if isinstance(tool_response, dict): - tid = tool_response.get("id") or tool_response.get("taskId") or tool_response.get("task_id") - if isinstance(tid, str) and tid: + nested_task = tool_response.get("task") or {} + if isinstance(nested_task, dict): + tid = ( + nested_task.get("id") + or nested_task.get("taskId") + or nested_task.get("task_id") + ) + if isinstance(tid, str) and tid.strip(): + return tid + + tid = ( + tool_response.get("id") + or tool_response.get("taskId") + or tool_response.get("task_id") + ) + if isinstance(tid, str) and tid.strip(): return tid return None diff --git a/pact-plugin/tests/fixtures/wake_lifecycle/README.md b/pact-plugin/tests/fixtures/wake_lifecycle/README.md new file mode 100644 index 00000000..a8450bd6 --- /dev/null +++ b/pact-plugin/tests/fixtures/wake_lifecycle/README.md @@ -0,0 +1,70 @@ +# wake_lifecycle hook stdin fixtures + +Captured `PostToolUse` stdin payloads for the +`pact-plugin/hooks/wake_lifecycle_emitter.py` hook. These fixtures fossilize +the **production** shape of `tool_response` for `TaskCreate` and `TaskUpdate` +so tests cannot silently drift away from what the platform actually delivers. + +Background: between PR #603 (regression introduction) and #620 (fix), every +test in `test_inbox_wake_lifecycle_emitter.py` used a hand-constructed flat +`tool_response: {"id": "..."}` payload. Production `TaskCreate` `tool_response` +is **nested** (`tool_response.task.id`) per #612's logging-shim capture from +session `pact-56ce3a2a` on 2026-05-02. The hook silently returned `None` on +every TaskCreate while tests stayed green. This directory exists to make that +class of failure structurally impossible going forward. + +## Capture-provenance convention (MANDATORY) + +Every fixture in this directory MUST be a JSON object with a sibling +top-level `_meta` key documenting where the payload came from: + +```json +{ + "_meta": { + "capture_session_id": "pact-56ce3a2a", + "capture_date": "2026-05-02", + "capture_method": "logging-shim", + "issue_ref": "#612" + }, + "tool_name": "TaskCreate", + "tool_input": { "...": "..." }, + "tool_response": { "task": { "id": "...", "...": "..." } } +} +``` + +`_meta` is a sibling top-level key. It is NOT nested inside `tool_input` or +`tool_response`. Tests read it for diagnostic context and ignore it when +piping the payload through the hook (the hook itself ignores unknown +top-level keys). + +### `_meta` fields + +| Field | Required | Purpose | +| --------------------- | -------- | -------------------------------------------------------------------------- | +| `capture_session_id` | Yes | PACT session ID where the payload was captured (e.g., `pact-56ce3a2a`). | +| `capture_date` | Yes | ISO-8601 date of capture (e.g., `2026-05-02`). | +| `capture_method` | Yes | How it was captured: `logging-shim`, `manual-stdin-redirect`, or `legacy`. | +| `issue_ref` | Yes | Issue or PR that justifies preserving this fixture (e.g., `#612`). | +| `notes` | No | Free-form notes (e.g., "preserved as regression backstop"). | + +### `capture_method` values + +- `logging-shim` — payload was captured by an in-hook stdin logger writing + the raw stdin bytes to a side-channel file. Highest fidelity; preferred for + any new fixture covering platform-shape behavior. +- `manual-stdin-redirect` — payload was captured by tee-ing the hook's + stdin into a file during a real PACT session. Equivalent fidelity to + logging-shim; noted separately for traceability. +- `legacy` — payload predates the convention and was hand-constructed. + Permitted ONLY for backward-compat regression backstops (i.e., tests that + intentionally assert behavior on the broken pre-fix shape). Never use + `legacy` for new shape-resilience fixtures. + +## Future hooks + +This convention applies to **all future hook-stdin fixtures**, not just +wake_lifecycle. When adding fixtures for another hook (e.g., the +`peer_inject` SubagentStart payload referenced by the audit-test addendum +on PR B / #628), create a sibling subdirectory with its own README and +mirror this convention. The provenance-capture discipline IS the structural +defense against the failure class that #620 surfaced. diff --git a/pact-plugin/tests/fixtures/wake_lifecycle/task_create_legacy_fixture_shape.json b/pact-plugin/tests/fixtures/wake_lifecycle/task_create_legacy_fixture_shape.json new file mode 100644 index 00000000..200da5b6 --- /dev/null +++ b/pact-plugin/tests/fixtures/wake_lifecycle/task_create_legacy_fixture_shape.json @@ -0,0 +1,18 @@ +{ + "_meta": { + "capture_session_id": "n/a", + "capture_date": "pre-2026-05-02", + "capture_method": "legacy", + "issue_ref": "#620", + "notes": "Legacy hand-constructed TaskCreate fixture preserved for regression backstop. This is the FLAT tool_response.id shape that test fixtures used between PR #603 and #620; it does NOT match production. Kept here so a test can assert the function still extracts a task_id from this shape (the fix is additive — flat fallback remains, nested probe is added ahead of it). DO NOT use this shape for new tests; new tests MUST use task_create_production_shape.json." + }, + "tool_name": "TaskCreate", + "session_id": "synthetic-legacy", + "cwd": "/tmp/proj", + "tool_input": { + "taskId": "5" + }, + "tool_response": { + "id": "5" + } +} diff --git a/pact-plugin/tests/fixtures/wake_lifecycle/task_create_production_shape.json b/pact-plugin/tests/fixtures/wake_lifecycle/task_create_production_shape.json new file mode 100644 index 00000000..2ecf7ef6 --- /dev/null +++ b/pact-plugin/tests/fixtures/wake_lifecycle/task_create_production_shape.json @@ -0,0 +1,27 @@ +{ + "_meta": { + "capture_session_id": "pact-56ce3a2a", + "capture_date": "2026-05-02", + "capture_method": "logging-shim", + "issue_ref": "#612", + "notes": "Captured TaskCreate PostToolUse stdin showing the nested tool_response.task.id production shape. This is the shape every TaskCreate has carried since PR #603, but tests prior to #620 used the legacy flat shape (see task_create_legacy_fixture_shape.json) and the divergence stayed undetected because _extract_task_id silently returned None." + }, + "tool_name": "TaskCreate", + "session_id": "pact-56ce3a2a", + "cwd": "/Users/mj/Sites/collab/PACT-prompt", + "tool_input": { + "subject": "First active teammate task", + "description": "Initial work item dispatched to a teammate.", + "activeForm": "Working on first item" + }, + "tool_response": { + "task": { + "id": "5", + "subject": "First active teammate task", + "status": "pending", + "owner": "backend-coder", + "blockedBy": [], + "blocks": [] + } + } +} diff --git a/pact-plugin/tests/fixtures/wake_lifecycle/task_update_production_shape.json b/pact-plugin/tests/fixtures/wake_lifecycle/task_update_production_shape.json new file mode 100644 index 00000000..29e580bd --- /dev/null +++ b/pact-plugin/tests/fixtures/wake_lifecycle/task_update_production_shape.json @@ -0,0 +1,22 @@ +{ + "_meta": { + "capture_session_id": "pact-56ce3a2a", + "capture_date": "2026-05-02", + "capture_method": "logging-shim", + "issue_ref": "#612", + "notes": "Captured TaskUpdate PostToolUse stdin showing the FLAT tool_response.id shape. TaskUpdate is currently flat in production (unlike TaskCreate which is nested). Fossilized here so a future platform shape change is caught by the parametrized shape-resilience test rather than silently regressing. The hook's flat-shape probe MUST keep working for this payload." + }, + "tool_name": "TaskUpdate", + "session_id": "pact-56ce3a2a", + "cwd": "/Users/mj/Sites/collab/PACT-prompt", + "tool_input": { + "taskId": "5", + "status": "completed" + }, + "tool_response": { + "id": "5", + "subject": "First active teammate task", + "status": "completed", + "owner": "backend-coder" + } +} diff --git a/pact-plugin/tests/test_inbox_wake_lifecycle_emitter.py b/pact-plugin/tests/test_inbox_wake_lifecycle_emitter.py index 36464eb2..f63d6a63 100644 --- a/pact-plugin/tests/test_inbox_wake_lifecycle_emitter.py +++ b/pact-plugin/tests/test_inbox_wake_lifecycle_emitter.py @@ -43,7 +43,7 @@ def _run_emitter(stdin_payload: str | bytes, env_extra: dict | None = None) -> t return proc.returncode, proc.stdout.decode("utf-8"), proc.stderr.decode("utf-8") -def _pact_session_env(tmp_path: Path, team_name: str) -> dict: +def _pact_session_env(tmp_path: Path) -> dict: """ Build env vars + on-disk pact-session-context so the emitter's pact_context.init() resolves the team_name from the synthesized @@ -114,13 +114,13 @@ def test_hooks_json_registers_emitter_under_post_tool_use(): # ---------- Fail-open paths ---------- def test_malformed_stdin_exits_zero_with_suppress(tmp_path): - rc, out, _ = _run_emitter(b"\x00not-json\xff", env_extra=_pact_session_env(tmp_path, "t")) + rc, out, _ = _run_emitter(b"\x00not-json\xff", env_extra=_pact_session_env(tmp_path)) assert rc == 0 assert json.loads(out) == {"suppressOutput": True} def test_non_dict_stdin_exits_zero_with_suppress(tmp_path): - rc, out, _ = _run_emitter("[]", env_extra=_pact_session_env(tmp_path, "t")) + rc, out, _ = _run_emitter("[]", env_extra=_pact_session_env(tmp_path)) assert rc == 0 assert json.loads(out) == {"suppressOutput": True} @@ -132,9 +132,9 @@ def test_missing_team_name_exits_zero_with_suppress(tmp_path): "session_id": "abc", "cwd": "/tmp/x", "tool_input": {"taskId": "1"}, - "tool_response": {"id": "1"}, + "tool_response": {"task": {"id": "1"}}, }) - rc, out, _ = _run_emitter(payload, env_extra=_pact_session_env(tmp_path, "t")) + rc, out, _ = _run_emitter(payload, env_extra=_pact_session_env(tmp_path)) assert rc == 0 assert json.loads(out) == {"suppressOutput": True} @@ -198,7 +198,7 @@ def test_arm_emitted_on_first_task_create(tmp_path): "session_id": sid, "cwd": pdir, "tool_input": {"taskId": "task-1"}, - "tool_response": {"id": "task-1"}, + "tool_response": {"task": {"id": "task-1"}}, } out = _emit_output(payload, home) hso = out["hookSpecificOutput"] @@ -213,7 +213,7 @@ def test_arm_includes_idempotency_clause(tmp_path): _write_task(home, team, "1", status="pending", owner="x") out = _emit_output({ "tool_name": "TaskCreate", "session_id": sid, "cwd": pdir, - "tool_input": {"taskId": "1"}, "tool_response": {"id": "1"}, + "tool_input": {"taskId": "1"}, "tool_response": {"task": {"id": "1"}}, }, home) additional = out["hookSpecificOutput"]["additionalContext"] # Case-insensitive — directive prose capitalizes 'Idempotent' but @@ -251,7 +251,7 @@ def test_arm_directive_contains_precondition_phrase(tmp_path): _write_task(home, team, "1", status="pending", owner="x") out = _emit_output({ "tool_name": "TaskCreate", "session_id": sid, "cwd": pdir, - "tool_input": {"taskId": "1"}, "tool_response": {"id": "1"}, + "tool_input": {"taskId": "1"}, "tool_response": {"task": {"id": "1"}}, }, home) assert "First active teammate task created" in out["hookSpecificOutput"]["additionalContext"] @@ -279,7 +279,7 @@ def test_no_op_on_second_active_task_create(tmp_path): _write_task(home, team, "new", status="in_progress", owner="y") out = _emit_output({ "tool_name": "TaskCreate", "session_id": sid, "cwd": pdir, - "tool_input": {"taskId": "new"}, "tool_response": {"id": "new"}, + "tool_input": {"taskId": "new"}, "tool_response": {"task": {"id": "new"}}, }, home) assert out == {"suppressOutput": True} @@ -297,7 +297,7 @@ def test_no_op_on_create_of_signal_task(tmp_path): ) out = _emit_output({ "tool_name": "TaskCreate", "session_id": sid, "cwd": pdir, - "tool_input": {"taskId": "sig-1"}, "tool_response": {"id": "sig-1"}, + "tool_input": {"taskId": "sig-1"}, "tool_response": {"task": {"id": "sig-1"}}, }, home) assert out == {"suppressOutput": True} @@ -309,7 +309,7 @@ def test_no_op_on_create_owned_by_exempt_agent(tmp_path): _write_task(home, team, "sec-1", status="in_progress", owner="secretary") out = _emit_output({ "tool_name": "TaskCreate", "session_id": sid, "cwd": pdir, - "tool_input": {"taskId": "sec-1"}, "tool_response": {"id": "sec-1"}, + "tool_input": {"taskId": "sec-1"}, "tool_response": {"task": {"id": "sec-1"}}, }, home) assert out == {"suppressOutput": True} @@ -433,7 +433,7 @@ def test_teardown_emitted_on_status_deleted_at_post_zero(tmp_path): assert "Skill(\"PACT:unwatch-inbox\")" in hso["additionalContext"] -def test_is_terminal_status_update_matches_completed_and_deleted(tmp_path): +def test_is_terminal_status_update_matches_completed_and_deleted(): """Direct unit test on the terminal-status predicate. The behavioral contract is "task transitioned to a terminal status" — both `completed` and `deleted` are terminal.""" @@ -487,7 +487,7 @@ def test_no_emit_when_session_id_does_not_match_lead(tmp_path): "session_id": teammate_sid, "cwd": pdir, "tool_input": {"taskId": "task-x"}, - "tool_response": {"id": "task-x"}, + "tool_response": {"task": {"id": "task-x"}}, } out = _emit_output(payload, home) assert out == {"suppressOutput": True}, ( @@ -514,7 +514,7 @@ def test_no_emit_when_team_config_missing(tmp_path): _write_task(home, team, "task-x", status="in_progress", owner="x") payload = { "tool_name": "TaskCreate", "session_id": sid, "cwd": pdir, - "tool_input": {"taskId": "task-x"}, "tool_response": {"id": "task-x"}, + "tool_input": {"taskId": "task-x"}, "tool_response": {"task": {"id": "task-x"}}, } out = _emit_output(payload, home) assert out == {"suppressOutput": True} @@ -579,7 +579,7 @@ def test_count_active_tasks_called_on_terminal_status_taskupdate(): from unittest.mock import patch with patch.object(emitter, "_is_lead_session", return_value=True), \ patch.object(emitter, "count_active_tasks", return_value=0) as mock_count: - result = emitter._decide_directive({ + emitter._decide_directive({ "tool_name": "TaskUpdate", "session_id": "sid", "cwd": "/tmp/p", "tool_input": {"taskId": "1", "status": "completed"}, @@ -600,11 +600,11 @@ def test_count_active_tasks_called_on_taskcreate(): with patch.object(emitter, "_is_lead_session", return_value=True), \ patch.object(emitter, "_extract_task_id", return_value="1"), \ patch.object(emitter, "count_active_tasks", return_value=1) as mock_count: - result = emitter._decide_directive({ + emitter._decide_directive({ "tool_name": "TaskCreate", "session_id": "sid", "cwd": "/tmp/p", "tool_input": {"taskId": "1"}, - "tool_response": {"id": "1"}, + "tool_response": {"task": {"id": "1"}}, }, "team-x") assert mock_count.call_count >= 1 @@ -640,7 +640,7 @@ def test_oversized_stdin_payload_fails_open_with_suppress(tmp_path): "session_id": sid, "cwd": pdir, "tool_input": {"taskId": "task-cap", "filler": filler}, - "tool_response": {"id": "task-cap"}, + "tool_response": {"task": {"id": "task-cap"}}, } payload_bytes = json.dumps(payload_dict).encode("utf-8") assert len(payload_bytes) > 1024 * 1024, ( @@ -671,3 +671,197 @@ def test_emitter_documents_payload_size_cap_constant(): import wake_lifecycle_emitter as emitter assert isinstance(emitter._MAX_PAYLOAD_BYTES, int) assert 64 * 1024 <= emitter._MAX_PAYLOAD_BYTES <= 16 * 1024 * 1024 + + +# ---------- Shape-resilience for _extract_task_id (#620) ---------- + +FIXTURES_DIR = Path(__file__).resolve().parent / "fixtures" / "wake_lifecycle" + +_REQUIRED_META_FIELDS = { + "capture_session_id", + "capture_date", + "capture_method", + "issue_ref", +} + + +def test_all_wake_lifecycle_fixtures_carry_meta_provenance(): + """Convention enforcement for `tests/fixtures/wake_lifecycle/`: every + JSON fixture MUST have a top-level `_meta` dict carrying the four + required provenance fields. The README in that directory documents + the convention; this test enforces it so a future contributor cannot + silently add an un-provenanced fixture and weaken the structural + defense against the #612-class shape-divergence regression. + + To extend this enforcement to a sibling fixture subdirectory (e.g., + a future `tests/fixtures/peer_inject/`), add a new test function + that points at the new dir, or refactor this function to parametrize + over a list of provenance-required fixture roots. + """ + fixture_paths = sorted(FIXTURES_DIR.glob("*.json")) + assert fixture_paths, ( + f"No JSON fixtures found in {FIXTURES_DIR}; convention is moot " + f"if the directory is empty — verify the test is pointed at the " + f"right path." + ) + for fixture_path in fixture_paths: + data = json.loads(fixture_path.read_text(encoding="utf-8")) + assert "_meta" in data, ( + f"{fixture_path.name}: missing top-level `_meta` sibling key. " + f"See {FIXTURES_DIR.name}/README.md for the convention." + ) + meta = data["_meta"] + assert isinstance(meta, dict), ( + f"{fixture_path.name}: `_meta` must be a dict, got {type(meta).__name__}" + ) + missing = _REQUIRED_META_FIELDS - set(meta.keys()) + assert not missing, ( + f"{fixture_path.name}: `_meta` missing required fields: {missing}. " + f"Required: {_REQUIRED_META_FIELDS}." + ) + + +class TestExtractTaskIdShapeResilience: + """Pin _extract_task_id behavior across every shape it must handle. + + Production `TaskCreate` `tool_response` is **nested** + (`tool_response.task.id`) per #612's logging-shim capture; production + `TaskUpdate` `tool_response` is **flat** (`tool_response.id`). The + regression in #620 was that the function only probed the flat shape, + so every TaskCreate returned None and the auto-Arm path was dead. + + This class fossilizes the precedence + shape-resilience contract. + Test #1 is the counter-test-by-revert for the #620 fix: reverting + the nested-task probe makes it fail. + """ + + @staticmethod + def _extract(input_data): + sys.path.insert(0, str(HOOK_DIR)) + import wake_lifecycle_emitter as emitter + return emitter._extract_task_id(input_data) + + def test_taskcreate_production_nested_task_shape(self): + """The #620 regression test. Pipes the production TaskCreate + shape (`tool_response.task.id`) and asserts the id is extracted. + Counter-test-by-revert: revert the nested-task probe and this + fails — the function returns None, replicating the bug.""" + result = self._extract({"tool_response": {"task": {"id": "5"}}}) + assert result == "5" + + def test_taskupdate_production_flat_shape(self): + """Fossilizes the working TaskUpdate shape. The flat fallback + must keep working alongside the new nested probe.""" + result = self._extract({"tool_response": {"id": "5"}}) + assert result == "5" + + def test_tool_input_taskid_priority(self): + """When both `tool_input.taskId` and a tool_response id are + present, `tool_input` wins. Pins the precedence so a future + reorder breaks this test rather than silently inverting.""" + result = self._extract({ + "tool_input": {"taskId": "from-input"}, + "tool_response": {"task": {"id": "from-response"}}, + }) + assert result == "from-input" + + def test_unknown_shape_returns_none(self): + """Fail-open on unknown shape: an unrecognized `tool_response` + sub-key returns None, allowing the caller to suppressOutput + cleanly without crashing.""" + result = self._extract( + {"tool_response": {"unexpected_key": {"id": "lost"}}} + ) + assert result is None + + @pytest.mark.parametrize( + "payload", + [ + {"tool_input": {}, "tool_response": {}}, + {}, + ], + ids=["both-empty-dicts", "fully-empty-input"], + ) + def test_empty_dicts_return_none(self, payload): + """No id anywhere → None. Covers both the empty-sub-dicts and + the fully-empty-input shapes.""" + assert self._extract(payload) is None + + @pytest.mark.parametrize( + "bad_id", + [5, None, ["x"], {"nested": "value"}, True], + ids=["int", "none", "list", "dict", "bool"], + ) + def test_non_string_id_returns_none(self, bad_id): + """Only string ids are accepted. Pins the type discipline so + a future relaxation (e.g., `str(tid)` coercion) breaks loudly. + + Probes both the nested and the flat path so a non-string id + in either position is rejected.""" + # Nested path + assert self._extract({"tool_response": {"task": {"id": bad_id}}}) is None + # Flat path + assert self._extract({"tool_response": {"id": bad_id}}) is None + + @pytest.mark.parametrize( + "whitespace_id", + [" ", "\t", "\n", " \t\n ", " "], + ids=["spaces", "tab", "newline", "mixed-whitespace", "nbsp"], + ) + def test_whitespace_only_id_returns_none(self, whitespace_id): + """Adversarial: a whitespace-only id is a string and truthy + (passes `isinstance(tid, str) and tid`), but downstream + `count_active_tasks` would silently fail to find a task by + whitespace id — masking the real failure mode. The hook's + `.strip()` handling rejects whitespace-only ids upfront so the + function returns None and `_decide_directive` exits cleanly. + + Counter-test-by-revert: removing the `.strip()` handling makes + this test fail (the function returns the whitespace string). + Probes both the nested and the flat path so the discipline + applies symmetrically. + """ + # Nested path + assert self._extract({"tool_response": {"task": {"id": whitespace_id}}}) is None + # Flat path + assert self._extract({"tool_response": {"id": whitespace_id}}) is None + + +def test_arm_emitted_on_captured_production_taskcreate_payload(tmp_path): + """End-to-end #620 regression: pipe the captured production + TaskCreate stdin (from `fixtures/wake_lifecycle/task_create_production_shape.json`) + through the full hook entry-point and assert an Arm directive is + emitted. Counter-test-by-revert: revert the nested-task probe and + `_extract_task_id` returns None on this payload → the + `if not _extract_task_id(...)` guard exits → no Arm emit → this + test fails. The hand-crafted unit test + `test_taskcreate_production_nested_task_shape` covers the same + failure mode at the function level; this test additionally + exercises the full subprocess pipe so a regression in the hook's + main() wiring (e.g., re-introducing a flat-only probe somewhere + downstream) is also caught.""" + fixture = json.loads( + (FIXTURES_DIR / "task_create_production_shape.json").read_text(encoding="utf-8") + ) + # Strip the diagnostic _meta sibling; the hook would tolerate it, + # but pipe a clean payload to mirror what the platform actually + # sends. + fixture.pop("_meta", None) + + home = tmp_path / "home"; home.mkdir() + sid = fixture["session_id"] + pdir = fixture["cwd"] + team = "team-prod" + _write_session_context(home, sid, pdir, team) + task_id = fixture["tool_response"]["task"]["id"] + _write_task(home, team, task_id, status="pending", owner="backend-coder") + + out = _emit_output(fixture, home) + hso = out.get("hookSpecificOutput") + assert hso is not None, ( + f"Expected Arm directive on captured production TaskCreate; " + f"got {out!r}. If `out == {{'suppressOutput': True}}`, the " + f"nested-task probe in _extract_task_id is missing — see #620." + ) + assert hso["hookEventName"] == "PostToolUse" + assert "Skill(\"PACT:watch-inbox\")" in hso["additionalContext"] diff --git a/pact-plugin/tests/test_plugin_json_orchestrator.py b/pact-plugin/tests/test_plugin_json_orchestrator.py index 4113c15b..2320379d 100644 --- a/pact-plugin/tests/test_plugin_json_orchestrator.py +++ b/pact-plugin/tests/test_plugin_json_orchestrator.py @@ -1,14 +1,11 @@ """ -plugin.json registers pact-orchestrator agent (13-entry list under v4.0.0). +plugin.json structural invariants for the PACT plugin. -C1 adds `./agents/pact-orchestrator.md` to the `agents` array; C9 drops -`./commands/bootstrap.md` and `./commands/teammate-bootstrap.md`; C11 bumps -the version to 4.0.0. These tests assert all three are landed. - -Marker discipline (C2): tests against production already on disk (C1: 13-entry -agents array including pact-orchestrator) are plain tests. Tests dependent on -C9 (drop bootstrap commands) and C11 (version bump) carry xfail-strict and flip -in C10 as their dependent commits land. +Pins the 13-entry alphabetized `agents` array (12 teammates + orchestrator) +and the absence of the removed bootstrap commands (`bootstrap.md` and +`teammate-bootstrap.md`) which are no longer registered now that the +orchestrator persona is delivered via the `--agent` flag. Cross-file +version-consistency is owned by sibling test_plugin_version_bump.py. """ import json from pathlib import Path @@ -47,12 +44,6 @@ def plugin_json(): return json.loads(PLUGIN_JSON_PATH.read_text()) -def test_plugin_json_version_is_4_0_0(plugin_json): - assert plugin_json["version"] == "4.0.0", ( - f"plugin.json version should be 4.0.0 (BREAKING), got {plugin_json['version']}" - ) - - def test_plugin_json_has_13_agents(plugin_json): agents = plugin_json.get("agents", []) assert len(agents) == 13, ( @@ -86,7 +77,7 @@ def test_plugin_json_agents_alphabetized(plugin_json): def test_plugin_json_drops_bootstrap_commands(plugin_json): - """C9 removes bootstrap commands — replaced by --agent flag.""" + """Bootstrap commands are not registered; orchestrator persona is delivered via --agent.""" commands = set(plugin_json.get("commands", [])) leaked = REMOVED_COMMANDS & commands assert not leaked, ( diff --git a/pact-plugin/tests/test_plugin_version_bump.py b/pact-plugin/tests/test_plugin_version_bump.py index ed67d1ad..4238c4e4 100644 --- a/pact-plugin/tests/test_plugin_version_bump.py +++ b/pact-plugin/tests/test_plugin_version_bump.py @@ -12,10 +12,9 @@ """ import json +import re from pathlib import Path -import pytest - REPO_ROOT = Path(__file__).resolve().parent.parent.parent PLUGIN_JSON_PATH = ( REPO_ROOT / "pact-plugin" / ".claude-plugin" / "plugin.json" @@ -23,6 +22,12 @@ TARGET_VERSION = json.loads(PLUGIN_JSON_PATH.read_text(encoding="utf-8"))[ "version" ] +# Word-boundary pattern so e.g. "4.0.1" does NOT match "4.0.10" as a +# substring. The negative-lookbehind/lookahead exclude digits and dots +# on either side, so the version must appear as a self-contained token. +_TARGET_VERSION_PATTERN = re.compile( + r"(?