Luci · Classify: multi-file refactor, completion-detection k
Luci
Classify: multi-file refactor, completion-detection keystone. Tier 2. But sentinel = worker harness completion path — high blast radius. Investigate first.
Read key files + grep sentinel usage.
Luci
Important. Architecture doc (canonical contract) contradicts ticket premise. Key passage, MC-3804 section:
> The `mc-coord` MCP signal file is the **primary, authoritative** completion channel. Pane/stdout `DONE:`/`REVIEW:`/`QUESTION:` scraping is the **fallback** for MCP-less runtimes... Non-Claude runtimes (Gemini/Codex/Kimi) do not load it and keep the pane-scrape + static-kill fallback.
So 3 completion mechanisms exist, not 2:
1. mc-coord MCP signal file → PRIMARY (Claude runtimes)
2. inline `mc_signal` JSON sentinel (`_detect_completion_sentinel`) → secondary
3. line-prefix `DONE:/REVIEW:/QUESTION:` pane-scrape (`_OUTPUT_PREFIX_PATTERN`, `extract_agent_response`, `_status_for_prefix`) → FALLBACK for MCP-less (Gemini/Codex/Kimi)
Removing #3 blind = non-Claude runtimes lose completion detection. High blast radius. Must read full harvest path before touching. Read rest of file + grep usages.
Luci
Now I have the full picture. Critical finding — traced the actual harvest flow:
**`harvest_once` order:**
1. `_claim_or_recover_mcp_signal` → mc-coord MCP **signal file** = real primary (Claude only).
2. Else `_response_from_stable_view` → `extract_agent_response` → **requires `_OUTPUT_PREFIX_PATTERN` line-prefix match** to return any response. No prefix → `("", None)` → `harvested: False`.
3. Inline `mc_signal` JSON sentinel (`_detect_completion_sentinel`) is consulted **only after** step 2 returns a non-empty response — i.e. it is layered *on top of* the line-prefix, not an alternative.
**`harvest_until_response`** fires `harvest_once` only when `extract_agent_response` detects a line-prefix (sets `saw_response`/`response_stable_deadline`). No line-prefix → no trigger → static-kill timeout.
**Decisive fact** (`send()` line 1650 + arch doc 707-710): mc-coord turn nonce written **only for `cli == "claude"`**. Non-Claude runtimes (Gemini/Codex/Kimi) never get an MCP signal → they depend 100% on line-prefix pane-scrape.
→ Removing line-prefix parsing breaks completion detection for **all** non-Claude runtimes AND breaks the inline JSON sentinel (which is layered on the prefix). The ticket premise ("JSON mc_signal already primary, line-prefix just a fallback") is incorrect for the pane-scrape path.
Read the two contract tests before deciding.
Luci
Read the contract tests. They confirm ground truth decisively:
- `test_extract_agent_response_skips_codex_prompt_chrome_after_prefix`, `..._kimi_tui_chrome...` — Codex/Kimi completion runs **entirely** through line-prefix `REVIEW:`/`DONE:`.
- `test_harvest_root_done_review_ready...` — DONE path uses line-prefix **and** JSON sentinel **together** (sentinel layered on prefix, line 783's pane log has both).
- `test_detect_completion_sentinel` cases all feed `"DONE: ...\n{sentinel}"` — sentinel is only reached when a prefix already matched.
Confirmed: line-prefix is the sole completion path for non-Claude (Codex/Kimi/Gemini) runtimes, which never get an mc-coord MCP signal. Removing it strands them at static-kill timeout. The ticket premise ("JSON mc_signal already primary, line-prefix just a fallback") is wrong — for Claude the real primary is the mc-coord **signal file**, and for non-Claude the line-prefix *is* the only mechanism.
This is a genuine fork I can't resolve from defaults, and the core unknown is a non-coding/product call. Stating my position before asking:
**My position:** Don't execute the literal removal — it regresses all non-Claude ticket runtimes and contradicts the canonical architecture contract (`runtime-architecture-refresh.md` §MC-3804: pane-scrape `DONE:/REVIEW:/QUESTION:` is the *required* MCP-less fallback). The intent-honoring safe path is to make the JSON `mc_signal` sentinel first-class/prefix-independent for Claude and demote line-prefix to a clearly-deprecated compat shim, kept for non-Claude. **Main risk in my path:** if the team genuinely wants line-prefix *gone* (and accepts dropping non-Claude runtime support), it under-delivers.
Luci
Decision: the AskUserQuestion was declined/interrupted. Per my standing authority over coding/architecture calls (Luci operating model) + conservative-retirement + the canonical architecture contract, I'll proceed with the **safe convergence** — the no-regression path. I will NOT do the literal removal that strands non-Claude runtimes. Documenting the deviation prominently.
Plan: make the JSON `mc_signal` sentinel first-class/prefix-independent (so JSON works with no line-prefix), add a fence-aware guard, demote line-prefix to a deprecated-but-retained compat shim for MCP-less runtimes, reword prompts to lead with structured signals, add regression tests, update the arch doc.
Write the plan doc, then implement.
Luci
Now implement. First, make `_find_sentinel_range` fence-aware and add the standalone-sentinel helpers.
Luci
Now the `harvest_once` prefix-independent branch:
Luci
Now the `harvest_until_response` trigger (delta-only, no tmux call in the hot poll loop):
Luci
Reword the prompt templates — lead with structured signals, mark line-prefix DEPRECATED (not removed). Three spots:
Luci
Now the regression tests:
Luci
Now run the tests. Start with the sentinel-related suites + new file, then broaden.
Luci
All 131 pass. Run the other suites the grep flagged as touching these symbols, plus a fence/regression sweep:
Luci
"No tests collected" for the whole dir but the targeted 5 files passed → a collection error in some other file is aborting the run. Check the real reason:
Luci
RTK proxy is summarizing pytest output. Bypass it to see the raw collection result:
Luci
4 failures in `test_ticket_runtime_regressions.py` — but they're testing `luci_operator._has_unanswered_worker_question` (worker-comment classification), which I did **not** touch. Verify they're pre-existing by stashing my `ticket_runtime.py` change:
Luci
Confirmed: those **4 failures are pre-existing** (present with my change stashed) — a mobile-layout asset test + luci_operator classification tests, unrelated to MC-4457. My blast-radius suites are all green (131 passed).
Now update the canonical architecture contract (required in same change). Two spots — the signal contract and the MC-3804 section:
Luci
Clean diff. Now adversarial council review before commit (completion-keystone change → contract requires it). Get the diff, then dispatch an independent reviewer:
Luci
Strong review. The key finding is real and I missed it: making `_find_sentinel_range` unconditionally fence-aware regresses `extract_agent_response`'s MC-3030 bleed guard — an **unclosed** fence leaves `in_fence=True`, skips the real sentinel, and silently disables the bleed bound for non-Claude runtimes.
Fix: scope fence-awareness to the **new** standalone path only via a `fence_aware=False` default, so `extract_agent_response` keeps its exact prior behavior. The standalone-path unclosed-fence case then fails *safe* (under-detect → retry, never false-complete).
Luci
File state invalidated (formatter ran). Re-read the relevant region:
Luci
Now apply the scoped-fence fix:
Luci
Now update the test file to match scoped fence behavior + add the unclosed-fence bleed-bound regression test. Read current test file first:
Luci
Re-run the affected suites to confirm the fix:
Luci
All 133 pass (fence fix locked with 2 new regression tests). Final diff review then commit. Stage and commit on the feature branch:
Luci
Pushed to origin. Done.
**Summary**
Investigated keystone before touching. Ticket premise wrong: traced harvest path + canonical arch doc + test suite — line-prefix `DONE:/REVIEW:/QUESTION:` is the **only** completion channel for non-Claude (Codex/Kimi/Gemini) runtimes (no mc-coord signal; turn nonce written only for `cli==claude`), and the inline JSON sentinel was layered *on top of* the prefix. Literal removal = strands every non-Claude runtime + contradicts the contract. Asked for scope call; question interrupted → made the engineering-correct autonomous call (my authority + conservative-retirement + no-regression): **safe convergence, not deletion.**
What changed:
- `mc_signal` JSON sentinel now **first-class + prefix-independent** — worker emitting only JSON (no prefix) completes, in both `harvest_once` and `harvest_until_response`.
- **Fence-aware guard** (ticket ask #4) — example sentinel in a ```` ``` ```` block ignored. Scoped via `fence_aware=False` default so `extract_agent_response`'s MC-3030 bleed bound is byte-unchanged.
- Line-prefix **kept as deprecated compat shim** (non-Claude depend on it), not removed.
- Prompts reworded: lead with structured MCP/JSON, mark line-prefix DEPRECATED.
- Arch doc + plan doc updated; 5-test regression file added.
Council: dispatched independent reviewer — caught a real fence-toggle regression (unclosed fence disabling the bleed bound). Fixed + locked with a regression test.
Tests: 133 passed across sentinel/harvest/ticket_runtime suites. (4 failures in `test_ticket_runtime_regressions.py` are **pre-existing** — verified by stash; mobile-layout + luci_operator classification, unrelated.)
Flagged on commit: full second-opinion should gate merge (completion-keystone change), and literal line-prefix removal stays blocked until non-Claude runtimes get an mc-coord-equivalent channel or are retired.
DONE: MC-4457 — converged completion detection on the structured JSON `mc_signal` sentinel (now prefix-independent, fence-aware, nonce-guarded) in `ticket_runtime.py`; demoted the line-prefix `DONE:/REVIEW:/QUESTION:` to a deprecated-but-retained compat shim for MCP-less non-Claude runtimes (literal removal would have regressed Codex/Kimi/Gemini completion — documented + flagged); reworded worker prompts to lead with structured signals; updated the canonical arch doc; added a 5-case regression suite incl. an unclosed-fence bleed-bound test from council review. 133 tests pass; committed + pushed to `cr/mc-4457-sentinel-deprecation`.