feat(auto): route safe_defaults through DomainProfile (#809 P3, PR 5/6)#27
feat(auto): route safe_defaults through DomainProfile (#809 P3, PR 5/6)#27shaun0927 wants to merge 65 commits into
Conversation
…pics (Q00#640) (Q00#738) * feat(auto): block risky-fallback answers for regulated topics When a deterministic answer would land on CONSERVATIVE_DEFAULT or ASSUMPTION for a high-risk topic that has no defensible generic default, upgrade it to a BLOCKER instead of silently committing the auto Seed to a fabricated stance. Targeted topics: - regulated personal data (PII, GDPR, HIPAA, SOX, PCI-DSS) - destructive bulk schema/table operations (truncate/purge tables/schemas) Existing safe-allowlists keep working: product-feature questions about credentials/branches and the explicit `_blocker_for` patterns are unchanged. REPO_FACT/USER_GOAL-backed answers also pass through without gating. Refs Q00#640 * fix(auto): scope risky-fallback gate to generative answer routes only Address PR Q00#695 blocking review finding: The post-routing gate keyed off broad keywords, so meta-questions like "What acceptance criteria should the HIPAA worker satisfy?" or "Which command output verifies the GDPR export flow?" were rejected as if they asked for regulated-data handling decisions. They actually hit the `_feature_acceptance_answer` / `_verification_answer` routes which return safe templates regardless of subject keywords. Restructure `answer()` so the gate only fires after generative routes (actor/IO, runtime, product behavior, default). Meta-routes (non-goal listing, verification, feature acceptance) return early without going through the gate. Add regression coverage for HIPAA/GDPR/PII acceptance and verification phrasings to ensure they keep returning CONSERVATIVE_DEFAULT answers. Existing PII/HIPAA generative-route block tests continue to pass. Refs Q00#640 * fix(auto): include EXISTING_CONVENTION runtime fallback in risky gate Address PR Q00#695 follow-up: a regulated runtime question without a supplied repo_fact, e.g. "Which runtime should the HIPAA worker use?", routes through `_runtime_answer` and returns `AutoAnswerSource.EXISTING_CONVENTION` with the generic "use the existing repository runtime" template. Because EXISTING_CONVENTION was not in `_RISKY_FALLBACK_SOURCES`, that fallback escaped the gate. Add EXISTING_CONVENTION to the risky-fallback set. REPO_FACT-backed runtime answers (full `runtime_context` supplied) remain unaffected, and the existing `does_not_block_regulated_topic_when_repo_fact_supplied` test continues to assert that REPO_FACT answers pass through. Add a regression test that covers the bot's exact case: HIPAA runtime question with no supplied facts now blocks with reason "regulated data handling". Refs Q00#640 * fix(auto): broaden destructive-bulk patterns and cover reverse phrasing Address PR Q00#695 follow-up: the destructive-operation matcher only caught ``verb ... noun`` phrasings such as ``purge tables``, so reverse phrasings like ``Which tables should the migration truncate?`` slipped through. The verb vocabulary was also narrow. Expand patterns: - Verbs: ``truncate``, ``purge``, ``wipe`` plus tense variants (``truncates``/``truncating``/``truncated`` etc.). - Nouns: ``table(s)``, ``schema(s)``, ``database(s)``, ``index/indexes/indices``, ``migration(s)``. - Both verb-then-noun and noun-then-verb directions matched. Note: ``drop ... database`` remains owned by ``_blocker_for`` (its existing branch fires first), and product-feature questions are still exempted by the safe-product allowlists, so this does not over-gate benign feature semantics. Add a regression test exercising both phrasing directions across the new verb/noun vocabulary. Refs Q00#640 * chore: drop stray local debug artifact accidentally committed in previous fix * fix(auto): add drop and erase to destructive-bulk verb vocabulary Address PR Q00#695 follow-up: phrasings like "Which tables should the migration drop?" or "Should we erase these schemas before re-seeding?" still flowed through to a generic auto answer because ``drop`` and ``erase`` were missing from `_DESTRUCTIVE_BULK_VERBS`. `_blocker_for` already handles ``drop|delete|erase|wipe ... database`` at the explicit-authority layer, so this matcher and the existing allow/deny list both treat the same families consistently. The risky-fallback gate runs after `_blocker_for`, so explicit authority-style prompts continue to block via the original code path. Add ``drop`` and ``erase`` (with tense variants) and extend the regression test to assert both verb-then-noun and noun-then-verb phrasings using the new vocabulary. Refs Q00#640 * fix(auto): require schema/data context for destructive-bulk gate (Q00#640) Add _DESTRUCTIVE_BULK_NON_DATA_QUALIFIERS to exempt verb/noun pairs that appear with a non-data artefact qualifier (release plan, docs, roadmap, etc.) from the destructive-bulk blocker. Also extend _DESTRUCTIVE_BULK_NOUNS with record/row/audit-log/audit-trail strong data-object nouns. Addresses ouroboros-agent[bot] follow-up warning on Q00#738: bare ``migration`` + ``drop`` and ``index`` + ``drop`` questions about release plans or documentation were overblocked. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(auto): allow product-semantics questions through risky-fallback gate (Q00#640) Add _is_safe_product_regulated_question() allowlist that passes through bounded product-behavior questions mentioning regulated nouns (PII/GDPR/ HIPAA/SOX/PCI-DSS) when paired with a product-semantics verb (export, download, display, show, view, access, …) and NOT a compliance-policy verb (store, handle, retain, collect, encrypt, …). Questions like "Should the app export PII reports?" or "Should users be able to download GDPR exports?" are feature-level requirements and must not be blocked; questions asking how to store/handle/retain regulated data still block as before. Addresses ouroboros-agent[bot] BLOCKING on Q00#738 (answerer.py:716). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(auto): route safe regulated-product questions through product-behavior answerer (Q00#640) Extend _is_product_behavior_question() with a new arm covering the product-semantics verbs used by _is_safe_product_regulated_question() that were not previously matched (download, allow, expose, render, enable, support) and the "be able to <verb>" phrasing gap for view/access. Previously, questions allowed past the risky-fallback gate (e.g. "Should users be able to download GDPR exports?") fell through to _default_answer(), producing a generic conservative-MVP ledger entry that silently discarded the regulated-product feature semantics. With this fix the router at answerer.py:122 sends those questions to _product_behavior_answer(), which writes subject-specific constraints.behavior.* and acceptance.behavior.* ledger entries that preserve the requested feature in the Seed contract. New test: test_auto_answerer_routes_safe_regulated_product_questions_to_product_behavior_answerer asserts blocker=None, source=CONSERVATIVE_DEFAULT, subject-specific ledger keys (not conservative_mvp), and regulated noun present in answer text/ledger entries for all three bot example questions. Addresses ouroboros-agent[bot] BLOCKING on Q00#738 (answerer.py:741). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(auto): phrase-scope destructive-bulk non-data qualifier (Q00#640) Previously the destructive-bulk exemption matched on bare tokens such as ``documentation`` or ``release plan`` anywhere in the sentence, which let real destructive operations slip past the gate when the question merely *referenced* documentation as an authority (e.g. "Which tables should we drop according to the documentation before redeploying?"). The qualifier is now strictly phrase-scoped to ``from the …`` so the exemption fires only when the artefact is the explicit object of the drop/wipe — the phrasing that signals "remove an entry from a process artefact" rather than "delete data from a system". Authority/reference phrasings ("according to the documentation", "per the release plan", "in the documentation example") no longer suppress the gate. Existing pass-through tests still hold: - "Which migration should we drop from the release plan?" → not blocked - "Which indexes should we drop from the docs?" → not blocked New regression test locks the safety boundary: - "Which tables should we drop according to the documentation …" → BLOCKER - "Which tables should we drop per the release plan?" → BLOCKER - "Per the documentation, which audit logs should we purge?" → BLOCKER - "According to the docs, which tables should we drop?" → BLOCKER Ref: ouroboros-agent[bot] BLOCKING on PR Q00#738 — answerer.py:688. 68 tests passing in test_ledger_grading_answerer.py (337 in tests/unit/auto). Ruff clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auto): widen artefact qualifier and trust adjectival compliance verbs (Q00#640) Two BLOCKING regressions raised by ouroboros-agent[bot] on the previous fix commit (fc11788): 1) **answerer.py:698** — destructive-bulk exemption only matched ``from the …`` so safe process-artefact phrasings such as ``Which indexes should we drop in the docs?`` and ``Which migration should we drop in the roadmap?`` were still mis-blocked as data destruction. The qualifier now also accepts ``in the …`` for the same artefact list (``release plan``, ``docs``, ``documentation``, ``plan``, ``roadmap``, ``backlog``, ``changelog``, ``spec``). Authority/reference phrasings (``according to the docs``, ``per the release plan``) still do not match the qualifier and remain blocked, locked in by an expanded regression test. 2) **answerer.py:782** — ``_is_safe_product_regulated_question()`` rejected any question containing a compliance-policy verb (``store``, ``handle``, ``encrypt``, ``share``, …) anywhere in the sentence. That over-blocked legitimate product-behavior questions where the compliance verb appeared as a past-participle adjective modifying the noun, e.g. Should admins be able to view stored PII fields? Should the dashboard display encrypted HIPAA files? In both, the main verb is product-semantics (``view`` / ``display``); the compliance verb is adjectival. The allowlist now requires (a) a regulated noun, (b) a product-question modal, and (c) a product-semantics verb. Pure compliance-policy phrasings (``How should the system handle GDPR data retention?``, ``What PII should the system collect?``) lack a product-semantics verb and remain blocked — covered by the existing ``test_auto_answerer_still_blocks_compliance_policy_regulated_questions``. The previously-defined ``_COMPLIANCE_POLICY_VERBS_RE`` constant is now unused and removed to avoid dead code. New regression coverage: - ``test_auto_answerer_allows_in_the_artefact_drop_questions`` — locks in ``in the docs/roadmap/release plan/changelog`` exemption. - ``test_auto_answerer_allows_product_questions_with_adjectival_compliance_verbs`` — locks in ``view stored PII``, ``display encrypted HIPAA files``, ``download retained GDPR exports``, etc. Existing safety tests (compliance-policy questions, authority-reference phrasings) all continue to block. 339 unit tests passing in tests/unit/auto. Ruff clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auto): block mixed-intent regulated questions via active-verb precedence (Q00#640) Bot follow-up on commit e846a47: the regulated-product allowlist was too permissive. Mixed-intent questions that pair a compliance-policy verb with a product-semantics verb — e.g. How should the system store and display HIPAA files? Should we retain and export PII records? still ask the auto pipeline to decide regulated-data handling and must remain blocked, even though they also mention a product verb. The fix adds an explicit precedence rule: an *active*-form compliance-policy verb (``store`` / ``stores`` / ``storing``, ``retain`` / ``retains`` / ``retaining``, ``encrypt``, ``handle``, ``collect``, ``share``, ``transmit``, ``disclose``, ``process``, ``manage``, ``govern``) blocks the question even if a product-semantics verb is also present. Past-participle forms (``stored``, ``encrypted``, ``retained``, ``collected``, ``shared``, …) are intentionally excluded from the negative list because they act adjectivally on the regulated noun (``view stored PII``, ``display encrypted HIPAA files``); the main verb of those sentences is the product-semantics one and the question is product-behavior over already- existing regulated data, not a compliance-policy decision. New regression test ``test_auto_answerer_blocks_mixed_intent_regulated_questions`` locks the precedence rule on the bot's own examples plus three more variants covering ``encrypt`` / ``share`` / ``collect``. Existing positive tests (adjectival compliance verbs, pure product semantics) and existing negative tests (pure compliance phrasings) all continue to pass. 340 unit tests passing in tests/unit/auto. Ruff clean. Ref: ouroboros-agent[bot] BLOCKING on Q00#738 — ``answerer.py:750``. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auto): align router verb list with regulated allowlist (Q00#640) Bot follow-up on commit 7ed761c: ``_PRODUCT_SEMANTICS_REGULATED_VERBS_RE`` allows ``export`` and ``show`` (and the rest of the safe-allowlist set), but the explicit alignment branch in ``_is_product_behavior_question()`` only listed a subset (``download/allow/expose/render/enable/support/view/access``). ``export`` / ``show`` / ``display`` were still matched by an earlier broader pattern in the same function, but the visible alignment was incomplete and prone to silent drift. The router branch added to bridge ``_is_safe_product_regulated_question()`` into ``_is_product_behavior_question()`` now lists every verb in the allowlist: export | download | render | display | show | expose | support | enable | allow | view | access This is a no-op for already-routed verbs but makes the allowlist↔router contract explicit and grep-checkable, eliminating the drift surface flagged in the bot's design note. Test changes: ``test_auto_answerer_routes_safe_regulated_product_questions_to_product_behavior_answerer`` now exercises every verb in the allowlist (export, show, display, render, expose, support, enable in addition to download/view/access). Each case asserts (a) the gate passes (``answer.blocker is None``), (b) the router takes the product-behavior path (``constraints.behavior.*`` and ``acceptance.behavior.*`` ledger keys, not the generic ``constraints.conservative_mvp`` from ``_default_answer()``), and (c) the regulated noun is preserved in the answer text or ledger value. 340 unit tests passing in tests/unit/auto. Ruff clean. Ref: ouroboros-agent[bot] BLOCKING on Q00#738 — ``answerer.py:548``. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auto): route regulated-product questions before IO/runtime branches (Q00#640) Bot follow-up on commit 52ef5ab: ``_is_safe_product_regulated_question()`` suppressed the risky-fallback blocker for any regulated-noun + product-verb combination, but the router checked ``_is_actor_or_io_question`` and ``_is_runtime_context_question`` *before* ``_is_product_behavior_question``, so prompts like What inputs should the GDPR export take? Which runtime should the GDPR export use? got a generic IO/runtime answer (``ASSUMPTION`` / ``EXISTING_CONVENTION``) and then bypassed the blocker via the safe-allowlist — silently dropping the regulated-feature semantics from the ledger. Fix: pull ``_is_safe_product_regulated_question`` to the top of the content-routing chain so any regulated-product question — IO-shaped, runtime-shaped, or product-shaped — is dispatched to ``_product_behavior_answer()``. The risky-fallback gate at the tail of ``answer()`` already consults the same predicate, so the router and the safe-allowlist now share a single answer path. Pure compliance phrasings remain blocked unchanged: they fail the allowlist (no product-semantics verb) and fall through to the previous branches, where the risky-fallback gate fires for any ``CONSERVATIVE_DEFAULT`` / ``ASSUMPTION`` / ``EXISTING_CONVENTION`` source. New regression test ``test_auto_answerer_routes_regulated_product_questions_before_io_or_runtime`` locks in: - Bot's example "What inputs should the GDPR export take?" - Bot's example "Which runtime should the GDPR export use?" - Two adjacent IO/runtime regulated-product variants Each case asserts (a) not blocked, (b) answer comes from ``_product_behavior_answer()`` (subject-specific ``behavior.*`` ledger keys, no IO/runtime keys), and (c) the regulated noun is preserved in the answer text or ledger value. 341 unit tests passing in tests/unit/auto. Ruff clean. The two failures in tests/unit/orchestrator/test_codex_cli_runtime.py are pre-existing and unrelated to this PR's scope (verified by stashing the patch). Ref: ouroboros-agent[bot] BLOCKING on Q00#738 — ``answerer.py:837``. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auto): preserve grounded REPO_FACT for regulated-runtime questions (Q00#640) Bot follow-up on commit 44e405f: the unconditional early route to ``_product_behavior_answer()`` for any question that ``_is_safe_product_regulated_question()`` recognised broke the existing runtime contract. With a supplied ``runtime_context`` repo fact, a question like ``Which runtime should the GDPR export use?`` should return a ``REPO_FACT`` runtime answer carrying the grounded evidence; the early route replaced it with a generic product-behavior entry, dropping the evidence. Restructure: keep the original IO/runtime/product/default order so grounded ``REPO_FACT`` answers stay on the runtime path, then re-route to ``_product_behavior_answer()`` only when the chosen route produced a non-grounded fallback (``ASSUMPTION`` / ``EXISTING_CONVENTION`` / ``CONSERVATIVE_DEFAULT``) AND the safe-allowlist recognises the question as regulated-product. Concretely: - ``Which runtime should the GDPR export use?`` + REPO_FACT → REPO_FACT runtime answer (preserved, with evidence). - ``Which runtime should the GDPR export use?`` without repo facts → EXISTING_CONVENTION runtime fallback re-routed through ``_product_behavior_answer()`` so the regulated-feature semantics are preserved in ``constraints.behavior.*`` / ``acceptance.behavior.*``. - ``What inputs should the GDPR export take?`` → IO ASSUMPTION re-routed to ``_product_behavior_answer()``. - ``Should the app export PII reports?`` → already routes through ``_product_behavior_answer()`` (CONSERVATIVE_DEFAULT) and is left untouched by the reroute. Pure compliance phrasings still block: they fail the allowlist (no product-semantics verb), keep their CONSERVATIVE_DEFAULT/ASSUMPTION/ EXISTING_CONVENTION source, and the risky-fallback gate fires for them. New regression test ``test_auto_answerer_preserves_repo_fact_for_regulated_runtime_question`` locks the REPO_FACT preservation contract: with a runtime_context repo fact supplied, the answer must be REPO_FACT, must contain the supplied runtime text, and must carry a runtime_context ledger entry with the supplied evidence. 342 unit tests passing in tests/unit/auto. Ruff clean. Ref: ouroboros-agent[bot] BLOCKING on Q00#738 — ``answerer.py:126``. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: drop stray empty .ouroboros_eval_artifact.md committed by mistake The previous commit (046ce3d) accidentally included an empty local debug artifact via ``git add -A``. Removing it; not part of the PR scope. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auto): tighten bare-scope and ambiguous-artefact regex (Q00#640) Two BLOCKING items raised by ouroboros-agent[bot] on commit a13fd6c: (1) ``answerer.py:793`` — ``_is_safe_product_regulated_question()`` allowed "compliance-scope-as-feature-flag" prompts (``Should the platform support HIPAA?``, ``Should the app enable GDPR?``, ``Should the system allow PII?``) to bypass the blocker, even though those frame the entire regulatory regime as a binary toggle and are compliance-policy decisions. Fix: a new ``_BARE_COMPLIANCE_SCOPE_RE`` rejects ``support|enable|allow`` + bare regulated noun followed by no qualifying feature noun (negative lookahead ``(?!\s+[a-z])``). Concrete-feature variants ("HIPAA audit logs", "GDPR consent banners", "PII redaction in exports", "GDPR data") have a qualifying noun and still pass through. (2) ``answerer.py:718`` — the destructive-bulk artefact qualifier listed standalone ``doc`` and ``plan`` tokens. ``from the doc`` is rare phrasing (use ``docs`` / ``documentation``) and bare ``plan`` collides with database-side meanings (query plan, execution plan, db plan), so a question like "Which tables should we drop from the plan?" was being exempted as a process-artefact edit. Fix: drop ``doc`` and ``plan`` (singular) from the artefact list. The remaining unambiguous artefacts are ``release plan``, ``docs``, ``documentation``, ``roadmap``, ``backlog``, ``changelog``, ``spec``. All existing positive tests already use these unambiguous variants. New regression coverage: - ``test_auto_answerer_blocks_bare_compliance_scope_questions`` — locks rejection of bare ``support|enable|allow + regulated noun`` for all five regulated-noun variants. - ``test_auto_answerer_allows_qualified_compliance_scope_questions`` — locks pass-through of ``support HIPAA audit logs`` / ``enable GDPR consent banners`` / ``allow PII redaction`` / etc. - ``test_auto_answerer_blocks_destructive_bulk_with_ambiguous_singular_tokens`` — locks blocker for ``from the plan`` / ``in the plan`` / ``from the doc`` destructive prompts. 345 unit tests passing in tests/unit/auto. Ruff clean. Ref: ouroboros-agent[bot] BLOCKING on Q00#738 — ``answerer.py:793`` and ``answerer.py:718``. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
…00#739) * feat(auto): classify resume capability for auto pipeline state (Q00#688) Adds AutoResumeCapability StrEnum and AutoPipelineState.resume_capability() that derive — purely from persisted state — what `ooo auto --resume` will actually do for a session. Centralizes hint rendering in ouroboros.auto.resume_render so CLI and MCP surfaces stay in lockstep. Honors the critic-driven matrix corrections: - REPAIR is non-terminal and classifies as RESUME (transitions to REVIEW on resume). - seed_generator with seed_path-only classifies as PARTIAL_RESUME. - run_starter with seed_path-only classifies as PARTIAL_RESUME. - COMPLETE classifies as NONE; render_resume_lines emits no hint when goal is omitted (which COMPLETE callers do). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(auto,cli,mcp): render truthful resume/retry semantics across surfaces (Q00#688) Wires the new AutoResumeCapability classifier into all three rendering surfaces so the hint line accurately describes what `--resume` will do: - AutoPipelineResult gains a `resume_capability: str` field (default "resume" for back-compat with existing test constructions); AutoPipeline._result() populates it from state.resume_capability(). - _print_status reads state.resume_capability() and emits the right RESUME / PARTIAL_RESUME / RETRY / Start-fresh hint via render_resume_lines, suppressing all hints for COMPLETE. - _print_result and the MCP _format_result read result.resume_capability and render via the same helper. They omit the Start-fresh hint because the result surface does not carry the original goal. The Q00#688 driver: an `interview.start` timeout that left `interview_session_id=None` now correctly renders as "Retry: ooo auto --resume <id>" instead of the old unconditional "Resume:" hint. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(auto): cover resume capability matrix and rendering surfaces (Q00#688) - tests/unit/auto/test_state.py: 18 matrix tests pinning every cell of AutoPipelineState.resume_capability(), including the critic-driven REPAIR / seed_path-only / run_starter+seed_path / FAILED-mirrors-BLOCKED cases. - tests/unit/auto/test_interview_pipeline.py: a regression test confirming that an `interview.start` timeout simultaneously satisfies `_recoverable_phase_for_tool('interview.start') == AutoPhase.INTERVIEW` AND `state.resume_capability() == AutoResumeCapability.RETRY`. - tests/unit/cli/test_auto_command.py: 9 tests asserting the bare substrings rendered by _print_status and _print_result for each of the four capability tiers. - tests/unit/mcp/tools/test_auto_handler_resume_lines.py: 4 tests asserting MCP `_format_result` emits the same substrings without Rich markup. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auto): classify seed_generator regen as RETRY (Q00#688) When seed generation blocked with only ``interview_session_id`` preserved (no seed_artifact, no seed_path), --resume re-invokes the generator from scratch. That is RETRY semantics, not RESUME — only the interview context carries forward; no prior generation work is reused. The previous classification matched the plan matrix in error and would have rendered ``Resume:`` for a path that, by the very definition of Q00#688, is a retry. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auto): shell-quote and Rich-escape goal in Start fresh hint (Q00#688) The ``Start fresh: ooo auto "<goal>"`` hint previously interpolated the user-controlled goal verbatim. A goal containing shell metacharacters (``"``, ``$``, ``\\``) would break out of the suggested command if the user copy-pasted it. With Rich markup enabled, ``[red]ALERT[/]`` would be rendered as styled output rather than literal text. - ``shlex.quote`` produces a shell-safe form regardless of the goal text. - When ``use_markup=True`` the quoted form is also Rich-escaped so any brackets in the original goal pass through as literal characters. - The MCP plain-text surface remains markup-free, so it only needs the shell quoting. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(mcp): gate resume_command meta on capability (Q00#688) Bot review on PR Q00#724 flagged that ``meta.resume_command`` was emitted unconditionally even when ``_format_result()`` deliberately suppressed the human-readable hint (capability=none, e.g. COMPLETE or unrecoverable BLOCKED/FAILED sessions). MCP clients that key off the metadata field would route users into a guaranteed-failing ``--resume`` call, directly contradicting Q00#688's premise. - ``meta.resume_command`` is now only set when ``resume_capability != "none"``. - ``meta.resume_capability`` is exposed alongside, so clients can render retry / partial-resume / resume actions independently. - Added MCP handler integration tests pinning both fields across all four capability tiers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auto): classify run_start_attempted without handle as NONE (Q00#688) Bot review on PR Q00#724 (commit `728e6041`) flagged that ``resume_capability()`` returned ``RESUME``/``PARTIAL_RESUME`` for blocked ``run_starter`` sessions that had ``run_start_attempted=True`` but no durable run handle. ``AutoPipeline.run()`` short-circuits at ``state.run_start_attempted`` (pipeline.py:273) to refuse a duplicate execution, so ``--resume`` cannot make progress in that state — the CLI and MCP surfaces would advertise an actionable resume that is guaranteed to bounce back to ``blocked``. Add the ``run_start_attempted`` gate to the RUN-phase classifier so the capability matches the actual recovery path. Two new tests pin both the ``seed_artifact`` and ``seed_path`` flavours of this case. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(auto): type AutoPipelineResult.resume_capability as enum (Q00#688) Q00 design feedback on PR Q00#724: make the program contract capability-first so that CLI text, MCP text, and MCP metadata cannot drift out of sync. The previous design stored ``AutoPipelineResult.resume_capability`` as a plain ``str``, then converted to ``AutoResumeCapability`` at every render site. Two surfaces could drift if a result was constructed with an arbitrary string (the default ``"resume"`` masked accidental fixtures that bypassed ``state.resume_capability()``). Tighten the contract: - ``AutoPipelineResult.resume_capability`` is now typed as ``AutoResumeCapability``; pipeline ``_result()`` passes the enum directly. - CLI ``_print_result`` and MCP ``_format_result`` consume the enum unchanged (no per-call coercion). - ``_result_meta`` serializes the enum to a stable string via ``.value`` for the wire format. - ``_result_meta`` capability gate compares against ``AutoResumeCapability.NONE`` directly. - Test fixtures construct results with the enum, removing the implicit ``"resume"`` default that hid drift. The wire format (``meta.resume_capability`` string) is unchanged, so MCP clients see no behaviour difference. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(hook): route 'ooo auto' to /ouroboros:auto skill The /ouroboros:auto skill exists at skills/auto/SKILL.md and is listed in the available skill set, but the UserPromptSubmit keyword detector at scripts/keyword-detector.py never had an 'ooo auto' entry in KEYWORD_MAP. Users who type 'ooo auto ...' get no Ouroboros routing suggestion, so when other plugins' keyword detectors fire on the same prompt their suggestions win by default. Add 'ooo auto' to KEYWORD_MAP so the detector emits a <skill-suggestion> block pointing at /ouroboros:auto, on the same footing as the other ooo subcommands (run, seed, interview, etc.). The MCP setup gate in main() applies automatically because /ouroboros:auto is not in SETUP_BYPASS_SKILLS, so first-time users without MCP still get redirected to /ouroboros:setup. Also add the missing row to the ooo command table in CLAUDE.md and unit tests covering bare invocation, goal argument, --resume flag, and a guard against over-matching the unrelated word 'autopilot'. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(hook): satisfy ruff format + sync ooo command summary table Address bot review feedback and unblock the Ruff Lint gate on PR Q00#741. - tests/unit/scripts/test_keyword_detector.py: reformat the new `assert ... , "..."` block to ruff's preferred parenthesized layout so `ruff format --check src/ tests/` passes. Behaviour unchanged. - CLAUDE.md: add the missing `ooo auto` row to the second `## ooo Commands` summary table (lower in the file). The dev-mode table at the top already lists `ooo auto`; the bot's non-blocking finding (CLAUDE.md:69) flagged this internal inconsistency. Mirrors the existing rows by recording the MCP tool the skill loads (`MCP: ouroboros_auto`). No production code is touched. The keyword routing change from the parent commit is unmodified; this commit only fixes lint and docs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(hook): cover ooo auto through main() setup gate Address ouroboros-agent[bot]'s non-blocking suggestion on PR Q00#741: the existing tests only locked `detect_keywords()` for the new `ooo auto` route; this commit also exercises `main()` so the user-visible behaviour through the setup gate is covered. - test_ooo_auto_unconfigured_redirects_to_setup: with MCP unconfigured, `ooo auto "..."` must steer the user to /ouroboros:setup (auto is intentionally not in SETUP_BYPASS_SKILLS, since the skill requires the MCP tool). - test_ooo_auto_configured_routes_to_auto_skill: with MCP configured, `ooo auto "..."` must surface /ouroboros:auto and must not emit the setup redirect. No production behaviour change; tests-only. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(rfc): UserLevel plugin layer Pins the framing decisions converged on in Q00#725 and the seven contract decisions locked in Q00/ouroboros-plugins issues #5–#11. Future debates about "should this go in core?" or "should we add this to ooo auto?" should be answered against this document. Sections (per Q00#726 spec): 1. Status — Accepted; supersedes growth-oriented portions of Q00#725. 2. Motivation — boundary problem, Q00#689 case study, defense framing. 3. Layer Model — 4-tier diagram (canonical reference). 4. Why Defense-Oriented — three reasons + success metric definition. 5. Manifest Schema — references upstream schemas/0.1/, vendoring strategy (resolves Q00#736). 6. Invocation Contract — firewall responsibilities (Q00#729), audit-event compatibility (resolves Q00#737). 7. UX — 'ooo plugin add <repo-url>' flow, anti-patterns. 8. Reference Plugin — Q00/ouroboros-plugins is curated, not a marketplace; github-pr-ops is the v0 contract proof (review only, per Q00/ouroboros-plugins#7 lock). 9. ooo auto Boundary — permanent shape; closed Q00#689 PR stack as de facto evidence; Q00#735 mechanical guard. 10. Deferred Decisions — what is intentionally postponed. 11. Related Work — cross-links to all 12 sub-issues of Q00#725 and 11 Q00/ouroboros-plugins issues. Closes Q00#726. * docs(rfc): address review feedback on UserLevel plugin RFC - Unify CLI naming: trust-failure error message uses `ooo plugin trust ...` to match every other CLI example in the document. The `ouroboros` binary is not a separate user-facing entrypoint. - Pin first-party trust semantics: first-party programs ship inside the core release artifact, so their `required: true` permissions are registered as implicitly trusted at boot. The firewall MUST NOT block them on the trust check; non-first-party plugins never inherit this. - Clarify "v0 lifecycle" scope: "lifecycle" refers narrowly to state-transition verbs (install/inspect/trust/disable/remove). The read-only `add`, `discover`, `list` commands ship in v0; only the `update` transition is deferred. - Sharpen the audit-payload ban: tokens / channel IDs / free-form user messages are forbidden in plugin-defined audit fields, not in argv (which is recorded as-is and is the caller's responsibility). Plugins MUST refuse to accept secrets via argv and document the secure path. Closes the BLOCKING review items raised by ouroboros-agent on commit 6f2699a and the two non-blocking documentation suggestions. * docs(rfc): resolve lifecycle/schema/audit ambiguities flagged on round 2 Addressing ouroboros-agent review on commit ecb6212: - Lifecycle paragraph (BLOCKING): the previous wording classified `add` as read-only, contradicting the UX section where `ooo plugin add` triggers selection + install. The actual Q00#731 split (per merged commits d2a214d / a014e5e) is: state-mutating `add/install/trust/disable/remove` vs read-only `discover/inspect/list`. The deferred verb is `update` only; the documented v0 upgrade path is `remove` + `add`. - Schema path consistency (warning): the URL examples use `schemas/<MAJOR.MINOR>/` (e.g. `0.1/`), but the vendoring strategy and versioning policy were written as `schemas/<major>/`. Tooling can't reconcile both. Both paragraphs now use `<MAJOR.MINOR>/` consistently and explicitly state the vendor path mirrors upstream one-for-one. - `docs/audit.md` reference (warning): that file does not exist in this repo, so an RFC-level "string-only per docs/audit.md" rule pointed at nothing. Replaced with a direct citation of `audit-event.schema.json`, which IS the canonical source, and added an explicit disclaimer that this RFC does not introduce a separate `docs/audit.md` contract. * docs(rfc): polish add-vs-install distinction and reframe boundary evidence Round-3 review came back APPROVE with two non-blocking documentation suggestions; addressing both for cleanliness: - UX section now spells out the `add` vs `install` relationship: `add` is the interactive entry point that internally invokes `install`; `install` is the non-interactive primitive addressed by plugin name. They are layered, not redundant. - "ooo auto Boundary" section reframes the point-in-time `grep` assertion as historical rationale rather than an evergreen claim. The durable enforcement of the boundary is the Q00#735 CI lint guard, not the snapshot. Removing/weakening that guard now triggers an explicit "this RFC must be revisited" clause. * docs(rfc): add Implementation Status matrix to disambiguate RFC vs main Round-4 review (commit ec13370) flagged that the document reads as if firewall.py / sync-plugin-schemas.sh / ooo plugin CLI / Q00#735 CI guard already exist on main, when none of them do. That made the "Accepted" RFC a misleading source of truth. Adds a new "Implementation Status" subsection inside Status that: - States explicitly that "Accepted" locks the design, not the artifacts. - Pins implementer-facing prose to RFC-2119 MUST: present tense in this document is the target contract, not a description of repo state. - Tabulates every concrete artifact referenced later in the document against its tracking issue and current status (shipped upstream vs. not yet present in core), so contributors can tell at a glance which paths are reality vs. target. - Calls out two follow-on consequences: (1) boundary enforcement is currently documentary until Q00#735 lands; (2) implementation PRs that build the unbuilt rows MUST conform to this RFC, with drift requiring an RFC amendment, not silent divergence. Also softens the "ooo auto Boundary" section so the Q00#735 guard claim is explicitly future-tense ("will add", "once that guard ships"), aligned with the matrix and removing the bot's third blocking finding. * docs(rfc): tighten argv redaction + pin plugin name→namespace mapping Round-5 review (commit 805dd45) raised one BLOCKING and one warning: - BLOCKING: argv-as-is creates a contract-level secret leak path. The caller-only obligation was too weak for a security boundary doc. Replaced with a 4-clause defense-in-depth contract: 1) plugins MUST NOT accept secrets via argv (primary control); 2) firewall MUST apply a built-in argv redaction policy before ledger write (well-known secret flags + high-confidence token formats: Bearer, gh[oprsu]_, sk-, AKIA-, JWT-shaped); 3) v0 redaction is exactly the built-in policy — per-command extension lands later (queued in Deferred Decisions); 4) caller responsibility persists — the safety net limits blast radius, not sanctions argv-as-secret-channel. Optional sha256-of-original argv may be recorded alongside redacted positions for forensic reconciliation; original value MUST NOT. - Warning: plugin name (`github-pr-ops`) vs command path (`ooo github-pr review`) was undefined. Pinned the rule: manifest `name` IS the user- facing command namespace, no aliasing. Plugin `github-pr-ops` is invoked as `ooo github-pr-ops <command>`. The example bash session now uses `ooo github-pr-ops review` to match. Collisions across installed plugins are rejected by `ooo plugin install`. * docs(rfc): bind trust identity to (source.url, manifest_digest), not name Round-6 review (commit 7ff2866) flagged a permission-escalation path: keying trust on manifest `name` alone lets a user `remove` plugin A, install plugin B from a different repo under the same `name`, and B inherits A's previously granted scopes without re-trust. That is a contract-level security bug. Splitting the two concerns: - Manifest `name` keeps its role as CLI namespace + install-time uniqueness check (collision = `install` refuses). - Trust records and `~/.ouroboros/plugins.lock` entries MUST be keyed by the tuple `( source.type, source.url, manifest_digest )`, where `manifest_digest` is the sha256 of the canonical-form manifest at install time and `source.url` is normalized. Concrete lifecycle obligations: - `remove` deletes every trust record bound to the current triple (no tombstone with implicit re-grant). - `install` re-computes the triple; any field difference from a prior record under the same `name` is treated as a fresh trust subject with all `required: true` permissions un-trusted. - `trust` writes records bound to the current triple only — trust never carries across triple changes, including same-source version bumps (digest changes ⇒ new subject). - Deferred `update` MUST surface the digest change and force re-confirm of any permission whose risk class changed; until then, the upgrade path is `remove` + `add`, which forces fresh trust by construction. This makes the trust subject "this exact artifact from this exact source", not "anything that ever called itself X". * docs(rfc): close trust-key seams (non-URL sources, install ambiguity) Round-7 review (commit c9b068c) flagged two consequential gaps in the trust-binding rule from round-6: 1) `(source.type, source.url, manifest_digest)` was undefined for `local_path` and `first_party`, since neither necessarily has a URL. Replaced `source.url` with `source_identity` and added a per-type dispatch table: - plugin_home → normalized repo URL (no scheme variants, no trailing `.git`, no fragment). - local_path → absolute resolved filesystem path of the plugin directory at install time (symlinks resolved; relatives rejected). - first_party → the manifest `name`; first-party records live in core's in-process trust table only and do NOT appear in the user lockfile, since their permissions are implicitly trusted at boot per the Manifest Schema section. `manifest_digest` (sha256 of canonical-form manifest at install time) applies in every case, including first_party. 2) `ooo plugin install <name>` was ambiguous when more than one known catalog exposes the same plugin name. Pinned the resolution rules: - Default form: succeeds iff exactly one known source provides the name; otherwise "ambiguous plugin name" error listing candidates. No heuristic selection. - Qualified form: `--from <repo-url|path>` always selects an explicit source. CI/scripts SHOULD prefer the qualified form. - No-match form: `install` instructs the user to `add` the repo first. MUST NOT search the network silently. * docs(rfc): close local_path lifecycle + cross-link Q00#727 architecture diff Round-8 was APPROVED with one follow-up warning and one non-blocking note; addressing both for a fully clean state before pinging: - Local-path source lifecycle (warning): the RFC referenced "registered local_path source" without defining how registration happens, leaving Q00#731 without an unambiguous entry-point for that source type. Pinned the rule: `ooo plugin install <name> --from <local-path>` is both register-on-first-use and install in the same command. There is no separate `register-local` verb in v0. Subsequent default-form installs can address that name if unambiguous. Also explicitly reaffirms that `first_party` does not go through registration at all (boot-populated, per the Manifest Schema section). - Architecture-doc cross-link (non-blocking): added an explicit note in the Layer Model section that core's `docs/architecture.md` still carries the older `PLUGIN LAYER` framing at RFC merge, that the divergence is tracked in Q00#727, that this RFC's diagram is the authoritative one for new work, and that any apparent conflict is resolved in favor of this RFC until Q00#727 ships. * docs(rfc): bind trust to artifact_digest, fix scheme aliasing, define disable Round-9 review (commit 0a7332f) raised three more security/contract gaps. All three addressed in this round: 1. Scheme aliasing in plugin_home normalization (BLOCKING). The prior wording "strips scheme variants" collapsed http:// and https:// to the same trust subject. Replaced with a strict-but-conservative normalization rule that PRESERVES scheme exactly, lowercases the host, and strips only trailing `.git`, fragment, and userinfo. `http://example.com/x` and `https://example.com/x` are now distinct trust subjects. 2. Manifest-only digest binding (BLOCKING). The prior trust subject used `manifest_digest`, leaving a code-substitution path: change the entrypoint without touching the manifest and the trust survives. Replaced with `artifact_digest`, dispatched per source type so it covers all executable bytes: - plugin_home: sha256 of the canonical-ordered tarball of the installed subtree (manifest + entrypoint + assets). - local_path: same canonical hash, but recomputed AT EVERY INVOCATION because the path is mutable. Drift fails closed with `result.status="trust_subject_changed"` and the user must re-issue `trust` to confirm the new artifact. - first_party: sha256 of the entrypoint at boot; rolls with each core release. 3. `disable` semantics undefined (BLOCKING). The prior text used `disable` as the only revocation path for first-party but never said what it does or how to re-enable. Defined `disable` as the shared revocation primitive (third-party AND first-party): - deletes every trust record for the current triple, - sets a durable `disabled: true` flag (lockfile for third-party, persisted override file for first-party), - leaves installed bytes in place. Re-enable is `ooo plugin trust …` on the same plugin, which clears the disabled flag and writes fresh grants bound to the current triple. No separate `enable` verb in v0 — the trust prompt is the only re-grant entrypoint, keeping every decision explicit. Also reformulates the two permission-escalation paths the trust subject defeats: same-name-reinstall-different-source AND code-substitution- under-same-source. Manifest-only binding is explicitly rejected. * docs(rfc): re-verify plugin_home digest per-invocation; widen name collision Round-10 review (commit 549b737): - Resolves the contradiction in the `plugin_home` digest rule. Prior text said "recomputed only at install/add time" AND "later invocations re-verify the snapshot against the recorded digest"; an implementer following the first sentence would never detect on-disk tampering. Pinned the rule: digest is recorded at install AND recomputed before every invocation against the on-disk subtree under `~/.ouroboros/plugins/<...>/`. Drift fails closed with `result.status="trust_subject_changed"`, identical to local_path. first_party stays at boot-time-only (the core release artifact is the unit of trust there, and tampering it is out of scope). - Widens the install-time name collision check. Previously the rule only forbade duplicates between installed third-party plugins, which would have allowed a third-party plugin named `auto`, `run`, or `pm` to hijack the first-party command namespace. The reserved set is now the union of first-party programs, built-in `ooo` subcommands, and installed third-party plugins. Collisions error out with the name of the conflicting occupant; no silent shadowing. * docs(rfc): align trust-subject summary with per-invocation rule for plugin_home Round-11 review (commit 13a5a0f) caught an internal contradiction I missed in round 10: the per-source-type table was updated to require per-invocation re-verification for both plugin_home and local_path, but the trailing "code substitution" summary still said plugin_home is checked at install/boot only. Implementers following only the summary would ship a weaker firewall that misses post-install tampering in ~/.ouroboros/plugins/... Updated the summary so both plugin_home and local_path explicitly require per-invocation re-verification, with the same fail-closed drift behavior. first_party stays boot-only with explicit rationale. * docs(rfc): close historical-trust regrant + boot-time collision gaps Round-12 review (commit b444c31): - BLOCKING: `remove` previously deleted only the *current* triple, leaving an unconfigured regrant path: a user could downgrade or reinstall back to an older digest and inherit prior trust silently. Tightened to delete every trust record whose (source.type, source_identity) matches, regardless of historical artifact_digest. Reinstalling any old version requires fresh `ooo plugin trust ...`. - BLOCKING: name-collision rule was install-time only; a new core release introducing a first-party command that matches an already- installed third-party plugin had no defined behavior. Pinned the boot-time rule: 1) first-party wins dispatch (releases are allowed to ship new commands); 2) the conflicting third-party plugin is auto-disabled (durable `disabled: true`) and its required permissions are stripped from the in-process trust table; bytes stay on disk so the user does not lose data; 3) explicit `plugin.failed result.status="name_collision_with_ first_party"` event + `ooo plugin list` surfaces the disabled state with reason; user resolves by `remove`. No silent shadowing in either direction. * docs(rfc): unify first-party persistence; replace ustar with tree-hash Round-13 review (commit 385bdff) — three findings, all addressed: 1) BLOCKING: first-party trust persistence had three contradictory statements (lockfile / no-lockfile / writes-on-trust). Pinned a single canonical model: - The user lockfile (~/.ouroboros/plugins.lock) holds zero first- party records, ever, in any state. - Durable state for first-party programs lives in a sibling ~/.ouroboros/first-party-overrides.json owned by core, with one entry per (name, artifact_digest) the user explicitly disabled. - Boot looks up (name, digest) in the override file: absent ⇒ implicitly trusted in-process; present ⇒ disabled, permissions stripped. - `disable` writes the entry; `trust` removes it. No lockfile writes for first-party in either path. - Release that rotates the digest invalidates stale overrides; the new program starts trusted by default. 2) BLOCKING: artifact_digest spec used `tar --format=ustar`, which is lossy for long paths and link targets — making trust decisions incorrect for plugins whose bundle exceeds ustar limits. Replaced with a tar-independent "canonical tree hash": - Walk subtree depth-first; collect (mode, path, content-hash) triples for files and symlinks (directories implicit; other file types rejected at install). - Per-entry record `<mode>\0<path>\0<sha256-content-or-link>\0` with no length cap on path. - Lexicographic sort by path; sha256 of concatenated records. Deterministic, covers all executable bytes, no implementation- defined truncation. 3) Non-blocking: "no catalog match" guidance only mentioned `add`; updated to mention both `add <repo-url>` (plugin_home) and `install <name> --from <local-path>` (local_path), so users with a local checkout are not misdirected. * docs(rfc): scope remove to plugin name; align tree-hash wording Round-14 review (commit 84edfb7): BLOCKING: `remove` was scoped only to (source.type, source_identity), which incorrectly aliased every plugin installed from the same plugin_home repo URL. A single catalog repo can host multiple plugins (this RFC's reference catalog itself is structured that way), so `remove github-pr-ops` would silently de-trust sibling plugins from the same repo. Tightened: the lockfile entry shape carries the manifest `name`, so remove's deletion scope is records matching (name, source.type, source_identity) for any historical artifact_digest — scoped to this plugin name only, never spanning siblings. Snapshot removal also restricted to this plugin's own subdirectory under ~/.ouroboros/plugins/<...>/, not the parent catalog. Non-blocking: one stale "canonical-tarball sha256" reference in the permission-escalation summary contradicted the new tar-independent canonical tree hash above. Updated to call out the canonical tree hash explicitly and remind readers that the digest is not a tarball hash. * docs(rfc): make first-party disable durable across upgrades Round-15 review (commit 7f574aa) flagged a real semantic regression in the first-party disable persistence model: keying overrides by (name, artifact_digest) AND saying release-time digest rotation "invalidates stale overrides" meant any ordinary core upgrade would silently re-enable a program the user had explicitly disabled. That makes `ooo plugin disable` non-durable for the most common upgrade path and contradicts the strict revocation model the rest of the doc spells out. Fix: key the override by `name` only. An explicit user disable persists across all upgrades. Re-enable requires an explicit `ooo plugin trust <name>` — the user re-decides — or the program being removed from a future release entirely (in which case the orphaned entry doesn't apply and may be GC'd). The artifact_digest at disable time MAY be recorded as audit metadata but is no longer part of the key. Removed the "stale override no longer matches" wording that produced the silent-re-enable behavior. * docs(rfc): split disable records from trust records (third-party) Round-16 review (commit f704689) identified the same regression I'd just fixed for first-party, applied to third-party. The lockfile rule keyed records by (name, source.type, source_identity, artifact_digest) and attached the `disabled: true` flag to that tuple-keyed row. Any digest change (upgrade, reinstall) created a new triple with no disabled flag, silently re-enabling the plugin. Especially bad for plugins with zero `required: true` permissions, since the pre- invocation trust check would not fire. Fix: split the third-party lockfile into two record kinds: - Trust records — keyed by (name, source.type, source_identity, artifact_digest). Wiped whenever any field of the triple changes. - Disable records — keyed by (name, source.type, source_identity) only, no artifact_digest. Survive every digest change, including upgrades and remove+add cycles that land the same source identity. The firewall MUST consult the disable record before any invocation, independently of whether trust records exist, so a permissions- empty plugin cannot bypass disable. Lifecycle adjustments to match: - `disable` writes the install-subject-keyed disable record AND clears trust rows for any historical artifact_digest. - `trust` clears any disable record AND writes a fresh trust row for the current triple. (Re-enable path is unchanged: trust is the only re-grant entrypoint; no separate `enable` verb.) - `remove` clears trust rows AND deletes the disable record for the install subject — uninstall fully resets the plugin's state.
… Programs (Q00#744) * docs(architecture): split PLUGIN LAYER → Skills Registry vs UserLevel Programs Per Q00#727: 'PLUGIN LAYER' was overloaded — it referred both to the internal Skills/Agents subsystem (Section 1 of this doc) and, after Q00#725, to the user-installable UserLevel plugin layer. Without a rename, PRs touching either subsystem confused the term. Changes: - Diagram top-left box: 'PLUGIN LAYER' → 'SKILLS REGISTRY' - Section 1 heading: 'Plugin Layer' → 'Skills & Agents Registry' (plus a Note: pointing readers at Section 7 and the RFC) - New Section 7: 'UserLevel Programs Layer' — describes the user-installable plugin layer introduced in Q00#725, with its own sub-diagram, distinguishes it from Section 1, links to the canonical RFC at docs/rfc/userlevel-plugins.md, and adds a 'Terminology guard' note recommending qualified terms in PRs ('skill plugin' vs 'UserLevel plugin'). Closes Q00#727. * docs(architecture): address ouroboros-agent review on PR Q00#744 Two findings from the bot review on commit bca5a06: 1. Dead-link: the doc twice cited `docs/rfc/userlevel-plugins.md` as the "canonical" reference, but that file does not exist on main yet — the RFC is being drafted in PR Q00#743 (still in review). Repoint both references at issue Q00#725 (the design issue, which contains the layer-model inline) and forward-reference Q00#743 with explicit "in flight" status, so this PR is self-contained at merge time. 2. Top diagram only showed Sections 1–6, while Section 7 was added in prose. This also mismatched issue Q00#727 acceptance criterion #1 ("diagram has both layers visually distinct, with no shared label"). Prepend a top-of-stack USERLEVEL PROGRAMS LAYER box (installable + first-party) connected to the existing core block via a "declared manifest contract" arrow. Closes the bot's blocking-but-downgraded follow-up, the non-blocking diagram suggestion, and Q00#727 AC#1. * docs(architecture): mark third-party plugin install surface as planned Bot follow-up on commit f1243ce flagged that Section 7 documented `ooo plugin add <repo-url>` as if it were a current command surface, but that CLI does not exist on `main` yet — only first-party programs (`ooo auto`, `ooo run`, `ooo pm`) ship today. Reword Section 7 so it clearly distinguishes shipped first-party programs from the planned third-party plugin install surface tracked under Q00#725. The architectural target is preserved so future PRs can reference consistent terminology, but readers no longer get the impression that `ooo plugin add` already works. Closes the bot's only remaining follow-up finding on PR Q00#744. * docs(architecture): rename inner box to SKILLS & AGENTS REGISTRY Cosmetic follow-up on commit e90ef9d4: the section heading was already "Skills & Agents Registry" but the inner ASCII box read just "SKILLS REGISTRY", leaving a small terminology inconsistency between the diagram label and the prose contents (the box documents both skills and agents). Use the same 2-line label treatment that the "PRESENTATION / LAYER" box already uses. Closes the bot's only remaining non-blocking suggestion on PR Q00#744.
* feat(plugin): manifest loader + vendored 0.1 schemas Implements Q00#728 (locked spec). src/ouroboros/plugin/manifest.py: - PluginManifest frozen dataclass with the locked 8 required + 2 optional fields (per Q00/ouroboros-plugins#6). - load_manifest(path) returns a frozen, validated manifest. - PluginManifestError exposes structured fields: path, json_pointer, expected, got — reviewers/tests assert on json_pointer rather than parsing message text. - source.type=first_party is a real branch (per Q00/ouroboros-plugins#8); source.path/repository optional for it. - schema_version routes to schemas/<major>/plugin.schema.json (per Q00/ouroboros-plugins#11). Out-of-window versions reject with a clear message naming the supported window. src/ouroboros/plugin/schemas/0.1/{plugin,audit-event}.schema.json: - Vendored copies of the upstream Q00/ouroboros-plugins/schemas/0.1/ schemas, reflecting all locked decisions: * 8 required + 2 optional (#6) * 3-value risk enum (#10) * per-major archive (#11) - schemas/0.1/_source.json records the upstream repo + sync intent. A scripts/sync-plugin-schemas.sh follow-up will keep these fresh once the upstream PRs land on main. pyproject.toml: - jsonschema>=4.21,<5.0 added as a runtime dependency. tests/unit/plugin/test_manifest.py: - 13 tests including the 11 specified in Q00#728's spec, plus 2 bonus cases (invalid JSON, missing file). Each error-case test asserts the exact JSON Pointer to the failing field. All 13 tests pass: PYTHONPATH=src python3 -m pytest tests/unit/plugin/test_manifest.py -v Sub-issue of Q00#725 (Phase 1). Closes Q00#728. * chore(plugin): satisfy ruff (I001 import order, UP037 quoted annotation) Resolves the three Ruff Lint failures on PR Q00#745: - src/ouroboros/plugin/manifest.py:25 — I001 import block ordering - src/ouroboros/plugin/manifest.py:131 — UP037 unnecessary quotes on `-> "AuditSpec"` (the surrounding `from __future__ import annotations` already makes the annotation lazy) - tests/unit/plugin/test_manifest.py:7 — I001 import block ordering No behavior change. `pytest tests/unit/plugin/test_manifest.py` still 13/13. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(plugin): require path on local_path/plugin_home sources Addresses ouroboros-agent[bot]'s BLOCKING finding on PR Q00#745: the 0.1 schema only required `source.type`, so a manifest like `{"source": {"type": "local_path"}}` would validate and the loader would return `SourceSpec(path=None)`. That left a manifest with no usable location for a local-path or plugin-home plugin and pushed the failure into downstream code instead of rejecting it at load time. Schema changes (`src/ouroboros/plugin/schemas/0.1/plugin.schema.json`): - `source.path` and `source.repository` now require `minLength: 1` (an empty string is no more useful than a missing one). - An `allOf` with `if`/`then` makes `path` required when `type` is `local_path` or `plugin_home`. `first_party` keeps its no-extra-required-fields branch (per Q00/ouroboros-plugins#8). Test additions (`tests/unit/plugin/test_manifest.py`): - `test_local_path_source_requires_path` — negative case for the exact contract hole the bot called out. - `test_plugin_home_source_requires_path` — symmetric guard for the other path-bearing source type. - `test_local_path_source_with_path_loads` — positive control next to the negative cases so a future schema regression that breaks the happy path is caught alongside the negative guards. `PYTHONPATH=src uv run python3 -m pytest tests/unit/plugin/test_manifest.py` → 16/16 pass. Ruff clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(plugin): normalize I/O failures into PluginManifestError Addresses ouroboros-agent[bot]'s follow-up finding on PR Q00#745: the loader's structured-error contract leaked raw `OSError` and `UnicodeDecodeError`, so callers had to know to catch `PluginManifestError` *and* the underlying I/O exceptions. That defeats the purpose of the single error type the loader is introducing. Changes: - `load_manifest()` now wraps `UnicodeDecodeError` (non-UTF-8 manifest bytes) and `OSError` (permission denied, transient FS errors, TOCTOU between `is_file()` and `open()`) into `PluginManifestError`. - `_load_schema()` now wraps `OSError`, `UnicodeDecodeError`, and `JSONDecodeError` from the vendored schema read. The raised error's `path` field still points back at the manifest the caller asked about, not at an internal vendored file the user cannot fix — callers always get a stable address. Test additions (`tests/unit/plugin/test_manifest.py`): - `test_non_utf8_manifest_reports_structured_error` — feeds raw bytes that are not valid UTF-8 and asserts a `PluginManifestError` with the right path. - `test_unreadable_manifest_reports_structured_error` — POSIX `chmod 0o000` on a real manifest, asserts the loader normalizes the resulting `PermissionError` into `PluginManifestError`. Skipped on Windows and when running as root (where chmod 0o000 cannot deny reads). `PYTHONPATH=src uv run python3 -m pytest tests/unit/plugin/test_manifest.py` → 18/18 pass. Ruff clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(plugin): apply ruff format to manifest.py Restores `ruff format --check src/ tests/` to clean. The CI Lint job runs both `ruff check` and `ruff format --check` and the prior commits satisfied the former but not the latter. No behavior change. Tests still 18/18 pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(plugin): sandbox source.path + ship schemas as packaged resources Addresses ouroboros-agent[bot]'s second-round review on commit 83241eb: **BLOCKING — `plugin.schema.json:43`, sandboxed `source.path`** The previous round's `minLength: 1` was not enough: a `plugin_home` (or `local_path`) manifest could still pass validation with `/etc/passwd` or `../../foo`, turning a downstream `os.path.join` into a sandbox escape. - New `_validate_sandboxed_path()` helper rejects absolute paths and any `..` segment for `local_path` and `plugin_home` source types, raising `PluginManifestError` with `json_pointer="/source/path"`. - 10 parametrised negative tests (5 traversal shapes × 2 source types) plus a `plugin_home`-with-relative-path positive control. **FOLLOW-UP — `manifest.py:171`, packaged-resource loading** `_load_schema()` was reading via a hard-coded filesystem path (`Path(__file__).parent / "schemas"`), which depended on hatchling's implicit asset handling. The same `pyproject.toml` already force-includes `ouroboros/opencode/plugin` for exactly this reason — the implicit handling is not reliable for non-Python assets. - `_load_schema()` now resolves the schema through `importlib.resources.files("ouroboros.plugin.schemas")`, which works whether the package is installed as a wheel, an editable install, or a zipapp. This also closes the round-2 non-blocking refactor suggestion on `manifest.py:45`. - `pyproject.toml` adds `src/ouroboros/plugin/schemas/**` to the wheel exclude list and `src/ouroboros/plugin/schemas` to `force-include`, mirroring the existing `opencode/plugin` pattern. - `test_vendored_schemas_are_packaged_resources` asserts the schema resource is reachable via `importlib.resources` and parseable, so a future refactor that drops the `force-include` line fails a unit test instead of every plugin load in production. - Verified locally: `uv build --wheel` produces a single ZIP entry for each of `0.1/plugin.schema.json`, `0.1/audit-event.schema.json`, and `0.1/_source.json` — no duplicate ZIP local headers. `PYTHONPATH=src uv run python3 -m pytest tests/unit/plugin/test_manifest.py` → 30/30 pass. Ruff lint + format clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(plugin): reject Windows path forms in sandboxed source.path Addresses ouroboros-agent[bot]'s round-3 BLOCKING finding on commit 4ded005: `_validate_sandboxed_path()` parsed `source.path` only with `PurePosixPath`, so Windows escape forms were accepted as "relative" even though a downstream consumer running on Windows would resolve them as absolute or as parent traversal: - Drive-qualified paths: `C:/Windows/System32`, `c:foo` - Backslash traversal: `..\escape`, `foo\..\bar`, `C:\Windows` The fix is platform-agnostic because the host that loads the manifest is not necessarily the host that resolves it. The loader cannot rely on the local OS's path semantics. Changes (`src/ouroboros/plugin/manifest.py`): - `_validate_sandboxed_path()` now rejects, in this order: 1. Any backslash anywhere in the path (POSIX slugs only). 2. Windows drive prefix `^[A-Za-z]:`. 3. POSIX absolute leading `/`. 4. Any `..` segment (split on `/`, since backslashes were already rejected). - `PurePosixPath` import dropped — the new logic is plain string inspection, which keeps the validation deterministic and avoids pulling in `pathlib`'s platform-specific quirks (e.g. `PurePosixPath` does not see backslashes; `PureWindowsPath`'s `is_absolute()` returns False for `c:foo` and `/foo`). Test changes (`tests/unit/plugin/test_manifest.py`): - `test_sandboxed_source_path_rejects_traversal` parametrisation expanded from 5 to 10 bad paths — adds 5 Windows escape forms. 10 bad paths × 2 source types = 20 negative cases covering both POSIX and Windows traversal syntaxes. `PYTHONPATH=src uv run python3 -m pytest tests/unit/plugin/test_manifest.py` → 40/40 pass. Ruff lint + format clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(plugin): cover ImportError and add real wheel-build packaging guard Addresses ouroboros-agent[bot]'s round-4 BLOCKING findings on commit 2994718: **1. `_load_schema()` leaks raw `ModuleNotFoundError` (manifest.py:182)** `importlib.resources.files("ouroboros.plugin.schemas")` raises `ModuleNotFoundError` (a subclass of `ImportError`) if the namespace package itself is missing from the installed wheel — the same root cause the round-2 follow-up flagged, just one layer earlier than `FileNotFoundError`. Without an explicit handler, the loader leaks the raw import exception, breaking the structured-error contract this PR introduces. `_load_schema()` now catches `(ModuleNotFoundError, ImportError)` and re-raises as `PluginManifestError` with `json_pointer="/schema_version"` and a message that names the missing package, matching every other loader failure path. **2. The packaging unit test cannot catch real packaging breakage (test_manifest.py:315)** `test_vendored_schemas_are_packaged_resources` resolves `importlib.resources` against the editable source tree, so it passes even when `pyproject.toml` is misconfigured and a real wheel build would drop the schemas. The test cannot fail for the failure mode it claims to guard. New `tests/integration/plugin/test_wheel_packaging.py`: - `test_built_wheel_ships_vendored_schemas` runs `uv build --wheel` and inspects the shipped archive, asserting that `ouroboros/plugin/schemas/<version>/{plugin,audit-event}.schema.json` appears exactly once (no duplicate ZIP local headers, which PyPI rejects) for every `SUPPORTED_SCHEMA_VERSIONS` entry. - Skips if `uv` is not on `PATH` so isolated environments do not flake. The original unit-level `test_vendored_schemas_are_packaged_resources` stays — it is a fast smoke check for code-level reachability — and the new integration test is the actual install-time guard. Together they cover the unit/installed split the bot called out. `PYTHONPATH=src uv run python3 -m pytest tests/unit/plugin/test_manifest.py tests/integration/plugin/test_wheel_packaging.py` → 41/41 pass. Ruff lint + format clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…0#754) * fix(providers,interview): isolate subprocess from host plugin env ClaudeCodeAdapter subprocesses spawned from an MCP server context inherit the host plugin set and run with permissive tool privileges, which lets the child invoke ouroboros_interview recursively, hit --max-turns 1, and exit with code 1 (stderr empty). Three coupled root causes: 1. CLAUDE_PLUGIN_DATA / CLAUDE_PLUGIN_ROOT are inherited via os.environ. Claude Code strips CLAUDECODE when launching MCP servers but leaves the plugin env intact, so the child loads the same plugin set (including ouroboros) and exposes mcp__plugin_ouroboros_* tools. 2. InterviewHandler.handle() used ``self.llm_adapter or create_llm_adapter(...)``. The shared adapter injected at server startup is non-None and permissive (allowed_tools=None), so the read-only whitelist from _interview_allowed_tools() was dead code. 3. The standalone InterviewHandler in mcp/server/adapter.py also had ``interview_engine`` injected; the engine carries the same shared permissive adapter, which silently bypasses any handler-side adapter rebuild. The AutoHandler-internal InterviewHandler in the same wiring already does not inject the engine — this restores that consistency. All three changes are required (verified by ablation): - env isolation alone leaves --allowedTools unrestricted, - adapter rebuild alone is overridden by the injected engine, - dropping engine injection alone still inherits the host plugin set. * fix(providers,interview): pass strict_mcp_config when allowed_tools is pinned When ClaudeCodeAdapter spawns its SDK subprocess from inside a Claude Code MCP server context, the child auto-loads plugin-provided MCP servers and registers their tools (notably ouroboros's own mcp__plugin_ouroboros_*). The subprocess can then call ouroboros_interview recursively, hit --max-turns 1 immediately, and exit with code 1 (stderr empty). When the caller pins an explicit read-only tool envelope via allowed_tools (the interview path uses _interview_allowed_tools()), also forward strict_mcp_config=True to the SDK. This maps to the CLI's --strict-mcp-config flag, restricting the child to MCP servers we pass explicitly and ignoring plugin-provided servers, so the recursion path is closed without touching plugin env, hooks, or composition-root wiring. * fix(providers): scope strict_mcp_config to interview policy path Addresses ouroboros-agent[bot] BLOCKING on d2b2ff6: applying strict_mcp_config=True to every explicit allowed_tools envelope silently broke the existing adapter contract — sessions that legitimately pinned MCP names like ``mcp__ouroboros__qa`` would lose access to plugin/project MCP servers at runtime. Narrow the gate to the actual offender. Recursion into ``ouroboros_interview`` is only triggered by the interview policy path; other explicit envelopes do not require MCP isolation. Changes: - ``ClaudeCodeAdapter`` now takes an explicit ``strict_mcp_config`` flag (default ``False``). The SDK option is forwarded only when the flag is set, decoupled from the ``allowed_tools is not None`` branch. - ``create_llm_adapter`` enables ``strict_mcp_config`` for the ``claude_code`` backend exclusively when ``use_case == "interview"``. - Tests: - ``test_explicit_allowed_tools_sets_visible_sdk_tools`` now asserts ``strict_mcp_config`` is **not** present on the SDK options for generic envelopes (regression-locks the contract the bot flagged). - New ``test_strict_mcp_config_opt_in_forwards_to_sdk`` proves the flag flows through when explicitly enabled. - New ``test_default_tool_policy_does_not_set_strict_mcp_config`` proves the default path leaves plugin MCP servers reachable. - New factory tests prove ``use_case="interview"`` ⇒ ``strict_mcp_config=True``, default ⇒ ``False``. Recursion fix from d2b2ff6 is preserved end-to-end: the interview MCP tool still reaches the adapter through ``create_llm_adapter`` with ``use_case="interview"``, which keeps the SDK isolation in place. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(providers): scope strict_mcp_config to MCP entrypoint, use SDK extra_args Addresses ouroboros-agent[bot] BLOCKING ×2 on a0168cb: (1) Scope was still too broad. ``use_case="interview"`` is also taken by CLI interview entrypoints (``ooo init`` / ``ooo pm``) where recursion is impossible. Auto-enabling strict isolation for every interview-use-case adapter would make those flows silently ignore plugin/project ``.mcp.json`` servers — a regression for brownfield interviews that rely on repository-provided MCP tools. Move opt-in from the factory's ``use_case`` derivation to the actual nested entrypoint: ``InterviewHandler.handle()`` in ``mcp/tools/authoring_handlers.py``. That handler is the only path where the spawned subprocess can rediscover plugin-provided ouroboros and recurse. CLI entrypoints leave ``strict_mcp_config`` at its default ``False``. (2) SDK compatibility. ``ClaudeAgentOptions`` does NOT expose ``strict_mcp_config`` as a typed kwarg in the supported pin range (``claude-agent-sdk>=0.1.0,<1.0.0``); even the currently installed v0.1.50 lacks it. Forwarding it unconditionally would raise ``TypeError`` at ``ClaudeAgentOptions(**options_kwargs)`` and break every Claude interview at runtime — the prior tests didn't catch this because they mock the Options class. Introspect ``ClaudeAgentOptions`` once and forward ``strict-mcp-config`` via ``extra_args`` (the canonical CLI-flag passthrough surface). If a future SDK adds a typed ``strict_mcp_config`` field, that is preferred. If the SDK exposes neither surface, log a warning and skip the flag (degraded but safe — recursion remains possible, but ``ClaudeAgentOptions`` construction does not crash). Changes: - ``ClaudeCodeAdapter``: - New module-level ``_claude_options_field_names`` (lru-cached ``inspect.signature`` lookup). - Forward strict isolation via ``extra_args["strict-mcp-config"]`` when supported; fall back to typed kwarg if the SDK adds one; skip with warning if neither is supported. - ``create_llm_adapter``: drops the ``use_case``-based auto-derivation; ``strict_mcp_config`` is now a direct passthrough parameter that defaults to ``False`` and is only forwarded for the ``claude_code`` backend. - ``InterviewHandler.handle()`` (the MCP nested entrypoint) opts in by passing ``strict_mcp_config=True`` to ``create_llm_adapter``. - Tests: - Factory: prove ``use_case="interview"`` alone does NOT enable strict (CLI regression-lock); prove explicit ``strict_mcp_config=True`` is forwarded; prove default is False. - Adapter: prove ``extra_args`` is used when only ``extra_args`` is available; prove typed ``strict_mcp_config`` is preferred when present; prove safe no-op when neither is available; lock regression that generic explicit envelopes never set either. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(providers): fail fast when claude-agent-sdk cannot apply --strict-mcp-config Addresses ouroboros-agent[bot] BLOCKING on d228ad7: The previous fallback path silently skipped ``--strict-mcp-config`` and logged a warning when the installed claude-agent-sdk exposed neither a typed ``strict_mcp_config`` field nor an ``extra_args`` passthrough. That left ``InterviewHandler.handle()``'s opt-in call site re-exposed to the very ``ouroboros_interview`` recursion this PR is closing — without any actionable failure. Replace the silent skip with a fail-fast ``ProviderError``: - The error message tells operators exactly what to do (upgrade claude-agent-sdk to a release that exposes ``extra_args`` so ``--strict-mcp-config`` can be passed through). - ``ClaudeAgentOptions`` is not constructed when isolation cannot be honored, so the failure is surfaced at the policy boundary rather than as a downstream ``TypeError``. - ``error_type="ConfigurationError"`` makes the error non-retryable — the existing retry classifier only treats ``ProcessError``/``CalledProcessError`` exits with empty stderr as retryable, and message-pattern matching does not hit ``"strict-mcp-config"``-related strings. The previous test that locked the silent-skip behavior is replaced with one that asserts ``ProviderError`` is raised, the error_type is ``ConfigurationError``, and ``ClaudeAgentOptions`` is never constructed in this state. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(interview): lock strict_mcp_config wiring at the MCP entrypoint Addresses ouroboros-agent[bot] non-blocking suggestion on 5a203d0: the nested-MCP recursion fix was previously covered only indirectly through factory and adapter unit tests. This adds a focused regression lock that exercises ``InterviewHandler.handle()`` directly and asserts the factory call kwargs include ``strict_mcp_config=True`` on the nested entrypoint path. Pairs with ``test_factory.py``'s ``test_claude_code_interview_use_case_does_not_auto_enable_strict_mcp_config`` which proves CLI interview entrypoints (``ooo init`` / ``ooo pm``) do NOT receive the flag, so the symmetric guarantee is now locked end to end: - nested MCP interview ⇒ ``strict_mcp_config=True`` - CLI / other interview entrypoints ⇒ default ``False`` Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(providers): lock live-SDK extra_args invariant; clarify fail-fast scope Addresses ouroboros-agent[bot] BLOCKING on f0a13c3: The bot warned that the fail-fast branch could trigger for SDK versions in the declared support range ``>=0.1.0,<1.0.0`` that lack ``extra_args``, breaking previously supported installs. Verified empirically against the published PyPI history that ``extra_args`` has been a field on ``ClaudeAgentOptions`` since the earliest public release of ``claude-agent-sdk`` (``0.0.23``) and is therefore present at every version in the declared range. There is no in-range version where the fail-fast branch can actually fire on a pip-installed SDK; it is defense-in-depth for vendored, partial, or monkey-patched builds where the field has been removed. Lock that invariant in CI so any future floor bump or vendored SDK swap that drops ``extra_args`` fails at test time rather than at runtime in production: - New ``test_live_claude_agent_sdk_supports_extra_args`` introspects the actual installed ``ClaudeAgentOptions`` and asserts ``extra_args`` is in its parameter list. Clears the ``lru_cache`` on ``_claude_options_field_names`` to stay independent of monkeypatch fixtures elsewhere in the module. - Adapter source comment is expanded to cite the verified PyPI floor and document why the fail-fast branch is defense-in-depth, not a contract change against the declared support range. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(providers): skip live-SDK invariant when claude-agent-sdk extra is absent Addresses ouroboros-agent[bot] BLOCKING on bda1a98: The live-SDK invariant test introduced in the previous commit hard- required ``claude-agent-sdk`` to be installed. ``claude`` is a declared optional extra (``ouroboros-ai[claude]``) and the rest of this file mocks ``sys.modules['claude_agent_sdk']`` precisely so the test suite does not depend on the runtime extra. Guard the test with ``pytest.importorskip("claude_agent_sdk")`` so it gracefully skips on installs without the extra. When the extra IS installed, the invariant still fails fast in CI if a future SDK build drops ``extra_args`` — the protection the previous commit set out to provide is preserved. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: shaun0927 <70629228+shaun0927@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(auto): CI lint guard for ooo auto product boundary Implements consolidated Q00#734 + Q00#735 (locked spec). Branched off main directly for clean independent diff; the documentary RFC text in Q00#743 (CR-1) lives elsewhere and pairs with this script. Verified pre-flight: at the time of this commit, grep -nE '\bgithub\b|\bpull_request\b|/pulls?/' on the watched ooo auto source set returns empty. The boundary that Q00#725 protects is intact; the closed Q00#689 PR stack (Q00#697, Q00#707, Q00#712, Q00#715, Q00#721) confirms the de facto position. scripts/check-auto-boundary.py: - Greps a watched set of 9 ooo auto source files for forbidden domain keywords (github, pull_request, /pulls/, api.github.com, jira, slack, linear.app, github_pr). - Exits non-zero with a per-finding report showing the file, line number, matched pattern, and the offending line. Provides the fix-up hint (allowlist marker) and points at Q00#725 for context. - Allowlist marker `# domain-keyword-allowed: <reason>` lets a line genuinely needing a forbidden keyword (rare; usually a legacy import) opt out — each usage is reviewer-signoff. - Self-tests live alongside (see below) so the script's correctness is its own merge gate. .github/workflows/auto-boundary.yml: - Runs the script on every pull_request to main and on push to main. Can be made a required check in repo settings (flagged for maintainer in PR description). tests/unit/scripts/test_check_auto_boundary.py — 5 tests: - main repo passes via subprocess (clean-state invariant) - synthetic offender caught - allowlist marker bypasses - missing watched file does not error - meta-test: each forbidden pattern catches a representative offender (so adding a pattern to FORBIDDEN_PATTERNS takes effect without wiring code) CONTRIBUTING.md: - Adds the boundary check to the "Lint and Format" step with the rationale and the allowlist marker syntax. All 5 self-tests pass; the guard runs clean on the current main: $ python3 scripts/check-auto-boundary.py ooo-auto-boundary: OK (9 files scanned, 0 findings) Sub-issue of Q00#725 (Phase 4). Closes Q00#734 and Q00#735 (consolidated). * fix(auto): harden ooo-auto-boundary guard per bot review Address the two BLOCKING findings from ouroboros-agent[bot] on PR Q00#753: 1. Forbidden-keyword regex bypass via identifier embedding. The original `\bgithub\b`, `\bjira\b`, `\bslack\b` patterns silently skipped realistic Python identifier forms — `GitHubClient`, `github_client`, `JiraIssue`, `SlackNotifier`, `FOO_GITHUB_BASE` — because Python identifier characters are all word-class. Replace word-boundary regex with case-insensitive substring matching, which catches PascalCase, camelCase, snake_case, and SCREAMING_SNAKE_CASE forms uniformly. Verified zero false positives on current main across all 16 watched files. 2. Silent-failure mode on missing watched files. The original guard treated a missing watched file as a clean pass: a refactor that renamed or removed a load-bearing core file would silently strip enforcement coverage while CI stayed green. Split the file list into ANCHOR_FILES (must-exist; missing => fail loud) and SCAN_DIRS / SCAN_EXTRA_FILES (the actual scan target). Also addresses the bot's design note about hand-maintained list brittleness: the scan target is now the *union* of every `*.py` under `src/ouroboros/auto/` plus the explicit cli/commands/auto.py extra, so any new module added under the auto package is covered automatically without a manual list update. The watched-file count grows from 9 to 16 on current main; ANCHOR_FILES retains the 9 load-bearing files as required existence anchors. Tests: 17 cases (was 5), covering the original behaviors plus the new identifier-form catch surface, fail-loud-on-missing-anchor, auto- discovery of new files, docstring scanning, and SCAN_EXTRA_FILES coverage. Existing 5 self-test invariants are preserved. Doc: CONTRIBUTING.md describes the union-discovery, identifier-form matching, and ANCHOR_FILES contract. Lint: fixes the I001 import-sort that blocked Ruff CI on the previous commit (force-sort-within-sections semantics). Refs: Q00#725, Q00#734, Q00#735 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auto): close compressed-form gap in boundary guard Bot review on e75eabe flagged a residual bypass: PullRequestHandler / pullRequestId are not caught by the substring `pull_request` (the underscore breaks contiguity when matching against an underscore-stripped identifier), and LinearClient / LinearAdapter are not caught by `linear.app` (URL form only). Add `pullrequest` (camelCase compressed form) alongside `pull_request`, and replace `linear.app` with bare `linear` (which also subsumes the URL form). Verified zero false positives on the actual scan target (no `linear` or `pullrequest` substrings in `src/ouroboros/auto/` or `src/ouroboros/cli/commands/auto.py` on current main). Tests: extend FORBIDDEN_PATTERNS sample mapping and the parametrized identifier-form fixtures to cover PullRequestHandler, pullRequestId, PullRequestBase, LinearClient, LinearAdapter, and linear_client (23 cases total, was 17). Refs: Q00#725 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auto): honor allowlist marker only inside real comments Bot review on 841d267 found a real bypass: ALLOWLIST_MARKER was checked with naive substring containment, so a forbidden keyword on a line whose only marker lives inside a string literal -- e.g. ``MSG = "domain-keyword-allowed: github"`` -- silently passed the guard. The contract documented in CONTRIBUTING.md and the script docstring promised "trailing # domain-keyword-allowed comment", but the implementation honored the marker anywhere on the line. Tokenize each scanned file with the stdlib ``tokenize`` module and only honor the marker when it appears inside a ``COMMENT`` token. On tokenize failure (malformed Python), default to NO allowlist for that file, which is the safer default for a boundary guard. Tests: add three cases: - marker inside a string literal does NOT bypass (regression) - marker in a real trailing ``#`` comment still bypasses (preserves contract for legitimate use) - line where the marker is in a string and a separate non-marker comment exists does NOT bypass Refs: Q00#725 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auto): tighten Linear pattern to avoid false positives Bot review on 9085d80 flagged that the bare ``linear`` substring introduced in the previous iteration was overly broad: it would falsely reject benign code like ``linear_time``, ``linearize``, and docstrings such as "linear pipeline" / "linear scan", turning the guard into a CI-noise generator. Switch FORBIDDEN_PATTERNS from substring matching to Python regexes with inline ``(?i:...)`` flags so each pattern can mix case-insensitive and case-sensitive sub-matches as needed. Tighten the Linear-the-product entry to require one of: * the URL form ``linear.app`` / ``linear.com`` (case-insensitive); * a PascalCase composition ``(?i:linear)[A-Z]`` -- the Linear prefix is case-insensitive but the trailing letter is case-sensitive uppercase, which discriminates ``LinearClient`` from ``linearize`` / ``linear_time``; * an explicit integration suffix in snake_case (``linear_client``, ``linear_webhook``, ``linear_notifier``, ``linear_adapter``, ``linear_api``, ``linear_sdk``, ``linear_service``, ``linear_integration``, ``linear_hook``, ``linear_bot``, ``linear_messenger``, ``linear_action``). Also collapse the old separate ``/pulls/`` and ``/pull/`` into a single ``(?i:/pulls?/)`` regex. Tests: add 6 false-positive guards (``linear_time``, ``linearize``, "linear pipeline", "linear scan" docstrings, ``STEP_LINEAR``, string literal ``'linear'``) and 9 positive-match cases (PascalCase composition, integration suffixes, URL forms with case variations). Total now 41 cases (was 26). Refs: Q00#725 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auto): anchor every keyword to a left letter-boundary Bot review on 3f29cf0 flagged that the Linear regex without a left boundary still matches benign identifiers that merely contain ``linear`` -- ``collinearPoints``, ``bilinearForm``, ``nonlinearAdapter``, ``nonlinear_adapter``. Same class of bypass applies in principle to the other keywords (``mygithub_thing``, ``mypull_request_data``, ``myslack_data``). Anchor every FORBIDDEN_PATTERNS entry with ``(?<![A-Za-z])`` (negative lookbehind: not preceded by a letter). This rejects letter-embedded forms while still accepting the legitimate snake_case-composition catches the bot earlier asked us to keep -- ``notify_slack_user``, ``FOO_GITHUB_BASE``, ``linear_client``, ``github_client`` -- because ``_`` is not a letter under that lookbehind. We deliberately do NOT use ``\b`` here: ``\b`` treats ``_`` as a word character, which would silently drop those snake_case catches. Tests: extend the false-positive parametrize fixture with 9 new embedded-substring guards (``collinearPoints``, ``bilinearForm``, ``nonlinearAdapter``, ``nonlinear_adapter``, ``mygithub_thing``, ``myjira_data``, ``myslack_data``, ``mypull_request_data``, ``mypullrequest_handler``). 50 cases total (was 41); all pass. Refs: Q00#725 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auto): catch camelCase-compound keyword leaks Earlier iterations of the boundary guard only matched a forbidden keyword when it sat at a token start (preceded by a non-letter), so mid-identifier camelCase compounds bypassed the check: openGithubIssue() makePullRequestUrl() sendToSlack(msg) openJiraIssue() notifyLinearAdapter() Each keyword now carries two recognition arms: 1. Token-start: ``(?<![A-Za-z])(?i:<kw>)`` -- unchanged. 2. camelCase-boundary: ``(?<=[a-z])<KW first-letter capitalized>(?i:<rest>)`` -- catches ``Github`` after a lowercase letter while keeping ``mygithub_thing`` benign (the keyword's first letter is lowercase there, so the case-sensitive uppercase in the new arm does not match). Test sweep extended with 8 new camelCase-compound positive cases covering Github, PullRequest, Slack, Jira, and Linear. Existing embedded-substring negatives (``mygithub_thing``, ``collinearPoints``, ``nonlinear_adapter``, ...) still pass. 58/58 tests green; clean main still scans clean (16 files, 0 findings). * fix(auto): tighten Linear arm to integration-suffix allowlist Bot follow-up review on 293d87c flagged that the Linear arm ``(?<![A-Za-z])(?i:linear)[A-Z]`` accepts any uppercase letter after ``linear``, which rejects benign generic compounds (``linearTime``, ``LinearTime``, ``linearTransform``, ``LinearOperator``, ``LinearRegressor``, ``LinearProgrammingSolver``) as if they referenced Linear-the-product. Although non-blocking, that false-positive class would fail unrelated PRs in CI and turn the boundary guard into noise that contributors allowlist around. Replaced both Linear arms (token-start and camelCase-boundary) with an enumerated integration-suffix allowlist already used by the snake_case form: ``client|adapter|auth|api|webhook|sdk|service| integration|notifier|hook|bot|messenger|action``. The optional underscore (``linear_?<suffix>``) collapses snake_case, PascalCase, camelCase, and SCREAMING_SNAKE forms into one expression. Real Linear-integration forms (``LinearClient``, ``LinearAdapter``, ``LinearAuth``, ``linearClient``, ``linear_webhook``, ``LINEAR_NOTIFIER``, URL forms) are still caught; benign ``linear<English noun>`` compounds no longer false-positive. Tests extended with 8 new false-positive guards (``linearTime``/``LinearTime``/``linearTransform``/ ``LinearTransform``/``LinearOperator``/``LinearRegressor``/ ``linearScan``/``LinearProgrammingSolver``). 66/66 pass; clean main still scans clean. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: shaun0927 <support@omofictions.com>
* test(auto): define interview convergence contract * test(auto): align stalled-loop contract test with documented What else? regression Re-target the stalled gap-reduction regression test on the contract path the docs actually pin: a generic ``What else?`` follow-up loop that exhausts ``max_rounds`` without filling required ledger sections. The previous ``test_convergence_contract_stalled_unsafe_loop_reports_actionable_gaps`` named "unsafe loop" but used a benign-sounding ``What production credentials should be configured?`` prompt that does not match the unsafe ``_blocker_for`` patterns; renaming and reshaping the test removes that mismatch and pins the stall-signal regression that the convergence contract describes. Also tightens the regression-coverage section of ``docs/auto-interview-convergence-contract.md`` so the broad-benign bullet matches what the test actually exercises (safe auto assumptions only, no bounded repo facts) and explicitly states the documented unsafe-blocker reason and the round-cap unresolved-section blocker behaviour. Validation: - ``PYTHONPATH=src python -m pytest tests/unit/auto/test_ledger_grading_answerer.py tests/unit/auto/test_interview_pipeline.py -q`` -> 163 passed - ``ruff check tests/unit/auto/test_interview_pipeline.py docs/auto-interview-convergence-contract.md`` -> clean Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Hermes Agent <hermes-agent@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(auto): steer interviews toward open ledger gaps
* fix(auto): preserve specific answers and same-turn ledger repair
Address two design issues in the gap-steering interview driver:
- Restore broad-prompt detection as the gating signal so backend-
specific answers (e.g. follow-up acceptance/verification questions)
are not silently replaced by gap-targeted fallbacks when they don't
shrink the required-gap set in this turn. Steering now triggers only
when the answer is a repeated generic default or the prompt itself
is broad ("anything else?" / "what else should we know?").
- Run the same-turn repair check (does this answer actually reduce
open required gaps?) before raising a hard blocker on the first
detected gap. Previously, a CONFLICTING/BLOCKED first gap would
short-circuit the loop even when the current answer would resolve
it, leaving resumed/persisted ledgers stuck in `blocked`.
`goal` remains an unconditional blocker: it can never be filled
automatically and must come from the user.
Adds a regression test covering two specific feature/acceptance
prompts while other required sections remain open, asserting the
second reply still answers the asked question rather than being
overridden by a gap-targeted fallback.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(auto): distinguish generic-default route from feature-specific answers
Bot review on b30be46 flagged that ``_is_repeated_default_answer``
classified every ``CONSERVATIVE_DEFAULT`` answer as a repeated generic
fallback. Feature-specific helpers (``_feature_acceptance_answer``,
``_runtime_answer``, ``_io_actor_answer`` …) also use that source, so a
backend that asked the same specific acceptance question twice would
have the second reply silently swapped for an unrelated gap-fill —
exactly the regression the previous commit set out to prevent.
Add a ``generic_default`` boolean to ``AutoAnswer`` (default ``False``)
and set it ``True`` only on the catch-all ``_default_answer`` route.
``_is_repeated_default_answer`` now keys off this flag instead of the
answer source, so repeated specific follow-ups are preserved while the
broad-prompt convergence pressure for generic defaults is unchanged.
Adds a regression test covering the bot's exact scenario: same
feature/acceptance question asked twice with other required ledger
sections still open, asserting both replies stay feature-specific.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Hermes Agent <hermes-agent@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(plugin): lockfile + per-user trust store Implements Q00#732 (locked spec). Stacked on feat/plugin-manifest-loader (Q00#728). src/ouroboros/plugin/lockfile.py: - LockEntry dataclass and Lockfile manager. - Persists at ~/.ouroboros/plugins.lock (TOML). - Atomic writes (temp file + os.replace). - POSIX flock for concurrent-write safety. - Deterministic name-sorted entry order so diffs are reviewable. - Schema versioned (schema_version = "0.1"); unsupported versions raise on read. src/ouroboros/plugin/trust_store.py: - GrantedScope, TrustRecord dataclasses; TrustStore manager. - Per Q00/ouroboros-plugins#9 lock: Q3 — exact-scope only (TrustRecord.has_scope is exact match; no parent-implies-child). Q4 — version bump invalidates trust (granting against a new version drops previous grants; reset_for_version_bump() writes empty grants). Q5 — per-user storage at ~/.ouroboros/plugins/<name>/trust.json. Q6 — granted_by + granted_at recorded; firewall (Q00#729) and CLI (Q00#731) emit plugin.trusted using these fields. - No tokens stored. The store has no token-storage API; the test_no_raw_token_in_persisted_file test enforces this. tests/unit/plugin/test_lockfile.py — 7 tests covering install/read, removal, sorted order, schema_version header, unsupported version, atomic-write crash safety, concurrent writes (multiprocessing). tests/unit/plugin/test_trust_store.py — 9 tests covering grant idempotence, exact-scope rule (Q3), version-bump invalidation (Q4), storage path (Q5), event-relevant fields (Q6), removal/dir pruning, unsupported schema_version, no-raw-token invariant, ordered missing(). All 16 tests pass: PYTHONPATH=src python3 -m pytest \ tests/unit/plugin/test_lockfile.py \ tests/unit/plugin/test_trust_store.py -v Sub-issue of Q00#725 (Phase 2). Closes Q00#732. * feat(plugin): UserLevel program registry Implements Q00#730 (locked spec). Stacked on feat/plugin-manifest-loader (Q00#728). src/ouroboros/plugin/userlevel_registry.py: - UserLevelProgramRegistry: in-memory registry of installed UserLevel programs, keyed by plugin name and namespace. - Namespace ownership enforced: first registered plugin owns the namespace; subsequent registrations for the same namespace raise RegistryError. - Duplicate name registration without replace=True raises. - all_programs(), get(), get_by_namespace(), unregister(). - Global singleton via get_userlevel_registry(); reset_userlevel_registry() for tests. - lookup_command(name) — top-level cross-registry helper. Tries the UserLevel registry by namespace, then by name, then falls through to the bundled SkillRegistry. Returns LookupResult with kind in {"userlevel", "skill", "none"}. Design choice: rather than refactor SkillRegistry to handle JSON manifests in addition to SKILL.md frontmatter, this module sits alongside it. The two artifact shapes have different lifecycle semantics (install/trust vs hot-reload); merging them would balloon the existing class. The cross-registry helper provides the "single lookup" the locked spec requires without forcing a class merge — and existing skill tests are unchanged (verified — see commit log). tests/unit/plugin/test_userlevel_registry.py — 10 tests: - register/get/get_by_namespace happy path - namespace collision rejected - duplicate name requires replace=True - unregister releases name AND namespace - all_programs returns every entry - lookup_command via namespace, via name, "none" for unknown - global singleton identity - skill registry unaffected (regression guard) All 10 tests pass; existing skill registry tests also still pass. Sub-issue of Q00#725 (Phase 1). Closes Q00#730. * feat(plugin): invocation firewall + audit-event emitter Implements Q00#729 (locked spec). Stacked on feat/plugin-manifest-loader (Q00#728), feat/plugin-userlevel-registry (Q00#730), feat/plugin-lockfile-trust-store (Q00#732) — branches merged into this one for cross-module integration. src/ouroboros/plugin/firewall.py: invoke_plugin(program, command_name, argv, *, trust_record, event_sink, correlation_id, confirm, subprocess_runner) Behavior, in locked order: 1. Pre-invocation trust check (Q00/ouroboros-plugins#9 Q1): - For non-first_party manifests, every required:true permission must be present in the trust record (exact-scope match per Q3). - On any missing required scope: emit ONLY plugin.failed with result.status="blocked" and a clean error message ("plugin requires `<scope>` (<risk>), which is not yet trusted. Run: ooo plugin trust <name> --scope <scope>"). NO plugin.invoked is emitted. Return InvocationResult(status="blocked"). - First-party programs (source.type=first_party) skip the trust check entirely (per Q00/ouroboros-plugins#8 lock). 2. Confirmation gate (Q00/ouroboros-plugins#9 Q2): - If command.requires_confirmation=true, call confirm() once. - On decline: emit plugin.failed (blocked, "user declined confirmation"). NO subprocess launched. 3. Emit plugin.invoked. 4. Emit one plugin.permission_used per required:true permission. Optional permissions (required:false) are NOT emitted in v0 (Option (a) coarse rule). When a real plugin proves the need for granular per-call emission, Option (b)/(c) graduates as a follow-up sub-issue. 5. Run entrypoint subprocess (subprocess.run by default; injectable for tests via subprocess_runner). 6. Emit plugin.completed (status=success) or plugin.failed (status=failed) with the exit code in result.message. Bounded payloads: - argv stored as-is (already user-facing). - Raw stdout/stderr NOT copied to events; only sha256 hashes are stored in provenance (stdout_sha256, stderr_sha256). - Tokens, channel IDs, free-form messages forbidden by contract. All emitted events conform to schemas/0.1/audit-event.schema.json. EventSink is a callable (the core ledger writer in production; a list.append in tests). The firewall does not own the audit log. tests/unit/plugin/test_firewall.py — 9 tests: 1. Happy path: invoked → permission_used → completed in order; no raw stdout in any event; sha256 hash recorded. 2. Trust violation: ONLY plugin.failed (blocked); explicitly assert plugin.invoked is NOT emitted; locked Q1 error format checked. 3. Subprocess failure: invoked + permission_used + failed; exit code in message. 4. Bounded payload: 1MiB stdout — no portion appears in events. 5. Confirmation declined: blocked, no subprocess launched. 6. Confirmation accepted: normal happy path. 7. Optional permission NOT emitted (locked Option (a) coarse rule). 8. First-party skips trust check; trust_state="first_party". 9. Entrypoint not found: failed, exit_code=127. All 9 firewall tests pass; full plugin test suite pass: PYTHONPATH=src python3 -m pytest tests/unit/plugin/ Sub-issue of Q00#725 (Phase 1). Closes Q00#729. * feat(ledger): plugin audit-event adapter for core EventStore Implements Q00#737 (locked spec). Stacked on feat/plugin-firewall (Q00#729). Investigation finding: the core EventStore (src/ouroboros/persistence/ event_store.py) stores rows shaped as (id, aggregate_type, aggregate_id, event_type, payload, timestamp). The plugin audit-event schema (schemas/0.1/audit-event.schema.json) declares additionalProperties:false, so any wrapping fields the core ledger needs MUST live ABOVE the audit-event boundary. The shapes are compatible without modifying the audit-event schema. src/ouroboros/plugin/ledger_adapter.py: wrap_plugin_event(audit_event, *, correlation_id, aggregate_id=None, envelope_id=None) -> dict Wraps a firewall-emitted audit event in a core-ledger row envelope: - aggregate_type = "plugin" - aggregate_id = correlation_id (override-able) - event_type = audit_event["event_type"] (mirrored to the envelope so the events_table.event_type column is queryable without parsing JSON) - payload = the full audit event (immutable copy) - timestamp = audit_event["occurred_at"] - id = uuid4 (override-able) The audit event itself is NOT mutated; payload remains schema-valid for downstream consumers that read it back. unwrap_plugin_event(envelope) -> dict Recovers the original audit event from a stored row. make_event_sink(append_fn, *, correlation_id, ...) -> EventSink Factory that produces a firewall-compatible EventSink. Production wires append_fn to EventStore.append; tests pass list.append. This module deliberately does NOT import event_store. It speaks the envelope shape, not the store. The CLI (Q00#731) is the integration point that bridges to a real async EventStore session. tests/unit/plugin/test_ledger_adapter.py — 14 tests: - basic wrap/unwrap, no input mutation - payload remains schema-valid (additionalProperties:false honored) - ROUND-TRIP for all 7 audit event types - unwrap rejects non-plugin envelopes - wrap rejects missing event_type, missing occurred_at, non-dict - aggregate_id override - make_event_sink integration with deterministic id factory - envelope.event_type mirrors payload.event_type - sanity: no token-shaped keys leak into envelope All 14 tests pass; full plugin test suite 207 passed, 1 skipped. Sub-issue of Q00#725 (Phase 1). Closes Q00#737. * feat(cli): ooo plugin {discover,inspect,list} (read-only) Implements the read-only half of Q00#731 (locked spec). Stacked on the Sprint 2 stack (Q00#745, Q00#746, Q00#747, Q00#748, Q00#749). src/ouroboros/cli/commands/plugin.py: - Typer app `plugin` registered as `ooo plugin <subcommand>`. - `discover <path>`: load and validate a manifest without registering or granting trust. Friendly error with JSON Pointer to failing field on schema violation. Lists required scopes that future invocations will need granted. - `inspect <name>`: read the lockfile + trust store; print installed metadata, trust state, granted scopes, and any missing required scopes. Both stores are overrideable via --lockfile / --trust-root so tests don't touch ~/.ouroboros. - `list [--json]`: enumerate installed plugins. Default is a Rich table; --json emits plain stdout (typer.echo) so consumers can pipe to jq. - Helper _trust_state_label() honors source.type=first_party. src/ouroboros/cli/main.py: - Imports `plugin` and registers it via app.add_typer. tests/unit/cli/test_plugin_command.py — 8 tests: - discover happy path - discover schema violation surfaces JSON Pointer + exit 1 - inspect on uninstalled plugin errors with clean message - inspect on installed-but-untrusted reports state + missing scopes - inspect on installed-and-trusted reports state + no missing - list on empty lockfile prints no-plugins notice - list --json emits parseable JSON - bare `ooo plugin` shows help State-mutating subcommands (add, install, trust, disable, remove) land in the follow-up PR (CR-6b). The module structure is laid out so the follow-up plugs in with minimal churn. Sub-issue of Q00#725 (Phase 2). Partial: Closes the read-only half of Q00#731. * fix(plugin): enforce trust/version contract at firewall + CLI boundaries Addresses the three BLOCKING findings from the ouroboros-agent codex review on PR Q00#750 plus the Ruff lint failures. The bot's design notes called out that the trust/version contract was not consistently enforced between the firewall and the CLI report; this commit brings both sides in line and adds regressions for each. 1. firewall._missing_required: a TrustRecord whose version no longer matches the manifest is now treated as if no scopes were granted, so an upgrade-without-reset can no longer silently bypass the trust gate. Aligns with trust_store.py's locked Q4 ("version bump invalidates trust"). Regression: test_stale_trust_version_blocks_invocation. 2. firewall.invoke_plugin: the subprocess launch path now catches OSError (covering FileNotFoundError, PermissionError, and ENOEXEC-class failures) and emits a terminal plugin.failed event instead of letting the exception escape. Conventional shell exit codes preserved (127 = not found, 126 = found-but-not-executable / generic launch failure). Regressions: test_entrypoint_permission_error_emits_failed_126, test_entrypoint_generic_oserror_emits_failed_126. 3. cli.commands.plugin: inspect/list now compute trust_state with the same gate the firewall enforces (manifest-version match + every required scope granted). A plugin missing a required scope, or with a stale-version trust file, no longer surfaces as "trusted". The list_command JSON/table output also exposes missing_required_scopes so consumers can pipe to jq for an automated re-trust step. Regressions: test_inspect_partial_trust_reports_installed_not_trusted, test_inspect_stale_version_reports_installed, test_list_json_partial_trust_reports_installed_not_trusted. 4. Ruff: cleared the I001 / UP035 / UP017 / F401 / UP037 violations across plugin/firewall.py, plugin/ledger_adapter.py, plugin/lockfile.py, plugin/manifest.py, plugin/trust_store.py, and the matching tests. Lint job is now green; no behavioral change beyond import shape and datetime.UTC alias. uv.lock picks up the already-declared jsonschema dep so the freshly created venv resolves without manual sync. Verified: pytest tests/unit/plugin tests/unit/cli/test_plugin_command.py tests/unit/cli/test_main.py → 253 passed, 1 skipped; ruff check src/ tests/ → clean; mypy on touched modules → clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * style(plugin): apply ruff format to satisfy CI format check Local CI uses `ruff format --check` in addition to `ruff check`. The previous fix commit landed lint-clean but not format-clean, so the Lint job failed on `ruff format --check`. Pure-whitespace reformat of the four files Ruff wanted to touch — no behavioral change. `ruff check`, `ruff format --check`, and the focused pytest suite are all green locally. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(plugin): close ledger-bridge contract + registry-replace + CLI state-file errors Addresses the second round of ouroboros-agent codex review on PR Q00#750. The first three BLOCKING findings landed in the prior commit; this follow-up resolves the new BLOCKING and the two warning-level follow-ups raised against commit a9d4e14, all sitting at the integration boundaries the bot's design notes flagged. 1. ledger_adapter.py — bridge to the real EventStore is now real (previous BLOCKING). `make_event_sink` accepts a sync `Callable[[dict], None]`, but `EventStore.append` is async and only takes `BaseEvent`. The docstring used to claim the two could be wired together directly, which would crash at runtime as soon as audit persistence ran. Added two helpers that close the gap: `envelope_to_base_event` (pure dict→BaseEvent converter, lazy imports `BaseEvent` so this module stays cheap) and `append_envelope_to_event_store` (async coroutine that converts then awaits `store.append`). Updated the module docstring + the `make_event_sink` parameter docs to spell out exactly how a sync firewall integrates with the async store (e.g. `asyncio.run_coroutine_threadsafe`). Test exercises the full path against an in-memory `sqlite+aiosqlite` EventStore and round-trips the persisted row back to a schema-valid audit event. 2. userlevel_registry.py — `register(replace=True)` now releases the prior namespace mapping when the replaced manifest moved to a different namespace under the same plugin name. Without this the old namespace stayed permanently owned by the plugin (so `get_by_namespace(old_ns)` returned the new program — wrong target) and other plugins were forever blocked from claiming it. Test covers all three observable consequences. 3. cli/commands/plugin.py — `inspect` and `list` now wrap `Lockfile.read()` and `TrustStore.read()` in a friendly-error helper. A malformed `plugins.lock` (TOMLDecodeError, ValueError from schema_version mismatch) or a corrupt `trust.json` (JSONDecodeError) used to crash these diagnostic commands with a raw traceback — exactly when the operator most needs a clear message. Tests cover bad TOML, schema-version drift, and bad JSON; all three exit with code 1 and a "no Traceback" check. Verified locally: ruff check + ruff format --check are clean; mypy is clean on the touched modules; focused pytest (227 passed, 1 skipped) and the wider plugin/cli sweep are green. The two unrelated codex-CLI-runtime test failures on my workstation (`test_codex_cli_runtime.py`) are local-environment-specific — they pass in upstream CI for both 3.12 and 3.13 on the same base commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(plugin): preserve audit timestamp through ledger bridge Round-3 BLOCKING from ouroboros-agent: `envelope_to_base_event` was constructing the `BaseEvent` without setting `timestamp`, so the default factory `datetime.now(UTC)` overrode the firewall's recorded audit time. Once persisted, the events_table.timestamp column would reflect when the bridge ran, not when the firewall observed the event — silently breaking ordering, recency queries, and replay for plugin audit rows. Fix: - The envelope already carries `timestamp` (set by `wrap_plugin_event` from `audit_event.occurred_at`). Parse it via a small helper and pass it to the `BaseEvent` constructor. - Defensive fallback to `payload.occurred_at` if a third-party builder skipped the envelope helper. Reject envelopes with no parseable timestamp instead of silently defaulting to "now" — that is the failure mode that tripped the bot. - Normalize the trailing `Z` to `+00:00` before `fromisoformat` so the parser is portable across Python builds and surface a clean ValueError citing the offending string. Tests: - `test_envelope_to_base_event_preserves_audit_timestamp` — uses an audit time well in the past so any "now"-default would diverge by more than a year, ruling out false-pass on a slow machine. - `test_envelope_to_base_event_round_trips_timestamp_through_real_store` — persists via `append_envelope_to_event_store`, replays from an in-memory `sqlite+aiosqlite` EventStore, asserts the row's timestamp equals the audit's `occurred_at`. - `test_envelope_to_base_event_rejects_missing_timestamp` — guarantees the bridge fails loud rather than defaulting silently. Verified locally: ruff check, ruff format --check, mypy, and the focused pytest sweep (56 passed) are green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(plugin): cover structurally-corrupt state files + deep-copy envelope payload Round-4 review from ouroboros-agent. The previous round closed the timestamp regression; this round addresses the new BLOCKING + warning + suggestion the bot raised on commit 6be2466, all about robustness at the dict/state boundaries. 1. cli/commands/plugin.py — friendly-error guard now also catches `KeyError` and `TypeError` (BLOCKING). Both `Lockfile.read()` and `TrustStore.read()` build dataclasses via unchecked `raw["..."]` lookups, so a parseable-but-structurally-corrupt state file (e.g. a `[[plugin]]` block with no `name`, or wrong field type) raised a raw `KeyError`/`TypeError` straight through `inspect`/`list` instead of the clean diagnostic those commands promise. 2. cli/commands/plugin.py — `inspect`/`list` now also handle `OSError` from `load_manifest()` (warning). `load_manifest` opens the file directly, so an unreadable installed manifest (chmod 000, broken symlink, vanished plugin_home) bypassed the existing `PluginManifestError` catch and crashed the diagnostic CLI. 3. ledger_adapter.py — `wrap_plugin_event` and `envelope_to_base_event` now deep-copy the payload (suggestion). The earlier `dict(audit_event)` was a shallow copy, so later mutation of nested objects like `audit_event["plugin"]` or `audit_event["command"]["argv"]` silently leaked into the persisted envelope/BaseEvent. Audit events are bounded (no raw stdout/stderr — see firewall payload guard), so deep copy is cheap and removes the foot-gun the bot called out. Tests: - `test_inspect_structurally_corrupt_lockfile_reports_friendly_error` — a TOML-valid lockfile missing the `name` field exits 1 with a clean message, no traceback. - `test_inspect_unreadable_manifest_reports_friendly_error` — chmod 000 on the installed manifest exits 1 with "manifest is unreadable"; auto-skips on Windows or when running as root. - `test_wrap_payload_is_deep_copied` and `test_envelope_to_base_event_data_is_deep_copied` — mutating nested fields on the source after the bridge runs cannot leak into the envelope or the persisted BaseEvent. Verified locally: ruff check + ruff format --check are clean; mypy is clean on the touched modules; focused pytest sweep is 234 passed, 1 skipped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(plugin): require source.path for path/home + discover OSError + manifest order Round-5 review from ouroboros-agent. Closes the two BLOCKING and the non-blocking suggestion raised against ed0bdc1, all about contract consistency at the schema and CLI surfaces. 1. plugin.schema.json — `source` now requires `path` when `type` is `local_path` or `plugin_home` (BLOCKING). The previous schema only required `type`, so a manifest with `{"type":"local_path"}` and no location metadata sailed through `load_manifest` and only failed later at install/runtime. Conditional `if/then` clause keeps `first_party` (which has no on-disk path) accepted while making the location-bearing source types reject incomplete manifests. Also added `minLength: 1` to `path` and `repository` so an empty string can't satisfy the required check. 2. cli/commands/plugin.py — `discover` now also catches `OSError` from `load_manifest()` (BLOCKING). `inspect` and `list` already converted the OSError path to a friendly message in round 4; `discover` is the same shape of diagnostic command and shouldn't crash on chmod 000 / broken symlink / transient I/O while the sibling commands gracefully report. 3. manifest.py — `capabilities` and `permissions` are now `tuple` instead of `frozenset` (suggestion). Frozenset iteration order varied across hash seeds and Python versions, leaking non-deterministic ordering into `discover`/`inspect`/`list --json` output and into the firewall's `plugin.permission_used` event stream for multi-scope plugins. The schema's `uniqueItems: true` already enforces no-duplicates at validation time, so a tuple preserves the operator-authored order with no loss of safety. Tests: - `test_source_local_path_requires_path` / `test_source_plugin_home_requires_path` — schema rejects manifests missing `path` for those source types. - `test_source_first_party_path_optional` — first-party manifests still load without `path`. - `test_discover_unreadable_manifest_reports_friendly_error` — chmod-000 on the discover target exits 1 with "manifest is unreadable", no traceback. Auto-skips on Windows / when running as root. - `test_capabilities_and_permissions_preserve_manifest_order` — loader returns tuples in the operator-authored order; both capabilities and permissions are checked against an explicitly non-alphabetic order to flush out hash-keyed iteration. Verified locally: ruff check + ruff format --check are clean; mypy clean on touched modules; focused pytest sweep is 239 passed, 1 skipped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(plugin): anchor entrypoint cwd to source.path + reject whitespace command Round-6 review from ouroboros-agent. Closes the BLOCKING and the follow-up warning raised against 94b3a01, both at the firewall's launch path. 1. firewall.py — `invoke_plugin` now anchors the subprocess `cwd` to the plugin's `source.path` for `local_path` / `plugin_home` manifests (BLOCKING). The bot's design notes flagged that the schema now requires that path metadata but the runtime layer never consumed it, so relative entrypoints (`./run.sh`) and commands that read files from the plugin directory failed whenever `ooo` was invoked outside the plugin home. First-party plugins (which ship inside the binary and have no plugin-local directory) keep inheriting the caller's cwd. The cwd is resolved to absolute via `Path.expanduser().resolve()` and only used when the directory actually exists; a missing dir gracefully degrades through the existing OSError → `plugin.failed` path (clean message instead of a pre-launch crash). 2. firewall.py + plugin.schema.json — whitespace-only entrypoint commands are now rejected at both layers (warning). Schema gains `pattern: "\\S"` so a manifest with `entrypoint.command = " "` is invalid up front. As defense in depth, the firewall also inspects `shlex.split(cmd_template)` at launch time: if it returns `[]`, the firewall emits a terminal `plugin.failed` with exit 126 and a clear message instead of crashing the caller. Either bypass alone now fails cleanly with audit output, satisfying the firewall's stated launch-failure contract. Tests: - `test_subprocess_runs_with_cwd_for_local_path_manifest` — captures `cwd` from the spy runner; resolves equal to `manifest.source.path`. - `test_subprocess_inherits_cwd_for_first_party_manifest` — first- party plugins still get `cwd=None`. - `test_whitespace_entrypoint_emits_failed_not_crash` — constructs a PluginManifest with whitespace `command` directly (bypassing schema validation) so we exercise the firewall's runtime guard, asserting no subprocess attempt and a terminal `plugin.failed` event. Verified locally: ruff check + ruff format --check are clean; mypy clean on touched modules; focused pytest sweep is 242 passed, 1 skipped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(plugin): per-plugin trust file lock + manifest path normalization Round-7 review from ouroboros-agent. Closes both BLOCKING findings raised against f2be357, both about real runtime data-integrity contracts at the persistence layer. 1. trust_store.py — `TrustStore.grant()` and `reset_for_version_bump` are now serialized by a per-plugin POSIX file lock (BLOCKING). The previous read-modify-write was unlocked, so two concurrent `grant()` calls for distinct scopes on the same plugin could both observe the same prior file and each overwrite it with a one-scope payload — silently losing the other grant. The lock mirrors the pattern `Lockfile` already uses (fcntl.flock on a sibling `*.lock` file), but is per-plugin so cross-plugin grants don't serialize against each other. `remove()` cleans up the lock file alongside the trust file so the per-plugin directory still collapses cleanly. 2. manifest.py — `load_manifest` now normalizes a relative `source.path` against the manifest file's parent directory at load time (BLOCKING). The schema permits relative strings, but the firewall consumed `manifest.source.path` as a subprocess `cwd`, so a manifest saying `"path": "plugins/github-pr-ops"` ran correctly only when `ooo` happened to be invoked from the right directory. Resolving at load time anchors the path to the plugin's actual install location, so the firewall's cwd is now stable regardless of where the operator started the CLI. First-party plugins (no on-disk path) are unaffected. Tests: - `test_concurrent_grants_do_not_lose_scopes` — fans 20 threads at a barrier, each granting a distinct scope; under the new lock every scope must survive in the persisted record. The prior racy implementation lost grants with high probability under this workload. - `test_relative_source_path_resolves_to_absolute` — manifest with `path: "."` resolves to the manifest dir at load time. - `test_first_party_source_path_stays_none` — guard that the normalization step does not invent a path for first-party manifests. Verified locally: ruff check + ruff format --check are clean; mypy clean on touched modules; focused pytest sweep is 245 passed, 1 skipped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(plugin): trust-store remove() locked + first-party CLI shortcut + stale grants Round-8 review from ouroboros-agent. Closes the BLOCKING and the warning + suggestion raised against d151160, all about state consistency. 1. trust_store.py — `TrustStore.remove()` now also runs inside the per-plugin POSIX flock (BLOCKING). Without this, a concurrent `grant()` could win a race against `remove()` and atomically recreate `trust.json` after `remove()` reported success — leaving a supposedly-removed plugin still trusted, or non-deterministically dropping grants. The unlink/prune sequence is now an observable single critical section, mirroring `grant()` / `reset_for_version_bump()`. 2. cli/commands/plugin.py — `inspect` and `list` no longer read `trust.json` for first-party plugins (warning). The firewall ignores the trust store for `source.type == "first_party"` by design (Q00/ouroboros-plugins#8), so a leftover or corrupt trust file there must not make these read-only diagnostic commands fail. The CLI now skips the trust read when the manifest declares first-party and reports "first_party" with no scopes — straight from the manifest, which is the authoritative source. 3. cli/commands/plugin.py — `list --json` no longer echoes raw grants when the trust file's recorded version no longer matches the manifest (suggestion). Those grants are not effective (firewall treats the record as invalidated), and the previous output left JSON consumers with contradictory fields (`trust_state: installed` alongside a populated `granted_scopes` list). The JSON view now zeroes those grants and adds an explicit `trust_version_stale: bool` flag so consumers can branch deterministically without having to compare versions themselves. Tests: - `test_inspect_first_party_ignores_corrupt_trust_file` — first- party inspect succeeds and reports `first_party` even when the trust file is unreadable JSON. - `test_list_json_zeroes_stale_grants_and_flags_version_drift` — stale-version trust produces `granted_scopes: []`, `trust_version_stale: true`, and surfaces the missing required scopes. Verified locally: ruff check + ruff format --check are clean; mypy clean on touched modules; focused pytest sweep is 247 passed, 1 skipped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(plugin): list survives doubly-corrupt row + accurate skip comment Round-9 review from ouroboros-agent (APPROVE on b25d69b) flagged the `manifest is unreadable` branch in `list`'s per-row loop: the inline comment claimed it would "skip and report stale-but-known state", but the next line still called `_read_trust_or_exit(...)`, which is itself an exit path. That asymmetry is a real diagnostic defect, not just a stale comment — when a row's manifest was already lost AND its `trust.json` happened to be structurally corrupt, the trust read would crash the entire `list` output, even though the row was guaranteed to fall through to the safe-default "installed" branch anyway. That defeats the diagnostic purpose of the command (one row's bad state should never erase every other row). cli/commands/plugin.py — the unreadable-manifest branch now sets `record = None` and skips the trust read entirely, matching the comment. The fallback handler downstream already reports such rows as `trust_state="installed"` with empty grants and `missing=[]`, so the resulting JSON shape is unchanged for the well-trodden single-corruption case (manifest unreadable, trust readable); only the doubly-corrupt case stops aborting `list`. Tests: - `test_list_survives_doubly_corrupt_row` — chmod 000s the manifest AND drops a trust.json that's missing required keys (so `TrustStore.read` would raise KeyError if reached). With the prior call to `_read_trust_or_exit` this aborted `list` with exit 1; under the fix the row degrades cleanly to `trust_state: installed`, `granted_scopes: []`, `trust_version_stale: false` and `list` exits 0. Verified locally: ruff check + ruff format --check are clean; focused pytest sweep is 248 passed, 1 skipped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(plugin): mirror firewall first-party trust bypass in CLI reporting Round-9 review approved 61fcf1c with a follow-up warning: the CLI's trust-state report still reads as if every `required: true` permission must be trusted, even for `source.type == "first_party"`. That contradicts the firewall — `invoke_plugin` explicitly skips `_missing_required` when the manifest is first-party — and the schema still permits first-party manifests to declare permissions (they are advisory in that mode). With the prior CLI: - `inspect` printed `missing scopes: ...` for a first-party plugin even though the firewall would never block its invocation. - `list --json` echoed those declarations as `missing_required_scopes` for the same reason. - `discover` told operators the scopes "must be trusted before invocation", which is false for first-party. cli/commands/plugin.py — `_missing_required` now short-circuits to `[]` for first-party manifests, mirroring the firewall's bypass at `invoke_plugin`. `discover_command` adjusts its header for first-party plugins to label the declarations as advisory ("first-party plugins bypass the trust gate") instead of demanding pre-invocation trust. `inspect`/`list` inherit the correction transitively because both call `_missing_required` to build their `missing_*` output, so the JSON shape stays stable — the values just stop being wrong. Tests: - `test_discover_first_party_marks_required_scopes_advisory` — declared scopes still surface (operators can see what the plugin asks for) but the output explicitly says "advisory" / "first-party" and never tells operators they must be trusted. - `test_inspect_first_party_does_not_report_missing_scopes` — `trust_state: first_party` and no "missing scopes:" line, even with a manifest that declares two `required: true` scopes. - `test_list_first_party_json_has_no_missing_required_scopes` — `missing_required_scopes: []` for the first-party row in JSON output, matching what the firewall would actually do. Verified locally: ruff check + ruff format --check are clean; focused pytest sweep is 251 passed, 1 skipped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(plugin): merge Q00#745's path sandbox with Q00#750's path normalization Q00#745 (manifest loader + schemas) merged into main as 202a487 while PR Q00#750 was iterating. The two stacks resolved the same problem (operator-friendly cwd handling for `local_path` / `plugin_home` plugins) along complementary axes: - Q00#745 added `_validate_sandboxed_path` to reject absolute paths, `..` traversal, Windows drive prefixes, and backslashes at load time. The output `source.path` stayed as the user-authored relative slug. - PR Q00#750's d151160 normalized relative `source.path` to absolute against the manifest file's parent directory at load time, so the firewall's subprocess `cwd` no longer depended on where the operator ran `ooo`. Without this commit the post-rebase loader rejected the firewall's own integration test (`test_subprocess_runs_with_cwd_for_local_path_manifest`, which had to write `path: str(tmp_path)`, an absolute string), and downstream consumers of `manifest.source.path` again behaved differently across cwd. Layering the two solutions is strictly better than picking one: 1. The sandbox check still validates the on-disk text the user authored — no traversal, no absolute, no Windows drive prefix. 2. After the validated relative slug is in hand, the loader resolves it against `manifest_path.parent`, so every runtime consumer (firewall cwd, CLI inspect/list output) sees a stable absolute filesystem location. manifest.py — `load_manifest` resolves the validated relative `source.path` against `manifest_path.parent` immediately after the sandbox check. First-party manifests (no path) and any non-sandboxed source type are unaffected. Test fixture updates: - test_local_path_source_with_path_loads / test_plugin_home_with_relative_path_loads — now assert the loaded `source.path` is the absolute resolution of the user-authored relative slug, not the slug verbatim. - test_relative_source_path_resolves_to_absolute (re-added from d151160) — `path: "."` resolves to the manifest dir. cli/commands/plugin.py — `_load_with_friendly_error` now reads the user-facing message from `PluginManifestError.args[0]` instead of hard-coding "manifest invalid:". `load_manifest` was extended in Q00#745 to convert OSError, JSON decode failures, and UTF-8 decode failures into structured PluginManifestError with self-describing messages ("manifest is unreadable: …", "manifest is not valid JSON: …"). The CLI now surfaces those messages directly, so operators can tell unreadable from schema-invalid without having to grep `expected`/`got`. The "manifest invalid:" prefix is still added for the jsonschema validation path, where args[0] is a raw schema error string and needs the leading classification. tests/unit/plugin/test_firewall.py — the cwd-anchoring test now uses `path: "."` (sandboxed relative slug) instead of `str(tmp_path)` (absolute, rejected by Q00#745's sandbox). Verified locally: ruff check + ruff format --check are clean; focused pytest sweep is 278 passed, 1 skipped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(plugin): close trust-store inode race + cwd fallback regression Round-9 review on 639975b raised two BLOCKING findings that survive the rebase. Both are real correctness bugs at the persistence and process-launch boundaries. 1. trust_store.remove() POSIX inode race (BLOCKING). The previous `remove()` body unlinked `trust.json.lock` *inside* the critical section while still holding the per-plugin flock. POSIX flock binds to the inode behind the lock-file path, so unlinking only removes the dirent — the lock stays attached to the now-orphan inode. A concurrent `grant()` would then `open(lock_path, "w")` against a brand-new inode, `flock` *that* exclusively, and run in parallel with the still-active `remove()`. By the time we released our lock on the orphan inode, the concurrent writer had already recreated `trust.json` — exactly the race the per-plugin lock was added to close. `remove()` now leaves the lock file alone; it is a synchronization primitive, not operation-scoped state, and must outlive individual operations for `flock` correctness. Best-effort `parent.rmdir()` already tolerates a non-empty directory, so no second-order regression. 2. firewall._resolve_plugin_cwd missing-dir fallback (BLOCKING). When a `local_path` / `plugin_home` plugin's `source.path` no longer exists on disk, the resolver used to return `None` and `invoke_plugin` then launched the entrypoint with `cwd=None`, silently inheriting the operator's current working directory. With a relative entrypoint (`./run.sh`) or any command that reads plugin-local files, that fallback can run the wrong program or read unrelated files instead of producing the documented `plugin.failed` launch error. The resolver now always returns the candidate path (resolved via `Path.resolve(strict=False)`), so `subprocess.run` raises `OSError(ENOENT)` from the `chdir` call, which the firewall's existing OSError-wrapping path (round 7) already converts into a terminal `plugin.failed` event with the missing-dir reason in the message. Tests: - `test_remove_keeps_lock_file_to_avoid_inode_race` — exercises `remove()` on a granted plugin and asserts `trust.json.lock` remains on disk so the next `grant()` / `remove()` cycle keeps the same inode-stable lock target. - `test_remove_drops_trust_file` updated: `trust.json` still vanishes, but the parent dir is no longer guaranteed to be pruned (the lingering lock file blocks `rmdir`, intentionally). - `test_missing_plugin_cwd_does_not_inherit_callers_cwd` — points the manifest at a guaranteed-missing absolute path (constructed in-memory to bypass the on-disk sandbox check), then asserts that `subprocess.run` was called with that path as `cwd` (not `None`) and that the synthesized `FileNotFoundError` surfaces as a terminal `plugin.failed` event. Verified locally: ruff check + ruff format --check are clean; focused pytest sweep is 280 passed, 1 skipped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(plugin): catch shlex.split errors + correct sandboxed-path docstring Round-10 review on d439d68 raised one BLOCKING and one documentation suggestion. 1. firewall: shlex.split ValueError handling (BLOCKING). A manifest with malformed shell quoting in `entrypoint.command` (e.g. `"\"unterminated`) passes the schema's `\\S` pattern check and `load_manifest()`, but `shlex.split()` raises `ValueError` from inside `invoke_plugin()`. Before this fix the firewall had already emitted `plugin.invoked` and any `plugin.permission_used` events for the call, then crashed up through the caller — a partial audit trail and a broken contract that says every launch failure must surface as a terminal `plugin.failed` event with a structured `InvocationResult`. `invoke_plugin()` now wraps `shlex.split(cmd_template)` in `try/except ValueError` and converts the error into the same clean `plugin.failed` exit-126 path the empty-command branch already uses, with the shlex error message threaded through `result.message` so operators can tell the failure mode at a glance. 2. manifest: sandboxed-path docstring drift (suggestion). The module comment on `_PATH_SANDBOXED_SOURCE_TYPES` claimed `local_path` resolves against the manifest dir while `plugin_home` resolves against the user's plugin home. Since the path-normalization fix in 73c2a7e merged the two, the loader now anchors both source types against `manifest_path.parent`. The comment is rewritten to describe the actual behavior (manifest-dir-relative for both) and to point out that the install/manager layer is responsible for placing `plugin_home` manifests under the correct user plugin directory before the loader runs. Tests: - `test_malformed_entrypoint_quoting_emits_failed_not_crash` — builds a `PluginManifest` with `command='"unterminated'` in-memory (so the schema's `\\S` check still accepts it), registers it, and invokes through the firewall. Asserts the spy `subprocess_runner` is never called, the terminal event is `plugin.failed`, exit code 126, and the message names the shlex failure. The corresponding `subprocess.run` is never reached, which is the whole point of catching the error *before* launch. Verified locally: ruff check + ruff format --check are clean; focused pytest sweep is 281 passed, 1 skipped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(plugin): close symlink-escape sandbox bypass Round-11 review on e88da53 raised one BLOCKING finding: the sandbox check validated only the user-authored path text, not the resolved filesystem target. A manifest could declare `source.path = "plugins/link"` where the on-disk `plugins/link -> /outside/path` symlink follows out of the plugin root. The textual sandbox accepted it (no `..`, no absolute, no drive prefix), and `Path.resolve()` honors symlinks during the path-normalization step, so the firewall ended up running the plugin with `cwd` outside the manifest's directory — a real sandbox escape. manifest.py — after resolving the candidate path, the loader now asserts `candidate.relative_to(anchor)` where `anchor = manifest_path.parent.resolve()`. The two checks are complementary: the syntactic sandbox rejects unsafe text up front; the resolved-target check verifies the kernel-level result still lives under the manifest root after symlink expansion. Either alone is insufficient. When the resolved target escapes, the loader raises `PluginManifestError` with `json_pointer="/source/path"` and the resolved escape target in `got` so the firewall and operator-facing CLI both surface a clean, structured rejection instead of silently producing a plugin that runs outside its sandbox. Tests: - `test_sandbox_rejects_symlink_escape` — places a `plugins/link -> outside_target` symlink under the manifest directory, asserts the loader rejects with the `/source/path` pointer and that the escape target appears in the error's `got` field (so CI logs and the CLI diagnostic both name the offending target). Verified locally: ruff check + ruff format --check are clean; focused pytest sweep is 282 passed, 1 skipped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…0#742) * feat(hook): route 'ooo publish' and 'ooo resume-session' keywords The CLAUDE.md ooo command table already documents `ooo publish` and `ooo resume-session`, and the corresponding skills exist at skills/publish/SKILL.md and skills/resume-session/SKILL.md, but neither pattern was wired into the UserPromptSubmit keyword detector at scripts/keyword-detector.py. As a result, typing either command produces no <skill-suggestion> output and the prompt either passes through unrouted or, when other Claude Code plugins are also installed, gets reinterpreted by their hooks. Add both patterns to KEYWORD_MAP next to the existing ooo subcommand entries. Include `ooo resume` as a short alias for `ooo resume-session` since the skill is canonically named with the hyphen but users habitually type the shorter form. The MCP setup gate in main() applies automatically — neither skill is in SETUP_BYPASS_SKILLS. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(hook): bypass setup gate for ooo resume-session The resume-session skill reads ~/.ouroboros/ouroboros.db directly and is specifically designed for the case where the MCP server is unreachable (after a Claude Code disconnect). Without this entry, the setup gate redirected the very command that is supposed to recover from a broken MCP setup, defeating its purpose in its primary use case. Add /ouroboros:resume-session to SETUP_BYPASS_SKILLS and cover both the canonical and short-alias forms with regression tests in TestSetupBypass and TestMainGate. publish stays gated: it requires the gh CLI plus issue creation against the live MCP-backed seed, so the no-MCP branch is still the right landing spot for it. Addresses ouroboros-agent[bot] BLOCKING review on PR Q00#742. * fix(hook): drop ambiguous 'ooo resume' short alias The detector matches patterns with `(?:^|\b)pattern(?:\b|$)`. With "ooo resume" in the keyword table, prose like "please ooo resume work on this" would match and silently route to /ouroboros:resume-session, mis-suggesting session recovery for ordinary text. Drop the short alias and keep only the unambiguous canonical form "ooo resume-session". This is also consistent with the design intent documented in skills/resume-session/SKILL.md, which explicitly chose the hyphenated name to avoid colliding with Claude Code's reserved /resume command — keeping a non-hyphenated alias would re-introduce the ambiguity the canonical name was picked to avoid. Add two regression tests pinning the canonical-form-only contract: prose containing "ooo resume" must not route to resume-session, and the bare short form is intentionally unsupported. Addresses ouroboros-agent[bot] follow-up finding on PR Q00#742. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…r_provenance (Q00#640) (Q00#740) Squashed rebase onto current main. - AutoPipelineResult.ledger_provenance (meta only) carries evidence_backed_sections, assumption_only_sections, and per-section LedgerSource summary (raw dict, gated on resolved status). - ouroboros_auto MCP result.meta["ledger_provenance"] mirror. - non_goals classified as evidence-backed. - coexists with Q00#691 gateway-source provenance field. Refs Q00#640
… format (Q00#746) Addresses the second ouroboros-agent[bot] review on PR Q00#746 (commit d60e793 → review record 6846863b): Blocking #1 — plugin.schema.json source object accepted local_path and plugin_home manifests with only {"type": ...} and no `path`, so load_manifest() could return a "validated" manifest that the installer later fails to resolve. Blocking #2 — TrustStore used the caller-supplied plugin name directly in filesystem paths (`self.root / plugin / "trust.json"` and the lock file path). A plugin name with `..` or `/` could escape the trust root and read/write/delete arbitrary user files via grant() / reset_for_version_bump() / remove(). The bot's design notes pointed to the same root cause for both findings: "two important invariants are still being trusted to callers instead of being enforced at the boundary where the new APIs are introduced." This commit enforces both invariants at the boundary. src/ouroboros/plugin/schemas/0.1/plugin.schema.json - source.path and source.repository now require minLength: 1. - allOf with conditional if/then rules: source.type=local_path and source.type=plugin_home both require `path`. source.type=first_party keeps the locked Q00/ouroboros-plugins#8 behavior of carrying no location field. src/ouroboros/plugin/trust_store.py - New _PLUGIN_NAME_RE matching the locked manifest /name pattern (^[a-z0-9][a-z0-9-]{1,62}[a-z0-9]$). - New _validate_plugin_name() helper raises ValueError on path separators, parent traversal, leading dots, or any other input that deviates from the locked schema. - read(), grant(), reset_for_version_bump(), and remove() all validate before touching any filesystem path (including the per-plugin lock path), so a malicious plugin identifier cannot create a lock file outside <root>/.locks/ either. tests/unit/plugin/test_trust_store.py - New parametrised test_invalid_plugin_name_rejected: every public method rejects "../escape", "..", "x/y", "x\\y", ".hidden", "X" (uppercase, fails locked pattern), "", "ab" (too short), leading and trailing dashes, and whitespace. - Existing tests retargeted from plugin="X" to "test-plugin" so the valid-name happy paths now use a name that actually matches the locked /name pattern. tests/unit/plugin/test_manifest.py - test_local_path_source_requires_path and test_plugin_home_source_requires_path: both reject manifests that declare a non-first_party source without a path, asserting the rejection points at /source. Misc: - ruff format pass (the ruff-format CI check, which is separate from ruff check, was failing on the previous commit). Verification: PYTHONPATH=src uv run pytest tests/unit/plugin/ -v 188 passed, 1 skipped uv run ruff check src/ tests/ && uv run ruff format --check src/ tests/ All checks passed! / 669 files already formatted uv run mypy src/ouroboros/plugin/ Success: no issues found in 15 source files
* feat(auto): classify interview questions by intent * fix(auto): require question-shape for cue-based intent classification ouroboros-agent[bot] flagged two silent misroutings on commit 9ab5ae1: - `_contains_intent_cue()` triggered RUNTIME_CONTEXT for any string containing broad cues like "repository" or "architecture", so questions such as "What is the repository status?" or "What architecture decisions are documented?" were routed to `_runtime_answer()` and mutated `runtime_context`/`constraints` ledger entries instead of falling back safely. - `_contains_actor_io_intent_cue()` triggered ACTOR_IO for any string containing "input" or "output", so "What is the output directory?" or "What is the input schema?" were treated as actor/IO contract questions and the answerer injected `actors`/`inputs`/`outputs` assumptions that did not answer the question. Both bugs share the same root cause called out in the bot's design notes: the multilingual cue layer treated broad noun substrings as authoritative intent without question-shape validation. False positives in this router are more expensive than false negatives because they mutate ledger state with the wrong contract. Fix: gate cue-based classification on question shape. - New `_has_runtime_context_intent()` accepts a cue match only when the strict English shape selector recognises a runtime selection question *or* a cross-lingual selection/decision verb (use/choose/configure/ adopt/사용/選ぶ/使用/etc.) accompanies the cue. - New `_has_io_cue_with_flow_shape()` requires an IO cue *and* a flow-verb or interrogative-shape anchor (English handled by the existing `_is_actor_or_io_question`; Spanish/French/German/Korean/Japanese/ Chinese contract-asking shapes added explicitly). - Actor cues continue to require an interrogative subject paired with the actor noun (extracted into `_ACTOR_NOUN_CUES`/`_ACTOR_QUESTION_CUES` for readability). Adds regression coverage for the four exact prompts the bot called out; the previously added multilingual fixtures continue to pass because they include selection verbs ("사용", "使い", "produire") or actor question words ("Quién"). Unknown property-shaped questions correctly fall through to the conservative default and produce only `constraints.conservative_mvp` plus `failure_modes.unverified_or_scope_creep` ledger entries. - 371 tests in tests/unit/auto pass - ruff clean on touched files * fix(auto): add multilingual product-behavior detection ouroboros-agent[bot] flagged commit 4694da0 with one BLOCKING finding: multilingual permission/product-behavior questions like ``Quels utilisateurs peuvent supprimer des branches?`` and ``哪些用户可以删除分支?`` now match the new actor cues (``_has_actor_cue_with_question_shape``) but ``_is_product_behavior_question`` is still English-only, so they silently routed to ``_io_actor_answer()`` and injected ``actors``/``inputs``/``outputs`` assumptions instead of preserving the requested authorization behavior. The bot's design note correctly identified the asymmetry: actor and runtime cues recognise non-English wording but PRODUCT_BEHAVIOR did not, so semantically richer questions ended up at the wrong ledger handler. Fix: introduce ``_has_product_behavior_intent()`` that combines the strict English shape selector with cross-lingual permission-modal + mutation-verb patterns covering Spanish, French, German, Korean, Japanese, and Chinese (both simplified and traditional). Because routing already preferences PRODUCT_BEHAVIOR over ACTOR_IO, restoring multilingual symmetry is enough to send these questions to ``_product_behavior_answer()`` and write ``constraints``/``acceptance_criteria`` ledger entries that preserve the requested behavior contract. Korean patterns are anchored on a verb-formation morpheme (``하|할|함|되|돼|됨|됩|됐``) immediately after the action noun so that substrings like ``저장`` inside ``저장소`` (repository) cannot falsely trigger the runtime-vs-product-behavior precedence — the existing multilingual runtime fixture (``어떤 런타임과 저장소 구조를 사용해야 하나요?``) still routes to ``runtime_context``. Adds regression coverage for the two prompts the bot called out plus three additional language siblings (Korean, German, Spanish). - 372 tests in tests/unit/auto pass - ruff clean on touched files * style(auto): apply ruff format Local ruff lint passed but ruff format --check flagged formatting drift on the previous commit (line-wrap differences in the new pattern literals). No semantic change — purely formatting. * fix(auto): word-boundary cue matching + German ASCII transliteration ouroboros-agent[bot] flagged commit 52a9ee7 with one BLOCKING and one warning-level finding, both rooted in lexical accidents in the cue layer: BLOCKING — bare ``"test"`` substring cue in ``_INTENT_CUES[QuestionIntent.VERIFICATION]`` matched unrelated words (``contest``, ``latest``, ``protest``, ``attestations``). Reproduced with ``Should users contest charges?`` and ``What is the latest output path?``, both of which previously returned ``verification_plan`` / ``acceptance_criteria`` instead of preserving product / property semantics. Warning — German product-behavior regex only accepted umlauted forms (``dürfen`` / ``löschen``) and not the conventional ASCII transliterations ``duerfen`` / ``loeschen``. Most non-DE keyboards type the ASCII form, so ``Welche Benutzer duerfen Branches loeschen?`` silently fell to ACTOR_IO and injected ``actors`` / ``inputs`` / ``outputs`` instead of preserving the requested authorization behavior. Fix: 1. Drop the bare ``"test"`` cue — ``_is_verification_question`` already anchors it with ``\btests?\b``, so the cue list adds no coverage and only false-positive risk. 2. Introduce ``_cue_matches()`` that uses regex word boundaries for ASCII-Latin single-word cues and substring matching for accented Latin or CJK cues / multi-word phrases. All cue checks (``_contains_intent_cue``, ``_has_io_cue_with_flow_shape``, ``_has_actor_cue_with_question_shape``) now route through it, so ``"test"`` cannot match ``"contest"`` and ``"verify"`` cannot match ``"overify"``. 3. Add the missing plurals to ``_IO_NOUN_CUES`` (``entrées`` / ``entrees`` / ``sorties`` / ``eingaben`` / ``ausgaben``) and ``_ACTOR_NOUN_CUES`` (``personas`` / ``stakeholders``) so the new word-boundary policy doesn't regress existing French / German / English plural fixtures. 4. Extend the German product-behavior regex with ``(?:[üu]|ue)`` / ``(?:[öo]|oe)`` / ``(?:[äa]|ae)`` alternation so umlauted and ASCII-transliterated forms route the same way. Adds regression tests for both the verification false-positive cases (``contest`` / ``latest`` / ``protest``) and the German ASCII variant. - 374 tests in tests/unit/auto pass - ruff check + ruff format --check both clean on touched files * fix(auto): tighten runtime cues to repository-/runtime-specific phrases ouroboros-agent[bot] flagged commit 0230bab with one BLOCKING finding: the runtime cue list still included broad design nouns (``architecture``, ``estructura``, ``cadre``). Combined with ``_has_runtime_context_intent`` accepting any cue + generic selection verb, this silently routed selection-shaped design questions like ``¿Qué estructura usamos para los datos?`` (and the French equivalent ``Quelle architecture utilisons-nous pour le rapport?``) into ``_runtime_answer()``, mutating ``runtime_context`` / ``constraints`` ledger entries instead of preserving the actual product/design contract. The earlier word-boundary tightening only covered noun-only false positives (``What architecture decisions are documented?``); selection- shaped cases were still slipping through. Fix: drop the broad cues — ``architecture``, ``estructura`` (alone), ``cadre`` — and keep only repository-/runtime-anchored phrases (``runtime``, ``stack``, ``repo``, ``repository``, ``framework``, ``package manager``, ``project structure``, ``project runtime``, ``estructura del proyecto``, ``repositorio``, ``référentiel``, ``gestionnaire de paquets``, ``projektstruktur``, plus CJK). English runtime-status semantics like ``Should the app display runtime status?`` remain handled by ``_should_preserve_runtime_route()`` and ``_is_product_behavior_question()``. Adds regression coverage for the bot's selection-shaped design example plus a French sibling and an English variant that should preserve product/design semantics. ``Which framework should we use?`` and ``What runtime?`` continue to route to ``runtime_context`` via the strict English shape selector, and ``어떤 런타임과 저장소 구조를 사용해야 하나요?`` / ``リポジトリのランタイムは何を使いますか?`` continue to use the multilingual runtime path. - 375 tests in tests/unit/auto pass - ruff check + ruff format --check both clean on touched files * fix(auto): drop bare CJK runtime cues + actor-anchored user-verify path ouroboros-agent[bot] flagged commit 4c1ee42 with two BLOCKING findings, both rooted in the same root cause it has been highlighting throughout this iteration: cross-lingual cue / shape pairs that are too broad and silently misroute non-runtime / non-verification questions. Bug 1 — runtime selection shape included bare CJK noun-style tokens (``설정`` / ``設定`` / ``配置``) that also surface in ordinary settings / display questions. Combined with the existing CJK runtime cues (``런타임`` / ``ランタイム`` / ``运行时``), questions like ``런타임 설정은 어디에 표시되나요?``, ``ランタイム設定はどこに表示されますか?``, and ``运行时配置显示在哪里?`` silently routed property lookups into ``_runtime_answer()``. Bug 2 — verification cues like ``vérifier`` / ``验证`` / ``확인`` are generic verbs that also surface in user-facing feature questions. Because VERIFICATION returned early in the routing layer, feature questions like ``"Can users verify their email?"`` (and the multilingual siblings) were collapsed into a generic verification-plan template instead of preserving the requested email-verification feature contract. Fix: 1. Drop bare CJK noun-style cues from ``_RUNTIME_SELECTION_SHAPE_PATTERNS``. The Korean / Japanese / Chinese selection lists now only carry verb-distinctive tokens (``사용`` / ``선택`` / ``채택`` / ``도입`` ; ``使う`` / ``使用`` / ``選ぶ`` / ``採用`` / ``導入`` ; ``使用`` / ``选择`` / ``选用`` / ``采用`` / ``採納`` / ``采纳``). ``설정`` / ``設定`` / ``配置`` / ``设定`` / ``設置`` / ``设置`` / ``구성`` / ``構成`` are gone — they leak into status/display questions. 2. Add ``_has_user_verify_feature_shape()`` that requires ``actor_noun + permission_modal + verify_verb`` simultaneously (English / Spanish / French / German plus CJK actor / modal / verb variants). An actor noun (``users`` / ``utilisateurs`` / ``사용자`` / ``用户`` / etc.) is *mandatory* so engineering questions like ``"How should we verify the HIPAA worker tests pass?"`` continue to route to ``_verification_answer()`` and through the existing regulated-data blocker. 3. Reorder routing so PRODUCT_BEHAVIOR is preferred over VERIFICATION / ACCEPTANCE_CRITERIA when both are inferred — required for (2) to take effect on multilingual user-verify questions. RUNTIME_CONTEXT precedence is unchanged. Adds two regression tests covering CJK property-lookup misrouting and the multilingual user-verify feature contract. - 377 tests in tests/unit/auto pass - ruff check + ruff format --check both clean on touched files * fix(auto): exclude meta-verify questions from user-verify product path ouroboros-agent[bot] flagged commit 4ae40d4 with one BLOCKING regression: ``_has_user_verify_feature_shape()`` accepted any question that contained an actor noun + permission modal + verify verb in any order, so meta / engineering questions like ``"Should we verify users can reset passwords?"`` and ``"How should we validate admins can log in?"`` matched the helper and routing precedence demoted VERIFICATION to PRODUCT_BEHAVIOR. Those are not feature questions — the OUTER subject is engineering / first-person plural. Fix: add ``_FIRST_PERSON_META_RE`` and reject the user-verify shape when a first-person-plural subject (``we`` / ``us`` / ``our`` / ``nous`` / ``notre`` / ``wir`` / ``uns`` / ``unser`` / ``nosotros`` / ``nuestro`` / ``우리`` / ``저희`` / ``私たち`` / ``私達`` / ``我们`` / ``我們``) is also present. This restores VERIFICATION precedence for the engineering / meta cases while preserving PRODUCT_BEHAVIOR routing for the user-facing siblings (``Can users verify their email?``, ``Les utilisateurs peuvent-ils vérifier leur e-mail ?``, ``用户可以验证他们的电子邮件吗?``, ``사용자가 이메일을 확인할 수 있나요?``). Adds regression coverage for the bot's three new examples (English meta verify, English meta validate, French meta verify); the existing user-verify test continues to pass. - 378 tests in tests/unit/auto pass - ruff check + ruff format --check both clean on touched files * fix(auto): cover English user actor + drop bare acceptance cue ouroboros-agent[bot] flagged commit f59d4e7 with two BLOCKING findings: 1. The actor cue list omitted English ``user`` / ``users``, and ``_is_actor_or_io_question()`` only matched the very narrow ``\b(who|which)\s+(is|are)\s+the\s+users?\b`` shape. Common prompts like ``"Who is the primary user?"`` and ``"Which user is the primary user?"`` therefore fell through to ``_default_answer()`` and never populated ``actors`` / ``inputs`` / ``outputs``. 2. ``QuestionIntent.ACCEPTANCE_CRITERIA`` carried a bare ``"acceptance"`` cue, which silently misrouted property/status questions like ``"What is the acceptance status?"`` into ``_feature_acceptance_answer()`` — the same false-positive class eliminated earlier for ``output`` / ``repository``. Fix: - Add ``user`` and ``users`` to ``_ACTOR_NOUN_CUES``. With the word-boundary cue matcher introduced earlier, ``\buser\b`` cannot substring-match ``"stakeholder"`` or other unrelated tokens; ``users`` covers the plural form. ``_has_actor_cue_with_question_shape`` already requires both an actor noun and an interrogative subject (``who`` / ``which user`` / ``primary user`` / etc.), so coverage expands without re-introducing false positives. - Drop bare ``"acceptance"`` from ``_INTENT_CUES[QuestionIntent.ACCEPTANCE_CRITERIA]``. Multilingual acceptance phrases (``criterios de aceptación`` / ``critères d'acceptation`` / ``akzeptanzkriterien`` / ``인수 조건`` / ``受け入れ基準`` / ``验收标准`` / etc.) remain. English acceptance questions stay covered by ``_is_feature_acceptance_question`` which pre-filters ``\b(acceptance|criteria)\b`` and then requires a feature shape. Adds regression coverage for the bot's three new English actor examples plus the acceptance status property-lookup case. - 380 tests in tests/unit/auto pass - ruff check + ruff format --check both clean on touched files * fix(auto): make _slug_key Unicode-aware so CJK ledger keys do not collide ouroboros-agent[bot] flagged commit 1581a7b with one BLOCKING contract- boundary regression: ``_slug_key()`` stripped everything outside ``[a-z0-9]``, so CJK questions added by this PR all collapsed to the same fallback ``requested_behavior`` key. Reproduced by the bot with ``"哪些用户可以删除分支?"`` and ``"用户可以验证他们的电子邮件吗?"``. Both produced the same ``constraints.behavior.requested_behavior`` / ``acceptance.behavior.requested_behavior`` keys, which means in a real interview unrelated requirements would silently merge into the same ledger slot — defeating the point of the new multilingual routing. Fix: switch the substitution character class to Unicode-aware ``\\w`` (``re.UNICODE`` is the default in Python 3) and use ``casefold()`` for locale-friendly case folding. Now CJK / German umlaut / Cyrillic questions each produce a distinct, descriptive slug: ``"哪些用户可以删除分支?"`` → ``constraints.behavior.哪些用户可以删除分支`` ``"用户可以验证他们的电子邮件吗?"`` → ``constraints.behavior.用户可以验证他们的电子邮件吗`` ``"Can users delete branches?"`` → ``constraints.behavior.can_users_delete_branches`` ASCII slugs are unchanged (``"the requested behavior" → "the_requested_behavior"``, ``"" → "requested_behavior"`` fallback intact). Adds a regression test covering the bot's two reproduction prompts plus key-uniqueness for distinct questions. - 381 tests in tests/unit/auto pass - ruff check + ruff format --check both clean on touched files * fix(auto): add multilingual direct-lookup runtime shape ouroboros-agent[bot] flagged commit 6a939bf with one BLOCKING regression: ``_has_runtime_context_intent`` only accepted multilingual runtime cues when paired with a *selection* verb, but ``_is_runtime_context_question`` is still English-only. As a result, bare direct-lookup runtime questions in non-English languages — ``"¿Qué framework?"``, ``"Quel framework ?"``, ``"Welches Framework?"``, ``"ランタイムは何ですか?"``, ``"框架是什么?"``, ``"런타임은 무엇인가요?"`` — fell through to ``_default_answer()`` and never populated ``runtime_context``. English equivalents (``What runtime?`` / ``Which framework?``) continued to work via the strict English shape selector. Fix: add ``_RUNTIME_DIRECT_LOOKUP_PATTERNS`` covering the multilingual direct-lookup shape: - Spanish / French / German: interrogative + optional copula/article + runtime cue, anchored to the start of the question and ending after the cue (only ``?`` and whitespace allowed at the tail) so longer property questions don't match. - Korean: cue + optional 은/는/이/가 + 무엇/뭐/어떠한. - Japanese: cue + は/の + 何/どの/どんな/なに. - Chinese (simplified + traditional): cue + (是|有)? + 什么/什麼/哪个/哪個/哪些/啥. ``_has_runtime_context_intent`` now accepts a cue match when paired with either a selection verb (existing) or a direct-lookup interrogative (new). The earlier conservative defaults for property/status questions are preserved: - ``"What is the repository status?"`` → conservative_default ✓ - ``"¿Qué estructura usamos para los datos?"`` → conservative_default ✓ (``estructura`` is no longer a runtime cue) - ``"What architecture decisions are documented?"`` → conservative_default ✓ - ``"런타임 설정은 어디에 표시되나요?"`` → conservative_default ✓ - ``"运行时配置显示在哪里?"`` → conservative_default ✓ Adds regression coverage for the bot's six bare-lookup prompts. - 382 tests in tests/unit/auto pass - ruff check + ruff format --check both clean on touched files * fix(auto): route precedence preserves verification for meta-verify ouroboros-agent[bot] flagged commit 5e60302 with one BLOCKING correctness regression: PRODUCT_BEHAVIOR was demoting VERIFICATION whenever any product-behavior matcher fired, including the broader English (``should…delete``), French (``peuvent…supprimer``), Spanish (``pueden…eliminar``), and Chinese (``可以…删除``) permission patterns that legitimately match the INNER permission clause of meta-verify questions. Reproductions: ``"Should we verify users can delete branches?"`` ``"Devrions-nous vérifier que les utilisateurs peuvent supprimer des branches?"`` ``"¿Deberíamos verificar que los usuarios pueden eliminar ramas?"`` ``"我们是否应该验证用户可以删除分支?"`` These all changed the ledger contract from "how do we test this" to "implement this behavior". Fix: 1. Demote VERIFICATION / ACCEPTANCE_CRITERIA in favour of PRODUCT_BEHAVIOR ONLY when the question matches the user-verify shape itself (``_has_user_verify_feature_shape``: actor + permission modal + verify-verb, AND no first-person-plural meta subject). Other product-behavior matchers no longer cause demotion, so meta- verify questions whose inner clause matches a permission pattern keep their VERIFICATION precedence. 2. Add Spanish (``verificar`` / ``comprobar`` / ``confirmar``) and German (``verifizieren`` / ``validieren`` / ``bestätigen`` / ``bestaetigen``) verify-verb infinitives to ``_INTENT_CUES[QuestionIntent.VERIFICATION]``. The classifier already recognised English / French verify cues; this restores symmetry for the languages used in the bot's reproductions. 3. Extend ``_FIRST_PERSON_META_RE`` with Spanish ``-mos`` and French ``-ons`` first-person-plural verb forms (``deberíamos``, ``debemos``, ``verificamos``, ``devrions``, ``vérifions``, etc.). Spanish and French regularly drop the subject pronoun, so ``"Deberíamos verificar"`` / ``"Devrions-nous vérifier"`` need to be recognised as meta even though no explicit ``nos`` / ``nous`` is present. Existing routing for the user-facing siblings (``"Can users verify their email?"``, ``"Les utilisateurs peuvent-ils vérifier leur e-mail ?"``, ``"用户可以验证他们的电子邮件吗?"``, ``"사용자가 이메일을 확인할 수 있나요?"``) continues to route to PRODUCT_BEHAVIOR, and engineering-side ``"How should we verify the HIPAA worker tests pass?"`` continues to route through the regulated-data blocker. Adds the bot's four new reproductions to the existing meta-verify regression test. - 382 tests in tests/unit/auto pass - ruff check + ruff format --check both clean on touched files * fix(auto): drop bare what user/which user actor cues + add UI past participles ouroboros-agent[bot] flagged commit 8e0d789 with one BLOCKING regression: ``"what user"`` / ``"which user"`` actor question cues, combined with the ``user`` / ``users`` actor noun cues added earlier this iteration, silently misrouted product-behavior questions like ``"What user settings should be displayed?"`` and ``"Which user fields should be editable?"`` into ``_io_actor_answer()`` (writing ``actors`` / ``inputs`` / ``outputs`` assumptions instead of preserving the requested settings-display behavior). Fix: 1. Drop bare ``"what user"`` / ``"which user"`` from ``_ACTOR_QUESTION_CUES``. Specific actor phrases ``"primary user"`` / ``"end user"`` and bare ``"who"`` remain — they are unambiguous "who is the user" signals. Existing actor question fixtures (``"Who is the primary user?"`` / ``"Which user is the primary user?"`` / ``"Who is the end user?"``) still classify as ACTOR_IO via the ``primary user`` / ``end user`` / ``who`` cues + ``user`` actor noun. 2. Extend ``_is_product_behavior_question``'s ``be|become`` past- participle pattern with common UI / data verbs in past-participle form (``displayed`` / ``shown`` / ``rendered`` / ``hidden`` / ``stored`` / ``saved`` / ``sent`` / ``received`` / ``returned`` / ``generated`` / ``created`` / ``updated`` / ``imported`` / ``exported`` / ``validated`` / ``verified`` / ``confirmed`` / ``approved`` / ``notified``). Without this, ``"should be displayed"`` failed the existing ``\bdisplay\b`` boundary check (the ``ed`` suffix kept the word boundary inside the token), so settings questions fell to the conservative default instead of routing to PRODUCT_BEHAVIOR. Adds regression coverage for three settings/fields questions; the existing English actor + multilingual fixtures continue to pass. - 383 tests in tests/unit/auto pass - ruff check + ruff format --check both clean on touched files * fix(auto): drop possessives from meta-RE + block compliance-policy noun scope Two BLOCKING regressions flagged by ouroboros-agent[bot] on a447fc1: 1. _FIRST_PERSON_META_RE treated possessive determiners (our/ours/notre/ nos/unser*/nuestro*) as engineering meta subjects, so user-feature questions like "Can our users verify their email?" were silently demoted from PRODUCT_BEHAVIOR to VERIFICATION. Drop possessives; keep only unambiguous 1pp pronoun subjects and 1pp verb forms. CJK 1pp pronouns are now anchored to a trailing topic/subject particle so possessive use ("우리 사용자", "我们的产品") does not trip the meta path. 2. _is_safe_product_regulated_question() only rejected active-form compliance verbs (retain/store/handle/...), not the noun forms. Phrasings such as "Should the app support HIPAA data retention?" or "Should the platform enable GDPR data storage?" therefore passed the gate and silently mutated the ledger as bounded product behaviour even though they are compliance-policy decisions. Extend _BARE_COMPLIANCE_SCOPE_RE to also reject support|enable|allow + regulated-noun + (optional 'data') + compliance-policy noun (retention/storage/encryption/handling/processing/collection/ disclosure/governance/compliance/transmission/redaction). Concrete product features whose name happens to contain a policy noun ("HIPAA retention reports", "GDPR storage dashboards") are still allowed because the policy noun is followed by a qualifying feature noun. Adds 4 regression tests covering both fixes plus the inverse cases (meta-QA still routes to verification; concrete-feature variants stay unblocked). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Hermes Agent <hermes-agent@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(auto): finalize safe default interview gaps * fix(auto): widen safe-default unsafe-context scan to interview answers and active non-policy ledger Addresses bot blocking finding on src/ouroboros/auto/safe_defaults.py:183 — the unsafe-context gate previously only inspected the goal, pending question, USER_GOAL ledger entries, and past interview questions, so risky requirements introduced mid-interview (in answers or in derived REPO_FACT/INFERENCE entries) could slip past the gate and cause finalize_safe_defaultable_gaps() to mark gaps DEFAULTED on a session that should remain blocked. The scan now reads every active ledger entry across all sources, plus both questions and answers from the interview transcript. Inactive entries (weak/conflicting/blocked) and the policy's own DEFAULTED ASSUMPTION outputs remain excluded so the gate does not re-flag its own boundary text on a subsequent pass — preserving the regression guarded by test_safe_default_non_goals_do_not_make_later_finalization_unsafe. Adds two regressions: - interview answer carrying production/OAuth/credential terms keeps gaps unsafe - non-USER_GOAL ledger entry (REPO_FACT) carrying credentials/production terms keeps gaps unsafe * fix(auto): persist safe-default synthesis to interview transcript Addresses bot blocking finding on src/ouroboros/auto/interview_driver.py:257 (review of c072ed9). Previously the safe-default finalization path mutated only the in-memory SeedDraftLedger and returned "seed_ready", but the seed generator downstream (HandlerSeedGenerator → ouroboros_generate_seed) builds its prompt from the persisted interview transcript rounds — never the auto ledger. That left a split-brain where the ledger reported Seed-readiness while the transcript that the seed generator actually reads still missed those assumptions, so the generated Seed could silently omit them or a stricter backend could reject generation outright. The fix adds build_safe_default_synthesis() — a single auditable text that enumerates every defaulted section's value plus rationale — and wires the interview driver to push that synthesis through backend.answer() before declaring seed_ready. Persisting through the legitimate backend channel keeps the interview engine in charge of transcript state, so the seed generator now sees the same assumptions the ledger records. If the backend rejects the synthesis (timeout or error) the session is marked blocked with an explicit "safe-default synthesis ..." reason, preserving the existing safety contract. Also records the synthesis as a question/answer pair on the ledger's question_history so audit consumers can trace the finalization turn back to the bounded interview that produced it. Tests: - existing finalization happy-path test now asserts the synthesis text was passed to backend.answer() and recorded in question_history - new test asserts the session is blocked when the backend refuses the synthesis answer * fix(auto): drop bare 'external' from unsafe-context gate to stop benign-phrase blocks Addresses bot blocking finding on src/ouroboros/auto/safe_defaults.py:112 (review of bb0a8fc). The ambiguous-external-side-effect alternation matched the standalone token ``external``, which flagged benign goals like "no external dependencies" or "use existing external API schema files only" as unsafe and turned ordinary bounded interviews into hard blocks. The alternation now requires a concrete side-effect token (deploy, release, publish, production/prod/live target, send email, webhook, notify users, create account, delete branch, database migration) — matched phrases must imply an actual external action rather than the mere presence of the word "external" in benign descriptive context. Adds parametrised regressions covering the two phrasings the bot called out plus a third "external integration documentation already vendored" case to lock in the new boundary. * fix(auto): tighten unsafe-context detector to user assertions only Addresses two bot blocking findings on src/ouroboros/auto/safe_defaults.py (review of 952ee99): 1. Confirmed NON_GOAL entries no longer count as active unsafe scope. A user who explicitly excludes "auth, credentials, and production deployment" via non-goals is signalling those areas are *out* of scope; reading the same text as active unsafe context inverts the user's intent and turned ordinary bounded interviews into hard blocks. 2. Backend-authored interview questions (and the still-open ``pending_question``) are no longer scanned. A clarifying question such as "How should this authenticate?" or "Does this deploy to production?" does not authorize a deploy or auth flow — only the user's answer can. Continuing to feed questions into the regex bank meant a harmless clarifier could permanently poison finalization, even after the user answered "no auth, local only". The detector now inspects only user-authored assertions: the original goal, active non-NON_GOAL/non-ASSUMPTION ledger entries, and the user's interview answers. ``pending_question`` is kept on the public signature for backward-compatible call sites but is no longer fed into the regex context — documented inline. Adds two regressions covering the exact bot scenarios: - "non-goals are auth, credentials, and production deployment" finalizes cleanly instead of blocking - a question containing "authenticate"/"production" plus an unrelated ``pending_question`` mentioning "production credentials" no longer blocks when the user answer is "no auth, local only" * fix(auto): strip negated clauses before applying unsafe-context regex bank Addresses bot blocking finding on src/ouroboros/auto/safe_defaults.py:83 (review of 4f4d7a8). The unsafe-context detector was polarity-blind: phrases like ``No production deployment``, ``No authentication``, or ``Do not use customer credentials`` still matched ``production`` / ``authentication`` / ``credentials`` and incorrectly blocked safe-default finalization for users who had explicitly ruled the risky scope out. The detector now runs a negation pre-pass that blanks any clause headed by a negation cue (``no``, ``not``, ``never``, ``don't``, ``do not``, ``without``, ``none``, ``neither``, ``nor``, ``skip``, ``avoid``, ``exclude``, ``forbid``) up to the next sentence break or contrastive conjunction (``but``, ``however``, ``although``, ``except``), so the unsafe regex bank only sees positively asserted scope. Contrastive conjunctions intentionally re-open the gate — ``no prod deploys, but log credentials in env`` still flags via ``credentials`` because the second clause is a real assertion. Adds parametrised regressions for the bot's exact phrasings plus related negation forms (``Never deploy to production``, ``avoid OAuth and skip billing``, ``Without payment processing or webhook callbacks``, list-style negations like ``No auth, credentials, and production deployment``), and a contrastive case to lock the post-``but`` gate back in. * fix(auto): keep CONSERVATIVE_DEFAULT entries visible to unsafe-context gate Addresses bot blocking finding on src/ouroboros/auto/safe_defaults.py:303 (review of 1051964). The previous filter dropped every DEFAULTED entry before checking source, even though the documented trust model says active CONSERVATIVE_DEFAULT entries should still participate in unsafe-context detection. That gap let a prior auto round preserve unsafe scope (for example "Production database connection is the conservative default") inside a defaulted ledger entry, after which max-round finalization would silently treat the context as safe and mark the Seed ready. The filter now skips by source only — ASSUMPTION (the safe-default policy's own outputs and other policy assumptions) and NON_GOAL (explicit user exclusions). DEFAULTED-status entries from other sources stay visible to the gate, so a CONSERVATIVE_DEFAULT entry that authorizes production/auth/billing scope still flags. Regression test asserts a DEFAULTED CONSERVATIVE_DEFAULT entry carrying production-database scope keeps the session blocked. * fix(auto): make safe-default finalization idempotent against policy-authored answers Addresses bot blocking finding on src/ouroboros/auto/safe_defaults.py:322 (review of f0c1156). _interview_answers() returned every persisted answer in question_history, which already contains backend-authored auto answers (AutoAnswerer.apply records each one with a "[from-auto]" prefix) and this PR's own safe-default synthesis. Those policy-authored answers routinely mention authentication/credentials/production as exclusions, so a later finalize_safe_defaultable_gaps() call would re-feed them into the unsafe gate and flag the policy's own boundary text as "unsafe context" — a non-idempotent path that broke legitimate resume/finalize flows. The fix: * Tag the safe-default synthesis with the same "[from-auto]" prefix AutoAnswerer uses (via a shared _AUTO_ANSWER_PREFIX constant) plus a "[safe-default-synthesis]" sub-tag. The transcript still records the full synthesis content so the seed generator sees the assumptions. * _interview_answers() now skips any answer whose text begins with "[from-auto]" — both AutoAnswerer outputs and our own synthesis. Two regressions: - a "[from-auto]" answer carrying authentication/credentials/production exclusions does not poison finalization - finalize → record synthesis as a question_history answer → finalize again still completes cleanly (idempotence guarantee) * fix(auto): close interview transcript via completion signal in synthesis turn Addresses bot blocking finding on src/ouroboros/auto/interview_driver.py:270 (review of b2df585). Pushing the safe-default synthesis through backend.answer() correctly added the assumption text to the transcript, but the production interview handler always runs ask_next_question() after recording an answer — so the transcript gained a trailing unanswered question while auto state declared the interview complete. ouroboros_ generate_seed builds its prompt from that persisted transcript, so seed generation could see an interview that appears to continue past the synthesized answer even though the auto pipeline thought it was closed. The fix: * build_safe_default_synthesis() now begins the synthesis with the explicit completion phrase "Mark the interview complete and hand off for seed generation." That string matches GenerateSeedHandler's _INTERVIEW_COMPLETION_PHRASES table, so the production interview handler closes the session in the same turn — no trailing unanswered round. * _record_safe_default_synthesis() now reconciles the returned InterviewTurn: if the backend honours the completion signal it returns with seed_ready/completed and the driver finalises auto state; if it silently keeps asking, the driver blocks with an explicit "interview backend did not honour the safe-default completion signal" reason instead of declaring seed_ready against a still-open transcript. Test changes: - existing finalize-after-max-rounds test now models the production contract: the test backend returns completed=True when it sees the completion phrase - new test asserts the driver blocks if a backend ignores the completion signal and keeps returning fresh questions * style(auto): apply ruff format to interview_driver and test file * fix(auto): break negation scope on commas that introduce a new imperative clause Addresses bot blocking finding on src/ouroboros/auto/safe_defaults.py:266 (review of c7412eb). The previous negation scrubber was too greedy across commas: a phrase like "No production deploys, use customer credentials from Vault." or "Without billing integration, send email notifications." was blanked end-to-end, hiding the positively-asserted second clause from the unsafe regex bank. That let the safe-default policy auto-finalize contexts that genuinely authorize unsafe scope. The negation pattern now ends a clause when a comma is followed by a fresh imperative verb (use, send, log, deploy, write, call, push, pull, store, configure, connect, publish, run, expose, read, fetch, create, delete, update, sync, forward, invoke, provision, grant, access, trigger, load, save, transfer, charge, notify, encrypt, authenticate, authorize — plus inflections). That keeps fully-negated phrases like "No production deployment" and list-style negations like "No auth, credentials, and production deployment" stripped, while ``No production deploys, use customer credentials...`` keeps ``credentials`` visible. Adds a parametrised regression covering the two phrasings the bot called out plus a semicolon variant. * fix(auto): loop completion signals to satisfy backend streak contract Addresses bot blocking finding on src/ouroboros/auto/interview_driver.py:399 (review of 6487586). The previous synthesis push assumed one "mark the interview complete" turn would close the transcript, then blocked otherwise — but the production interview handler in GenerateSeedHandler / authoring_handlers.py:1478-1585 only completes after a streak of AUTO_COMPLETE_STREAK_REQUIRED (=2) qualifying completion signals. A first signal commonly returns seed_ready=False with a "type done once more" shortfall, so the previous code's strict one-turn check would have aborted the safe-default recovery against a real backend whenever the streak was at 0. The synthesis path now sends the full assumption text once (so the transcript records every defaulted section's content), then sends up to two short follow-up turns carrying just the completion phrase "Mark the interview complete and hand off for seed generation. No remaining ambiguity for safe-defaultable sections." until the backend confirms with seed_ready/completed. The cap (3 attempts total) bounds the post-finalization work even if a misbehaving backend never confirms — the driver still blocks with an explicit reason rather than spinning forever. AUTO_ANSWER_PREFIX and SAFE_DEFAULT_SYNTHESIS_TAG are exported as public constants since they now span safe_defaults and interview_driver. Test additions: - new test models the production streak contract: backend returns seed_ready=False on the first completion signal, only completes on the second; driver must successfully finalize after two qualifying turns - existing rejection test still verifies that a backend which never honours the signal causes the driver to block with the post-attempt reason * fix(auto): break negation scope on every comma when sentence has no list connector Addresses bot blocking finding on src/ouroboros/auto/safe_defaults.py:414 (review of be07800). The previous comma-break heuristic only stopped the negation scope when the next token was an imperative verb, so a noun-led counter-clause like "No production deploys, customer credentials from Vault are still required." was stripped end-to-end and the live "credentials" requirement was hidden from the unsafe regex bank — letting finalize_safe_defaultable_gaps() auto-default a security-sensitive task. The negation pattern now picks one of two scope modes via a lookahead: * List mode (sentence contains "and"/"or"/"nor" before the next sentence break): scope continues across commas so list-style negations such as "No auth, credentials, and production deployment" stay fully scoped. Comma + imperative verb and contrastive conjunctions still end the scope as before. * Non-list mode (no list connector ahead): scope ends at the FIRST comma in addition to the usual sentence breaks and contrastive conjunctions. This catches mixed clauses where the second half is a positive assertion — both imperative-led ("..., use credentials") and noun-led ("..., customer credentials are required") variants now flag. Adds the noun-led counter-clause to the parametrised regression so the bot's exact phrasing is locked in. * fix(auto): roll back safe-default ledger on synthesis failure After rebasing onto upstream/main, the new convergence-contract test from PR Q00#760 (test_convergence_contract_stalled_generic_followups_report_ actionable_gaps) revealed that when the safe-default synthesis cannot be persisted to the interview transcript, leaving the ledger in its finalized "DEFAULTED everything" state hides the genuinely unresolved sections from downstream consumers — the test asserts that stalled interviews must block with an "unresolved gaps" reason that names at least one open required section. The driver now rolls back the safe-default policy's own ledger entries (those with key suffix ``.safe_default_finalization``) on the synthesis failure path so: * ``ledger.open_gaps()`` exposes the original unresolved sections, and * the canonical ``auto interview reached max rounds with unresolved gaps: <sections>`` blocker is emitted instead of a synthesis-specific message. ``_record_safe_default_synthesis`` no longer calls ``mark_blocked`` itself — it just returns the failure reason. The caller emits a single unified block transition after rolling back, which also fixes a ``blocked → blocked`` invalid-transition error surfaced by chained ``state.mark_blocked`` calls. Tests updated: - synthesis-rejection regressions now assert the rolled-back blocker ("unresolved gaps") plus that ledger.open_gaps() is non-empty and no ``.safe_default_finalization`` entries survive * fix(auto): keep pending_question synced with backend across synthesis turns Addresses bot blocking finding on src/ouroboros/auto/interview_driver.py:490 (review of cc12842). _record_safe_default_synthesis() advanced the backend by one or more turns but never updated state.pending_question, so on a later --resume AutoInterviewDriver.run() (interview_driver.py:135-149) trusted the stale cached prompt and answered against an interview session that had already moved on — replaying an already-answered question or mis-targeting the next round. The driver now syncs state.pending_question with each synthesis turn: * On every successful backend.answer() call, set pending_question to the returned turn.question (or None if the backend has nothing pending). Save state after each turn so a crash mid-loop does not lose the sync. * On TimeoutError or other exceptions, the backend may or may not have processed the call — the cached pending_question is no longer trustworthy, so clear it so resume queries live state via backend.resume() instead of replaying. * On exhausted-attempts failure, pending_question already reflects the backend's latest live prompt from the last successful turn, so resume re-enters at the correct round. Tests: - existing rejection regression now also asserts state.pending_question is None when backend.answer raises (forces resume via backend.resume) - existing ignored-completion regression now also asserts state.pending_question equals the backend's final live question after the synthesis attempts are exhausted * fix(auto): drop bare production/prod/live tokens from unsafe-context gate Addresses bot blocking finding on src/ouroboros/auto/safe_defaults.py:118 (review of f8fa1f9). The "ambiguous external side effect" alternation matched bare ``production``, ``prod``, and ``live``, so benign goals that merely *mention* those words for read-only context — "reproduce a production bug locally", "use the production schema snapshot already in the repo", "compile the live preview build", "document the prod logging format" — were misclassified as unsafe and turned ordinary bounded interviews into hard blocks. The pattern now requires an actual side-effect verb or phrase: ``deploy``, ``release``, ``publish``, ``send email``, ``webhook``, ``notify users``, ``create account``, ``delete branch``, ``database migration``, plus the production-cutover idioms ``go live``, ``going live``, and ``push live``. ``deploy``/``release``/``publish`` already catch real production-class actions ("deploy to production", "production deploy", "release to prod", "publish to npm"), so dropping the bare nouns does not weaken coverage of genuine deploy/release flows. Tests: - new parametrised regression for benign production/prod/live mentions (reproduce-bug, schema snapshot, captured trace replay, prod logging doc, live preview build) asserts finalization completes cleanly - new parametrised regression for genuine production actions (deploy/release/push live/going live) asserts finalization blocks with the "external side effect" reason - existing CONSERVATIVE_DEFAULT regression updated to use a phrase that still matches the tightened pattern ("deploys to production with customer credentials") so the original intent — DEFAULTED entries authorizing unsafe scope must still block — keeps locked in --------- Co-authored-by: Hermes Agent <hermes-agent@users.noreply.github.com>
…mmand-name index (Q00#747) (Q00#747) Squashed port of Q00#747 onto current main. main already had the basic UserLevel registry from the Q00#750 squash; this lands the post-stack bot review fixes that did not propagate through Q00#750: - Cross-axis identifier collision check (namespace / plugin name / command name treated as one lookup space). - Within-manifest duplicate command name rejection. - Global command-name uniqueness across plugins. - _command_owner index for O(1) get_by_command() lookup. - Transactional replace=True across all three indexes. Closes Q00#730.
…ronments (Q00#768) Surface package-manager paths (pipx/pip/brew) alongside the existing vendor pipe-to-shell so users and agents in environments that disallow `curl | sh` have a first-class install path. - scripts/install.sh: replace single-line uv install hint in both pipx and pip-fallback branches with a four-option block (pipx > pip > brew > vendor one-liner). - skills/setup/SKILL.md: same four-option block in the missing-deps and missing-prereqs prompts so the setup skill prescribes the package- manager path first when the environment supports it. - SECURITY.md: add Installation Channel and Bulk-Disclosure Scanner Policy sections documenting the canonical install paths and the severity ladder for `curl | sh` pattern reports. Refs Q00#766. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tarvation (Q00#770) The nested ouroboros_interview MCP entrypoint paired max_turns=1 with the read-only policy envelope (Read/Glob/Grep/...) introduced by d7bbbf0 ("Enforce read-only policy envelopes for MCP handlers"). On modern Claude (Opus 4.x) the model frequently picks a Read tool call as its first turn, exhausts the single-turn budget, and the SDK raises "Reached maximum number of turns (1)" before any final question can stream — failing ~90% of ouroboros_interview calls in v0.36.0. Match the prior-art pattern from PMInterviewHandler._get_engine (pm_handler.py): wire the question-generation adapter with allowed_tools=[] when the backend supports a tool envelope, and None for envelope-unaware backends (hermes). The _interview_allowed_tools helper is retained for a follow-up policy refactor (see issue Q00#765 discussion); a separate change should teach orchestrator/policy.py to honor the max_turns budget so the read-only envelope cannot be reintroduced under a single-turn adapter. Adds tests/unit/mcp/tools/test_interview_allowed_tools_wiring.py to lock the wiring against future "policy consistency" sweeps. Fixes Q00#765 Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…only) (Q00#805) Step 1 of the audit-fidelity plan that replaces the earlier Q00#801 truncate-with-sentinel approach. The previous PR Q00#801 capped argv at 4 KiB per element / 16 KiB total and replaced overflow with `<truncated:N>` sentinels. Multi-persona review (lateral_think) and the bot reviewer converged on the same critique: the cap values were picked without measurement, and silent truncation hides exactly the data the audit log exists to provide. This change does NOT cap or truncate. It only adds a fingerprint alongside the existing `argv` field so we can size the actual distribution before deciding any cap or spill policy: "command": { "namespace": "...", "name": "...", "argv": [...], # unchanged, full content "argv_summary": { "argc": 2, "byte_length": 31, "sha256": "ab12...c0d3" # 64-hex } } Hashing uses NUL as the element separator so two argv lists with the same concatenation cannot collide on sha (`["ab", "cd"]` vs `["abcd"]`). `byte_length` excludes the separators so the number reflects the operator-visible payload size. After 1–2 weeks of real `plugin.invoked` events with these summaries, a follow-up PR can pick a cap (or a content-addressed spill design) backed by the actual p99/p999 distribution rather than magic numbers. The full argv field is removed from the event ONLY in that follow-up, once the policy is data-driven. Schema additive — `argv_summary` added under `command` with `additionalProperties: false` on the inner object. Existing payloads without the new field continue to validate. Closes the conversation thread that produced PR Q00#801; tracked as the "step 1" follow-up of that closed PR. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…0#804) `_revert_safe_default_entries` filtered with `entry.key.endswith(".safe_default_finalization")` — but the only producer of that suffix, `finalize_safe_defaultable_gaps`, writes the exact key shape `{section}.safe_default_finalization`. The suffix-match form would also delete an unrelated user/answerer-authored entry whose key happened to share the suffix (for example, an answerer-synthesized constraint key `constraints.my.safe_default_finalization` would be wiped on synthesis-failure rollback). Switch to `entry.key == f"{section_name}.safe_default_finalization"`. This matches every entry the policy wrote (it is the SOLE producer of the canonical key) and matches only those entries — no user/answerer-authored ledger state is at risk on a synthesis failure rollback. Adds `test_revert_safe_default_entries_preserves_user_keys_with_matching_suffix` covering three keys in one section: the canonical policy entry (must go), a user entry sharing the suffix (must survive), and an unrelated entry (control). Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Q00#800) `re.escape(pattern)` quotes the pattern's inner punctuation literally, but the ``\\b`` boundaries on either side were looking only for word-character ↔ non-word-character transitions. Both ``-`` and ``_`` are non-word, so a pattern like ``"ooo resume-session"`` matched ``"ooo resume-session-extra"`` because ``\\b`` fired at the boundary between ``session`` and the trailing ``-``. Effect: a future hypothetical sibling command such as ``ooo resume-session-cleanup`` (or even a contributor-typo'd ``ooo resume-session-please-help``) would silently route to ``/ouroboros:resume-session``, masking the typo and hiding the mis-routed command from the user. Reproduction: >>> _word_boundary_match("ooo resume-session", "ooo resume-session-extra") True # bug — should be False Fix: replace ``\\b`` on both sides with explicit ``[^\\w-]`` (or start/end of string), so a hyphen is no longer a valid boundary character. Existing positive cases (``"ooo resume-session "``, ``"ooo resume-session."``, ``"ooo resume-session: please"``) still match. ``myooo auto`` continues to be rejected because ``y`` is a word character and now does not satisfy the leading boundary. Adds two regressions: * hyphen-extension does NOT collapse onto canonical command * trailing space / period / colon / newline / EOS still match Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…g list (Q00#798) `list` called `_read_trust_or_exit` on every readable-manifest row, but that helper raises `typer.Exit(1)` on a `json.JSONDecodeError` — so a single plugin's corrupt `trust.json` aborted the entire listing and hid every other installed plugin from the operator. The diagnostic purpose of `list` is to show what is installed; collapsing the output on the very file the operator needs to find is exactly backwards. The companion unreadable-manifest branch already documented this boundary inline (lines 391–401) and silently degrades to `record = None` so the row stays in the output. The trust path now mirrors that contract: catch `_STATE_FILE_ERRORS` and set `record = None`. The single-target `inspect` command's exit-on-error behavior is unchanged because `_read_trust_or_exit` is still reached there. Reproduction: echo '{not valid json' > ~/.ouroboros/plugins/bad-plugin/trust.json ooo plugin list # before: SystemExit(1), no output # after: bad-plugin row degraded, others listed Adds `test_list_survives_corrupt_trust_with_readable_manifest` covering a 2-plugin layout where one plugin's `trust.json` is invalid JSON. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`wrap_plugin_event` defensively `copy.deepcopy`s the input audit event
on the way into the envelope (line 125), but the inverse
`unwrap_plugin_event` returned only `dict(payload)` — a shallow copy.
A caller that mutated a nested container on the unwrapped value (e.g.
``result["plugin"]["name"] = "X"`` or ``result["command"]["argv"].append(...)``)
wrote through to the live envelope still referenced by the event store,
silently corrupting the audit log with no API surface to detect the
divergence.
Reproduction:
>>> ev = {"event_type": "plugin.invoked", "plugin": {"name": "foo"}, ...}
>>> env = wrap_plugin_event(ev, correlation_id="c")
>>> out = unwrap_plugin_event(env)
>>> out["plugin"]["name"] = "MUTATED"
>>> env["payload"]["plugin"]["name"]
'MUTATED' # bug — envelope corrupted
Fix: return ``copy.deepcopy(payload)`` to mirror the wrap-side defense.
``envelope_to_base_event`` (line 269) already uses ``copy.deepcopy``;
this restores symmetry across the three boundaries.
Adds `test_unwrap_returned_value_is_isolated_from_envelope` covering
mutations on every nested container the audit event has.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…#794) * fix(auto): NFKC-normalize unsafe-context input before regex bank `_unsafe_context_reason` lower-cased the joined goal/ledger/answer text but did not apply Unicode compatibility normalization, so fullwidth Latin (U+FF21..U+FF5A) — produced routinely by CJK IMEs and visually indistinguishable from ASCII — bypassed every `\b(deploy|production|...)\b` pattern in the unsafe-context regex bank. Reproduction: >>> from ouroboros.auto.safe_defaults import _unsafe_context_reason >>> from ouroboros.auto.ledger import SeedDraftLedger >>> _unsafe_context_reason( ... SeedDraftLedger(), ... goal="deploy to production", ... pending_question=None, ... ) None # bug: should return "ambiguous external side effect" Effect: a goal authorizing a production deploy that arrives as fullwidth Latin sails through the safe-default gate, and `finalize_safe_defaultable_gaps` marks the session ready instead of blocking. Same class of bypass affects the `fi`-ligature (U+FB01) and other NFKC compatibility forms (e.g. fl, fi, ½, ㍻). Fix: wrap the joined context in `unicodedata.normalize("NFKC", ...)` before lower-casing. NFKC collapses every compatibility variant onto its canonical ASCII form, so the existing regex bank covers them without rewriting any pattern. The negation pre-pass and the regex bank both run on the normalized form, so the negation scrubber is not regressed. Adds `test_safe_default_blocks_unicode_compat_production_actions` covering the fullwidth and ligature variants. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(test): correct NFKC regression docstring to match current matcher ouroboros-agent[bot] non-blocking suggestion on PR Q00#794 — the test docstring described the bypass as a failure of ``\b(deploy|production|...)\b``, but the current ``_UNSAFE_CONTEXT_PATTERNS`` external-side-effect arm no longer flags bare ``production`` / ``prod`` / ``live``. The verb tokens (``deploy`` / ``release`` / ``publish`` / ``go live`` / ``push live`` / ``database migration`` / ...) are what carry the block decision now. Tighten the docstring so the test's intent matches the actual regex bank and does not mislead the next maintainer. No behavior change. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…elper + manifest tuple ordering (Q00#807) Three small, contract-shaped improvements to plugin internals. No CLI surface change; this is foundation work the mutating ``ooo plugin {disable,trust,...}`` commands stack on. Splitting these into a focused PR keeps each concern reviewable in isolation. ## TrustStore: per-plugin lock covers the whole revocation surface Previously only ``grant()`` and ``reset_for_subject_change()`` ran inside ``_grant_lock``. ``remove()``, ``write_disable()``, and ``clear_disable()`` mutated ``trust.json`` / ``disabled.json`` outside any critical section, so concurrent ``ooo plugin disable`` + ``ooo plugin trust`` could interleave to produce a final state where ``disable`` reported success but ``trust.json`` was either preserved or re-created. This PR: - Wraps every revocation entry point (``remove``, ``write_disable``, ``clear_disable``) in ``_grant_lock``. - Adds lock-free internal bodies (``_remove_locked``, ``_write_disable_locked``, ``_clear_disable_locked``) so ``wipe_subject()`` can do trust + disable wipe inside ONE critical section without re-entering ``flock`` (which Linux/macOS treat as separate locks per FD and would deadlock). - Adds two atomic primitives the CLI needs: - ``apply_disable(name, source_type, source_identity, ...)`` — write disable record AND remove trust file in one critical section (the correct shape for ``ooo plugin disable``). - ``grant_and_clear_disable(name, scope, ...)`` — write trust grant AND clear any disable record in one critical section (the correct shape for the LAST step of ``ooo plugin trust``). - Tightens ``_subject_matches`` so a pre-RFC ``trust.json`` (blank ``source_type`` / ``source_identity`` / ``artifact_digest``) cannot be silently "upgraded" by a fresh subject-bound grant — any blank column is a hard mismatch when the caller plumbs the corresponding column. Test fixtures that don't plumb subject columns keep the version-only behavior. - Validates plugin name on every disable API (``is_disabled``, ``is_disabled_for_subject``, ``read_disable``, ``write_disable``, ``clear_disable``, ``apply_disable``). Lockfile rows are operator-editable and ``Lockfile.read()`` does not enforce the name regex, so ``../../x`` would otherwise escape ``DEFAULT_TRUST_ROOT`` via the disable surface. ## LockEntry.subject_for_disable() — canonical fallback The disable contract has two boundaries that must agree on the same ``(source_type, source_identity)`` tuple — the write side (``ooo plugin disable``) and every read side (dispatcher, ``inspect``, ``list``). Pre-RFC lockfile rows have empty subject columns; both sides MUST use the same fallback or a ``disable`` write lands under one subject and the dispatcher's ``is_disabled_for_subject`` check runs against another. Hoisting the fallback into a method on ``LockEntry`` lets every CLI surface share the rule without a cross-module cycle. ## Manifest: tuples, not frozensets ``PluginManifest.{capabilities,permissions}`` are tuples again so iteration order matches the manifest's declaration. ``frozenset`` reintroduces nondeterministic order in CLI output and emitted ``plugin.permission_used`` events for multi-entry manifests. No call site relied on set-only semantics — every existing reader is a list comprehension or a set construction. ## Tests - ``test_disable_apis_validate_plugin_name``: every disable API rejects malformed identifiers. - ``test_apply_disable_is_atomic``: post-condition observable in one step. - ``test_grant_and_clear_disable_is_atomic``: same for the trust side. - ``test_grant_resets_legacy_unbound_record_on_subject_bound_grant``: prevents legacy-record rehydration. - ``test_revocation_serializes_with_grant``: concurrent disable + remove + grant produce no scope inheritance. - ``test_wipe_subject_atomic_against_concurrent_grant``: wipe + grant cannot leave the forbidden ``trusted, enabled`` state. - ``test_subject_for_disable_*``: three cases covering populated columns, legacy local rows, legacy git rows. All 1163 plugin + CLI tests pass; lint and format clean.
…gacy trust under subject contract (Q00#808) * fix(plugin/firewall): fail-closed on tampered plugin home + refuse legacy trust under subject contract Two focused fail-closed improvements to ``firewall.invoke_plugin``. No CLI surface change. ## 1. Widen digest verification to handle the full tamper exception family ``canonical_tree_hash`` raises a family of post-install-tamper exceptions, not just ``FileNotFoundError``. The previous handler let ``NotADirectoryError`` (plugin_home replaced with a regular file), ``EscapingSymlinkError`` (symlink escaping the plugin root), ``UnsupportedFileTypeError`` (device/FIFO/socket inside plugin_home), and ordinary ``OSError`` (permissions, broken symlink loops, generic I/O) escape past the firewall — skipping the required terminal ``plugin.failed`` event and crashing the caller instead of refusing the invocation. All four cases now fail closed with ``result.status="trust_subject_changed"`` and a distinguishing ``provenance.reason``, so the audit trail records the exact tamper signal instead of a stack trace: - ``plugin_home_missing`` — file-system path gone or replaced with a non-directory. - ``plugin_home_tampered`` — symlink escape or unsupported file type detected. - ``plugin_home_unreadable`` — permission/IO error. Regressions: ``test_digest_fails_closed_when_plugin_home_replaced_with_file``, ``test_digest_fails_closed_on_escaping_symlink``. ## 2. Refuse legacy trust records when the dispatcher plumbs the install subject ``_record_matches_subject`` previously accepted blank ``source_type`` / ``source_identity`` / ``artifact_digest`` columns on the trust record as compatible with any same-version reinstall. That meant an operator who already had a pre-RFC trust grant could reinstall the same name+version from a different repo / a different local path / different bytes and the legacy grant would silently inherit, defeating the whole trust-subject binding. When the caller plumbs ``expected_source_identity`` or ``expected_artifact_digest`` (production CLI dispatch path), the helper now requires every subject column on the record to be populated. Records that are silent on any column cannot prove they were granted for THIS install subject, so they are refused. Firewall unit tests that don't plumb ``expected_*`` keep the legacy version-only contract for backwards compatibility — the existing ``test_artifact_digest_match_proceeds`` is updated to grant under the full subject (which is what production CLI does anyway), and a new ``test_legacy_trust_record_refused_when_subject_plumbed`` locks in the refusal path. All 1157 plugin + CLI tests pass; lint and format clean. * fix(cli/plugin): align _describe_trust_state with firewall's stricter subject contract; cover UnsupportedFileTypeError Addresses the bot's BLOCKING + non-blocking findings on commit fe1d33b. BLOCKING — CLI helper diverged from firewall (plugin.py:200-220): This PR's stricter ``_record_matches_subject`` refuses pre-RFC trust records (blank ``source_type`` / ``source_identity`` / ``artifact_digest``) once the dispatcher plumbs the install subject. The CLI's ``_describe_trust_state`` was unchanged and still treated the same record as ``"trusted"``, so ``ooo plugin inspect`` / ``ooo plugin list`` would report a trusted plugin while the firewall blocks invocation for the same install subject — a direct contract regression at the changed boundary that the helper's docstring explicitly promises against. Fix: when the caller plumbs an expected subject column (production CLI), the helper now requires every subject column on the record to be populated. Records that are silent on any column read as ``"installed"`` here just as the firewall refuses them. Legacy CLI tests that pass ``expected_*=None`` keep their version-only path so the firewall's standalone fixtures stay green. Regression: ``test_describe_trust_state_refuses_legacy_record_when_subject_plumbed`` exercises both call shapes (subject not plumbed → "trusted" legacy compat; subject plumbed → "installed" matching firewall). Non-blocking — UnsupportedFileTypeError coverage gap (test_firewall.py:663): Added ``test_digest_fails_closed_on_unsupported_file_type``: a FIFO inside ``plugin_home`` triggers ``canonical_tree_hash`` → ``UnsupportedFileTypeError`` and the firewall must fail closed with ``trust_subject_changed`` and ``provenance.exception_type == "UnsupportedFileTypeError"``. Skips on Windows where ``mkfifo`` is unavailable. Local: 1159 plugin + CLI tests pass; lint and format clean. * fix: hide stale legacy plugin grants under subject contract * fix: fail closed on plugin home symlink loop --------- Co-authored-by: Hermes Agent <hermes-agent@users.noreply.github.com>
…00#809 P1) (Q00#811) * feat(auto): user_preference source + deterministic ambiguity floor Phase 1 of RFC Q00#809 closes two gaps in `ooo auto` determinism without architectural changes. Gap 5 — caller-supplied user preferences flow into the deterministic answer policy: - `LedgerSource.USER_PREFERENCE` and `AutoAnswerSource.USER_PREFERENCE` - `AutoAnswerContext.user_preferences: Mapping[str, str]` keyed by ledger section name; only the Driver/MCP layer populates it - `_maybe_apply_user_preference` upgrades upgradable answer sources (CONSERVATIVE_DEFAULT / ASSUMPTION / EXISTING_CONVENTION) to USER_PREFERENCE when the section matches the question's intent; REPO_FACT, USER_GOAL, NON_GOAL are preserved as stronger sources - USER_PREFERENCE is in `_EVIDENCE_BACKED_SOURCES` (explicit user statement) AND in `_RISKY_FALLBACK_SOURCES` so it cannot bypass the safety gate for PII/GDPR/HIPAA/destructive-bulk topics - New MCP arg `user_preferences` validated against REQUIRED_SECTIONS and injected via custom context_provider in auto_handler Gap 4 — code can enforce a lower bound on `ambiguity_score`: - `deterministic_floor(ledger)` = 0.05*open_gaps + 0.10*conflicting + 0.05*assumption_only_ratio, clamped to [0,1] - `pipeline.py` applies `max(LLM_score, floor)` after seed generation, before review — sealing LLM self-rationalization at the A-grade gate - New `SeedDraftLedger.count_active_conflicting_entries()` helper Tests: 22 new tests in tests/unit/auto/test_user_preference_and_floor.py covering enum round-trip, evidence_backed surface, floor formula and clamping, answerer upgrade rules, safety-gate bypass prevention for regulated-data and destructive-bulk questions, pipeline integration (floor applies / floor preserves), and MCP arg plumbing. Verification: 22/22 Phase 1 tests pass; 1234/1234 auto+MCP regression; ruff and mypy clean on touched modules. Out of scope (Phase 1): - Skill-side memory file parsing (caller passes preferences via MCP arg) - Conflict resolution by source priority — Phase 4 - DomainProfile abstraction — Phase 3 Refs: Q00#809 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auto): determinism + resume contract for user_preferences (Q00#811) Two BLOCKING findings from automated review on PR Q00#811. (1) `_section_hints_for_intents()` iterated the input ``frozenset[QuestionIntent]`` directly. Python set iteration is hash-randomized, so for a question classifying into multiple intents with multiple matching preferences, the "first matching section" varied across processes and broke the answerer's determinism contract. Fix: iterate ``_INTENT_TO_SECTIONS.items()`` (Python 3.7+ guarantees dict-insertion order) and check ``intent in intents``. Output now depends only on the dict definition order. (2) ``user_preferences`` were threaded into the Driver for the current MCP call only — never persisted onto ``AutoPipelineState``. On ``ooo auto --resume``, the session silently lost its preferences unless the caller resupplied them, so later interview rounds could converge to a different Seed than the original session. A real behaviour regression for a resumable bounded workflow. Fix: add ``user_preferences: dict[str, str]`` to ``AutoPipelineState`` with empty default + JSON round-trip + validation. Backfilled with ``setdefault({})`` for legacy state files. ``auto_handler._run`` now persists the validated value on first call and prefers ``state.user_preferences`` on resume; a caller can still override by supplying a new value on the resume call. Tests added (6, total Phase 1 tests now 28): - determinism: order is dict-definition order across multi-intent input - determinism: 50 repeated calls return identical result - state: round-trip via to_dict/from_dict - state: legacy dump (no user_preferences key) loads with empty default - state: validation rejects empty key / empty value - store: end-to-end save → load preserves user_preferences Verification: 28/28 Phase 1 tests pass, 1240/1240 auto+MCP regression, ruff and mypy clean. Refs: Q00#811 (review by ouroboros-agent[bot]), Q00#809 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * style(auto): apply ruff format to Phase 1 files CI runs in addition to . The previous commits passed lint but not formatting. No semantic change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auto): close USER_PREFERENCE bypass on early-return safety paths Bot review BLOCKING finding on commit 374744c: > The new USER_PREFERENCE upgrade on the VERIFICATION / ACCEPTANCE_CRITERIA > branches returns early, so those answers never reach the risky-fallback > safety gate later in answer(). That lets caller-supplied > user_preferences[verification_plan] or [acceptance_criteria] land as > confirmed ledger entries for regulated or destructive prompts that > should still be blocked or rerouted. Real bypass: a user could supply `user_preferences["verification_plan"] = "skip the audit log"` for "How should we verify PII deletion?" and have it land in the ledger as a CONFIRMED USER_PREFERENCE entry, because the VERIFICATION early-return path in AutoAnswerer.answer() bypasses the risky-fallback gate that runs after the routing block. Fix: make _maybe_apply_user_preference itself the chokepoint. The helper now accepts `question` and `lowered` kwargs; before performing the upgrade, it calls _risky_fallback_blocker_for and returns a BLOCKER answer when a regulated/destructive pattern matches. Same policy now fires regardless of which call site requested the upgrade — early-return paths (VERIFICATION, ACCEPTANCE_CRITERIA, answer_gap) and routing-block path are equally protected. Call sites updated: - AutoAnswerer.answer VERIFICATION early-return: passes question+lowered - AutoAnswerer.answer ACCEPTANCE_CRITERIA early-return: passes question+lowered - AutoAnswerer.answer routing-block: passes question+lowered - AutoAnswerer.answer_gap: passes the synthetic gap question (no risk patterns match these, but consistency keeps the chokepoint single-source) Tests added (3, total 31): - test_user_preference_for_verification_does_not_bypass_pii_gate - test_user_preference_for_acceptance_criteria_does_not_bypass_destructive_gate - test_user_preference_for_verification_on_safe_question_still_upgrades (negative control: benign verification question still upgrades) Verification: 31/31 Phase 1 tests pass, 1260/1260 auto+MCP regression, ruff check + format + mypy clean. Refs: Q00#811 (review by ouroboros-agent[bot]), Q00#809 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auto): close USER_PREFERENCE bypass via answer_gap synthetic prompts Bot review BLOCKING finding on commit 83a8a03: > The new USER_PREFERENCE safety gate still has a bypass on the > answer_gap() path. When interview steering falls back to gap-filling, > this method replaces the original risky question with a generic > synthetic prompt before calling _maybe_apply_user_preference. The > blocker logic only inspects that generic text, so for a > regulated/destructive task a preference such as > verification_plan="skip the audit log" will still be upgraded to a > confirmed USER_PREFERENCE entry instead of being blocked. Real bypass: a goal like "Build a tool that exports PII records for compliance review" + interview-driver gap steering would trigger answer_gap("verification_plan", ...). The synthetic gap-fill prompt ("Which command output verifies the acceptance criteria?") contains no PII keywords, so _risky_fallback_blocker_for matches nothing and a USER_PREFERENCE upgrade ("skip the audit log") would land as a confirmed ledger entry. Fix: extend _maybe_apply_user_preference with an optional goal_text kwarg. The helper now runs _risky_fallback_blocker_for against BOTH the current question and goal_text, blocking the upgrade when either matches a regulated/destructive pattern. answer_gap derives goal_text from _latest_resolved_goal(ledger) and passes it through, so the synthetic prompt cannot erase the original risky context. The routing-block path leaves goal_text empty (the user's actual question already preserves the context). Tests added (2, total 33): - test_answer_gap_does_not_bypass_safety_via_synthetic_question — goal "exports PII records" + verification_plan preference → BLOCKER - test_answer_gap_with_safe_goal_still_upgrades_user_preference — benign goal → USER_PREFERENCE upgrade still works (negative control) Verification: 33/33 Phase 1 tests pass, 1262/1262 auto+MCP regression, ruff check + format + mypy clean. Refs: Q00#811 (review by ouroboros-agent[bot]), Q00#809 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auto): preserve USER_PREFERENCE on safe-product allowlisted questions Bot review BLOCKING finding on commit 6c9a96c: > USER_PREFERENCE was added to _RISKY_FALLBACK_SOURCES, so after > _maybe_apply_user_preference() upgrades a safe regulated-product > question, the post-check immediately reroutes it to > _product_behavior_answer() and drops the user's value. That means > allowlisted questions like "Should the app export PII reports?" can > no longer honor matching user_preferences even though the feature > contract says they should. Real regression: the safe-product re-route at the routing-block tail treats every member of _RISKY_FALLBACK_SOURCES as a "generic fallback that should be replaced by feature semantics". USER_PREFERENCE is NOT a generic fallback — it carries the caller's specific statement about the feature — so collapsing it back into the _product_behavior_answer template silently erases that statement. Root cause: redundancy between two policies. The helper _maybe_apply_user_preference now runs _risky_fallback_blocker_for at upgrade time (against both the question text and the converged goal). By the time an answer carries source=USER_PREFERENCE it has already passed exactly the same policy that the routing-block post-check applies to other risky-fallback sources. Including USER_PREFERENCE in the set therefore did nothing for risky questions (the helper already blocked them) but actively broke safe-product questions. Fix: remove USER_PREFERENCE from _RISKY_FALLBACK_SOURCES. Keep the helper as the single chokepoint. The earlier safety tests (regulated data / destructive bulk) still pass because the helper still emits BLOCKER for those — independent of set membership. Tests added (2, total 35): - test_user_preference_preserved_for_safe_regulated_product_question — goal "Build a privacy dashboard" + question "Should the app export PII reports?" + constraints preference → USER_PREFERENCE survives, user's value is in the answer text - test_user_preference_safe_account_deletion_question_preserved — second allowlisted pattern (account deletion permissions) → same Earlier test_user_preference_does_not_bypass_regulated_data_blocker (PII storage / destructive truncate / GDPR deletion) still passes — those questions are NOT on the safe-product allowlist, so _risky_fallback_blocker_for returns a real blocker and the helper short-circuits to BLOCKER as before. Verification: 35/35 Phase 1 tests pass, 1264/1264 auto+MCP regression, ruff check + format + mypy clean. Refs: Q00#811 (review by ouroboros-agent[bot]), Q00#809 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…0#791) * feat(auto): chain RUN→RALPH automatically with --complete-product Closes Q00#773. Adds the opt-in `--complete-product` / `complete_product=True` flag that chains RUN → RALPH_HANDOFF after a successful run handoff so a single `ooo auto` invocation iterates Ralph until QA passes, convergence, or a budget bound trips. Default-off behavior is byte-identical to pre-Q00#773. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auto): harden Ralph handoff resume edges * fix(auto): close Ralph handoff deadline and resume gaps * fix(auto): close Ralph deadline propagation + complete_product resume gaps (Q00#791 review-3) Addresses both blocking findings from ouroboros-agent[bot]'s review of commit 027dbc4: 1. **Cap Ralph per-iteration timeout at remaining pipeline budget** (`src/ouroboros/auto/pipeline.py`). `_handoff_to_ralph()` previously forwarded only `max_total_seconds` to `ouroboros_ralph`. `RalphLoopRunner` checks that budget only at the top of each iteration, so a 60s remaining deadline could still let the very first `evolve_step` block for the full 1800s default `per_iteration_timeout_seconds` — violating the top-level pipeline deadline contract pinned by Q00#779. The auto layer now forwards `per_iteration_timeout_seconds = max(_MIN_RALPH_PER_ITERATION_SECONDS, min(_DEFAULT_RALPH_PER_ITERATION_SECONDS, remaining))` alongside the existing `max_total_seconds`. The cap is floored at 30s (the Ralph handler's own minimum) and saturates at the 1800s default for callers with ample budget, so the worst-case overshoot past `deadline_at` is one iteration of up to 30 seconds. 2. **Persist `complete_product` as durable session state** (`src/ouroboros/auto/state.py`, `src/ouroboros/cli/commands/auto.py`, `src/ouroboros/mcp/tools/auto_handler.py`, `src/ouroboros/auto/pipeline.py`). `--complete-product` was previously a transient invocation flag: a session originally started with it but resumed without re-passing the flag silently fell back to the legacy RUN → COMPLETE path and skipped Ralph entirely. That is a behavioral regression for resumed sessions, not just a UX issue. `complete_product: bool = False` is now persisted on `AutoPipelineState`. CLI/MCP entry points write the operator's choice into the state on initial run and honor the persisted value on resume, mirroring the existing `--max-*-rounds` policy: lowering on resume is rejected; absence keeps the persisted truth. `AutoPipeline.run()` promotes from persisted state when called via direct in-process construction so test/library callers get the same behavior. Also reorders pipeline startup so that a malformed persisted Seed artifact is converted to a clean FAILED outcome via `_mark_invalid_seed_artifact` *before* the new legacy-deadline-arming save validates the full state. Without this reorder, a session with an invalid `seed_artifact` would raise raw `ValueError` from the very first `AutoStore.save` and break `tests/unit/auto/test_interview_pipeline.py::test_pipeline_review_resume_marks_malformed_seed_artifact_failed`. - `pytest tests/unit/auto/ -q` — 464 passed (5 new regression tests pin the per-iteration cap saturation/floor and complete_product round-trip). - `pytest tests/unit/auto/test_pipeline_ralph_handoff.py tests/unit/auto/test_interview_pipeline.py::test_pipeline_review_resume_marks_malformed_seed_artifact_failed -v` — 13 passed. - `ruff check src/ tests/` and `ruff format --check src/ tests/` — clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auto-mcp): surface Ralph handoff handles on result meta (Q00#791 review-4) Closes the bot's review-4 blocking finding on ``src/ouroboros/mcp/tools/auto_handler.py`` (`_result_meta` / `_format_result`). Before this commit the new Ralph handoff state was persisted internally on `AutoPipelineState` and exposed on `AutoPipelineResult`, but `_result_meta()` dropped `ralph_job_id` / `ralph_lineage_id` / `ralph_dispatch_mode` entirely. For MCP callers, that meant a run that delegated to plugin mode or stopped at a Ralph handoff exposed no structured tracking handle for the Ralph work — clients had to read local state files out-of-band to monitor or correlate the loop they had just dispatched. That was an API regression on the new MCP surface. This commit: - Surfaces all three fields on the MCP meta dict, each gated on presence so default-off `complete_product=False` runs keep the meta shape byte-identical to pre-Q00#773. - Renders a `Ralph handoff:` block in the human-readable text returned by `_format_result` so operators tailing the MCP response without a JSON parser still see the dispatch mode, job, and lineage handles. - Adds five regression tests in `tests/unit/mcp/tools/test_auto_handler_resume_lines.py` pinning the three contracts: handles-present, plugin-only (`job_id=None`, `dispatch_mode=plugin`), legacy-off byte-identity, and the matching text-block / no-block behavior of `_format_result`. ## Test plan - `pytest tests/unit/mcp/tools/test_auto_handler_resume_lines.py tests/unit/auto/ -q` — 495 passed (5 new MCP tests pin the meta and text contracts). - `ruff check src/ tests/` and `ruff format --check src/ tests/` — clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auto): reconcile RALPH_HANDOFF on resume + honor complete_product on RUN handle (Q00#791 review-5) Closes both blocking findings from ouroboros-agent[bot]'s review of commit abe59b5: 1. **Reconcile a persisted Ralph job on RALPH_HANDOFF resume** (`src/ouroboros/auto/pipeline.py`, `src/ouroboros/auto/adapters.py`, `src/ouroboros/cli/commands/auto.py`, `src/ouroboros/mcp/tools/auto_handler.py`). Before this commit, `_resume_ralph_handoff()` only emitted guidance text and returned a non-terminal `ralph_handoff` result. In long-lived runtimes — notably MCP, where the Ralph background job keeps running after the client disconnects — that meant any interrupted `--complete-product` session became stranded forever: `--resume` neither polled the persisted `ralph_job_id` nor mapped the finished Ralph result back onto `COMPLETE` / `BLOCKED` / `FAILED`. `AutoPipeline` now exposes a `ralph_resumer` callable. When wired, `_resume_ralph_handoff()` polls the persisted `ralph_job_id` (with a shared `JobManager` and `EventStore`) and runs the same `terminal_status` → auto-phase mapping as `_handoff_to_ralph()`, single-sourcing the COMPLETE / BLOCKED / FAILED contract pinned by `_RALPH_BLOCKED_STOP_REASONS`. CLI and MCP both construct the resumer alongside the starter from the same `RalphHandler` instance so both paths share the job table. When `ralph_resumer` is unset (in-process tests/library callers), resume falls back to the legacy guidance-only behavior so existing callers keep working. `HandlerRalphPoller` is the new adapter that bridges the auto pipeline to the runtime-owned Ralph job manager — it deliberately reuses `_wait_for_job_terminal` so dispatch and reconciliation converge on the same terminal-snapshot semantics. 2. **Honor durable `complete_product` on RUN resume with persisted handles** (`src/ouroboros/auto/pipeline.py`). The RUN-resume branch previously transitioned to `COMPLETE` immediately on any persisted `job_id` / `execution_id` / `run_session_id`, so a crash *after* run handoff but *before* `_handoff_to_ralph()` would silently bypass Ralph on resume even though the operator explicitly opted into RUN → RALPH_HANDOFF — regressing the persisted-session contract added in this PR. The branch now checks `self.complete_product and self.ralph_starter` before short-circuiting; when both are set it routes to `_handoff_to_ralph()` instead, so the durable session intent completes. - `pytest tests/unit/auto/ tests/unit/mcp/tools/ -q` — 1039 passed (5 new regression tests pin the polled-resume happy path, the `iteration_timeout → BLOCKED` mapping, the no-resumer fallback, and the RUN-resume `complete_product` on/off branches). - `ruff check src/ tests/` and `ruff format --check src/ tests/` — clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auto): checkpoint Ralph dispatch handle before terminal poll (Q00#791 review-6) Closes the bot's review-6 blocking finding on ``src/ouroboros/auto/adapters.py`` (`HandlerRalphStarter.__call__`). Before this commit, ``HandlerRalphStarter`` only returned after ``_wait_for_job_terminal`` finished, so ``_handoff_to_ralph()`` could not persist ``state.ralph_job_id`` until the entire Ralph loop was terminal. If the top-level deadline tripped, the client disconnected, or the process restarted *after* ``ouroboros_ralph`` had already created the background job but *before* that poll returned, the session was left in ``RALPH_HANDOFF`` with only ``ralph_lineage_id`` — and ``_resume_ralph_handoff`` could not call ``ralph_resumer`` (which keys off ``ralph_job_id``), so it fell back to guidance-only text and never reconciled the still-running Ralph job. That reintroduced the stranded-resume bug review-5 was meant to solve. This commit moves the durable handoff boundary to the *moment of dispatch*: - ``HandlerRalphStarter.__call__`` now accepts an optional ``on_dispatched`` callback and invokes it with the dispatch envelope (``job_id`` / ``lineage_id`` / ``dispatch_mode``) **before** blocking on the terminal-status poll. Plugin-mode dispatches fire the hook with the synthesized envelope as well so both modes have a single checkpoint contract. - ``AutoPipeline._handoff_to_ralph`` passes a checkpoint closure that writes ``state.ralph_job_id`` / ``state.ralph_dispatch_mode`` / ``state.ralph_lineage_id`` and saves immediately. The starter still returns the same terminal envelope shape, so downstream COMPLETE / BLOCKED / FAILED mapping is unchanged. - A ``TypeError`` fallback retries the starter without ``on_dispatched`` so older / third-party ``RalphStarter`` implementations that don't accept the kwarg keep working — they opt out of the early-checkpoint guarantee, but the legacy contract stays intact. After this checkpoint, a process death between dispatch and terminal leaves the persisted state with both ``ralph_job_id`` and ``ralph_dispatch_mode`` set, so ``_resume_ralph_handoff`` finds the job and ``ralph_resumer`` can poll it to a terminal auto phase. - `pytest tests/unit/auto/ tests/unit/mcp/ -q` — 1333 passed (2 new tests pin the early-checkpoint contract: the hook fires before the starter's terminal envelope returns, and a legacy starter without the hook still completes). - `ruff check src/ tests/` and `ruff format --check src/ tests/` — clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(auto): pin Ralph TypeError handoff retry regression Add focused coverage proving a Ralph starter that accepted on_dispatched and then raises TypeError is not invoked a second time, preventing duplicate Ralph loop dispatches. Also update the legacy-starter test wording for the signature-inspection path. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Hermes Agent <hermes-agent@users.noreply.github.com>
…ing RFC Force `strict_mcp_config=True` on the `ClaudeCodeAdapter` used by `InterviewEngine` so the spawned `claude` subprocess does not discover the project `.mcp.json` and recursively boot ouroboros-mcp inside the interview LLM call. `strict_mcp_config` already existed on `ClaudeCodeAdapter` with a docstring stating it was "Used exclusively by callers that must avoid recursion into the ouroboros MCP server (notably the interview policy path)" — but no caller was setting it. Without isolation, three-round interviews observed monotonic latency growth (11s → 38s → 102s+) as each call's `claude` subprocess re-booted ouroboros-mcp; with isolation the same flow runs at 22s / 20s / 33s with no accumulation. Implementation: - New `ClaudeCodeAdapter.with_strict_mcp_config()` returns a clone with the flag set; idempotent if already strict. - `InterviewEngine.__post_init__` calls `_isolate_adapter_for_question_generation()`, which duck-types `with_strict_mcp_config` so non-Claude adapters (anthropic, litellm) are untouched. Adds `docs/rfc/interview-hardening.md` covering this change (Change 3) plus the two follow-up PRs: - feat/interview-prompt-cap-raise (Change 1) - feat/interview-refine-restate (Change 2) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Raise two caps that previously silenced rich, structured answers from reaching the interview LLM: - `_MAX_TOTAL_PROMPT_CHARS`: 4,800 → 16,000 - `_MAX_USER_RESPONSE_CHARS`: 800 → 4,000 The 800-char per-answer cap was introduced as a workaround for an Agent SDK CLI empty-response quirk on oversized prompts, but it was applied uniformly across all adapters and across all interview rounds. As a result, multi-section answers (Decision / Reasoning / Constraints / Out of scope / Codebase context) were stored intact (security validator permits 10 KB) but truncated to the first ~800 chars when `_build_conversation_history` constructed the next-question prompt. The downstream effect: only the headline of each user answer reached the LLM. The dialectic collapsed into a label-confirmation loop because constraints and scope decisions were invisible to the question generator. Verified end-to-end: a 1,223-char structured answer is now preserved through `record_response` AND through the next-question prompt. The follow-up question references content from the answer's body (e.g. "monthly/annual recurring plans") rather than asking for a restatement of the headline decision. `_trim_messages_to_budget` continues to guard the total budget, so the total cap remains the safety net for runaway prompts. See `docs/rfc/interview-hardening.md` Change 1 (introduced in PR Q00#822). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Encode three changes to `skills/interview/SKILL.md` Path A so the main session sends rich, structured answers to the interview MCP and gates the seed transition with a one-line goal restatement. Step 3 — Multi-section payload by default. Single-line answers are now permitted only for PATH 1a auto-confirms and short PATH 2 answers (yes / no / single noun). All free-text answers are sent as a Decision / Reasoning / Constraints / Out of scope / Codebase context block. The interview MCP cannot read code, browse the web, or call tools — the text the main session sends is the only context driving the next question and ambiguity scoring, so compressing answers to a label degraded both. Step 4 — Refine before forwarding. Free-text answers go through a single AskUserQuestion confirming the structured payload preserves every reasoning, constraint, and scope element. The user can accept, add to a section, or rewrite. Auto- confirmed facts and option-pick confirmations skip this gate. Step 9 — Restate gate (post-Acceptance Guard). After the Seed-ready Acceptance Guard passes, the main session collapses the agreed answers into a one-sentence goal and confirms with the user before suggesting `ooo seed`. This is the one place where one-line compression is the goal — the rest of the interview kept everything in multi-section form. Path B (agent-mode fallback) inherits both gates by reference. Why "Refine" is reinterpreted, not copied: the standalone `refine` skill collapses meaning candidates into one shared-meaning line. Applying that literally inside the interview would re-introduce the compression problem this RFC is solving. The underlying principle of `refine` is "miss no nuance" — preserved by Step 4 via structure preservation rather than line consolidation. The one-line restatement happens once at Step 9, where compression is the explicit goal. See `docs/rfc/interview-hardening.md` Change 2 (introduced in Q00#822). Depends on Q00#823 (raised prompt caps): without raised caps, multi- section payloads would still be truncated. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
All three implementation PRs merged 2026-05-11: - Q00#822 adapter isolation (nested MCP boundary) - Q00#823 prompt budget caps - Q00#824 Refine/Restate gates in SKILL.md Records the scope adjustment to Change 3: strict_mcp_config=True is now applied at the nested MCP handler boundary rather than globally on InterviewEngine. CLI and PM interview entrypoints retain plugin/project .mcp.json behavior pending follow-up measurement. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… P2.1) (Q00#825) * feat(auto): EVALUATE phase verifies run output against seed AC (Q00#809 P2.1) Phase 2.1 of RFC Q00#809. Today `ooo auto --complete-product` chains RUN → RALPH_HANDOFF → COMPLETE without verifying that the actual run output satisfies the Seed's acceptance criteria — we trust Ralph's internal convergence. This change inserts a deterministic gate between Ralph's "converged" verdict and `ooo auto`'s COMPLETE: load the run artifact, ask the existing `ouroboros_qa` handler to grade it against the Seed AC, and only transition to COMPLETE on QA pass. On QA fail the pipeline transitions to BLOCKED with the QA differences and suggestions in the blocker text so a human (or Phase 2.2 lateral recovery) can act on it. Scope discipline: - EVALUATE only fires when `complete_product` AND `evaluator` are both wired; plain `ooo auto` (no `--complete-product`) is unchanged because the run is async and there is no synchronous artifact to grade. - Single-shot evaluation per session. No recovery loops, no SEED regeneration, no UNSTUCK_LATERAL persona — those land in Phase 2.2. What landed: - `AutoPhase.EVALUATE` enum value + `_ALLOWED_TRANSITIONS` `RALPH_HANDOFF → EVALUATE → {COMPLETE, BLOCKED, FAILED}` and recovery entry into EVALUATE from BLOCKED/FAILED. - New state fields persisted on `AutoPipelineState`: `last_qa_score`, `last_qa_verdict`, `last_qa_differences`, `last_qa_suggestions`, `evaluate_artifact_hash`. Backfilled with `setdefault(...)` for legacy state files; type-validated in `_validate_loaded`. - `EvaluateResult` frozen dataclass + `HandlerEvaluator` adapter in `auto/adapters.py` — builds quality_bar from `seed.acceptance_criteria` using the exact "The execution must satisfy all acceptance criteria" phrasing already used by `evolution_handlers.py`, calls `QAHandler.handle(...)` with `artifact_type=test_output`, `pass_threshold=0.80`, and seed YAML context; maps `meta["passed"]/score/verdict/differences/suggestions` onto a typed result. Transient infra errors return a result with `error` set instead of raising. - `AutoPipeline.evaluator` field + new `_evaluate_or_complete` and `_run_evaluate` methods. The evaluator runs at `_handoff_to_ralph` and `_poll_ralph_job` after Ralph reports `terminal_status=completed`, but ONLY when complete_product is true, evaluator is wired, and Ralph supplied a `result_text` to grade. Idempotent on resume via a sha256 of the artifact: matching hash + persisted verdict short- circuits without re-calling QA; a different artifact forces re-grade. - Timeout/handler error path: bounded by `AutoPhase.EVALUATE` timeout (default 90s) via `asyncio.wait_for`; on timeout or handler error the session is marked BLOCKED with `tool_name="evaluator"` so it remains resumable. - `AutoPipelineResult` gains `last_qa_score`, `last_qa_verdict`, `last_qa_differences`, `last_qa_suggestions` so MCP meta surfaces the verdict to callers. `_format_result` renders a "QA verdict" block with top-3 differences and suggestions. - MCP `auto_handler._run` instantiates `QAHandler` (only when `complete_product=True` to avoid wasted setup) and wires `HandlerEvaluator` into `AutoPipeline`. Tests (14 new, total auto+mcp regression 1308 passing): - State-machine: EVALUATE in allowed transitions, both directions - Pipeline happy path (pass → COMPLETE), fail path (revise → BLOCKED with diffs/suggestions in last_error), timeout → BLOCKED, handler error → BLOCKED - Opt-in guards: skipped when `complete_product=False`, skipped when evaluator is None, skipped when Ralph terminal meta lacks `result_text` - Resume idempotency: cached verdict on matching hash, re-evaluates on changed artifact - State persistence: round-trip via `to_dict`/`from_dict`, legacy state loads without QA fields - `HandlerEvaluator` adapter unit tests: quality_bar derivation from seed AC, QA `Result.err` path maps to `error`-bearing `EvaluateResult` Adjusts the pinned `RALPH_HANDOFF` transition test to accept the new EVALUATE bridge alongside the previous terminal destinations. Refs: Q00#809 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auto): plumb Ralph result_text + recoverable evaluator tool name Two BLOCKING findings from ouroboros-agent[bot] review of commit 0ae7c82: (1) Production Ralph adapters never surfaced result_text, so the EVALUATE phase never fired in real MCP/CLI runs. ``AutoPipeline`` only enters EVALUATE when ``ralph_result_text`` is non-empty; ``HandlerRalphStarter`` and ``HandlerRalphPoller`` only returned ``status``, ``stop_reason`` and related meta, never the human-readable artifact. The Phase 2.1 tests only passed because their stub starter wrote ``result_text`` directly. Fix: ``_wait_for_job_terminal`` now surfaces ``snapshot.result_text`` under a namespaced ``__result_text__`` key, and both adapters propagate it into the dict they return as ``result_text``. The pipeline already reads ``ralph_meta.get("result_text")``, so EVALUATE now fires for real Ralph runs in complete-product mode. (2) ``_recoverable_phase_for_tool`` had no mapping for ``tool_name="evaluator"``. On evaluator timeout or transient handler error the pipeline marks the session BLOCKED with that tool name, but ``--resume`` then classified the session as non-resumable (NONE capability) and returned the same BLOCKED state instead of re-entering EVALUATE. The cached verdict plumbing on ``AutoPipelineState`` was wasted because the session never re-entered the phase. Fix: map ``"evaluator"`` to ``AutoPhase.EVALUATE``. On resume the session re-enters EVALUATE, where the artifact-hash cache short- circuits to the prior verdict if available, or runs a fresh evaluator call if the artifact changed. Tests added (3, total 17 in test_pipeline_evaluate.py): - ``test_wait_for_job_terminal_surfaces_result_text`` — direct adapter test that the snapshot's ``result_text`` lands on the returned meta - ``test_handler_ralph_starter_returns_result_text`` — end-to-end starter call with a stub job manager verifies the ``result_text`` key reaches the dict the pipeline reads - ``test_recoverable_phase_for_evaluator_tool`` — pins the new mapping so future refactors don't silently re-introduce the non-resumable hole Refs: Q00#825 (review by ouroboros-agent[bot]), Q00#809 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auto): persist evaluate_artifact for real EVALUATE resume recovery Bot review #2 BLOCKING finding on commit 83c50b5: > Evaluator failures are not actually resumable. After Ralph completes, > _evaluate_or_complete() enters EVALUATE and passes ralph_result_text > only in-memory; on timeout/exception/transient QA error, the code > blocks with tool_name="evaluator" and advertises resume, but the > resume path later re-enters _run_evaluate(..., ralph_result_text=None) > and only has evaluate_artifact_hash/verdict persisted in state, not > the artifact itself. That drops straight into the "no cached verdict > and no run artifact" blocker, so --resume can never re-run the > evaluator after a transient failure. Real durability gap: the prior commit wired the recoverable-phase mapping (#1) but left the resume entry without the data it needs. The session would advertise "resumable" yet immediately re-block. Fix: persist the artifact text verbatim on AutoPipelineState as ``evaluate_artifact`` before invoking the evaluator. The hash is already stored; the artifact joins it so on resume: 1. ``_run_evaluate`` is entered with ``ralph_result_text=None`` 2. It reads ``state.evaluate_artifact`` (now non-empty) 3. The recomputed hash matches the persisted one — cache hit returns the cached verdict, OR cache miss re-invokes the evaluator with the same artifact No truncation on persistence so the recomputed hash matches the stored one. Bound state file growth at a higher layer if it ever becomes a problem (Ralph result_text is typically small relative to ledger/seed_artifact already on state). State changes: - new field ``evaluate_artifact: str | None = None`` - ``setdefault("evaluate_artifact", None)`` in ``from_dict`` for backward compat - type-validated in ``_validate_loaded`` Pipeline changes: - ``_run_evaluate`` resolves the artifact from three sources in order: 1. ``ralph_result_text`` (fresh call from Ralph terminal) 2. ``state.evaluate_artifact`` (resume after prior call) 3. neither → BLOCKED with the existing "no cached verdict and no artifact" message (still a recoverable corner case for a session interrupted before the first save) - persists artifact + hash BEFORE the evaluator call so timeout / exception / transient error paths all leave a recoverable trail Tests added (3, total 20 in test_pipeline_evaluate.py): - ``test_evaluator_timeout_persists_artifact_for_resume`` — first call times out, state.evaluate_artifact holds the Ralph payload - ``test_resume_after_evaluator_timeout_re_runs_with_persisted_artifact`` — full resume flow: first call times out, simulated resume into EVALUATE re-invokes the evaluator with the persisted artifact and reaches COMPLETE - ``test_state_round_trips_evaluate_artifact`` — state to_dict/from_dict preserves the new field Refs: Q00#825 (review by ouroboros-agent[bot]), Q00#809 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auto): EVALUATE cache correctness + empty artifact + resume capability Three findings from ouroboros-agent[bot] review #3 on commit f35f280. (1) BLOCKING — Stale verdict on new artifact Scenario: artifact A graded pass → verdict cached. Artifact B enters EVALUATE. The prior fix persisted hash(B) BEFORE the evaluator call so a timeout would leave a recoverable trail. But last_qa_verdict was still the pass from A. On resume the cache check saw ``hash(B) == hash(B) AND last_qa_verdict is not None`` and took the cache-hit path, declaring B passed without ever grading it. Fix: clear last_qa_score/verdict/differences/suggestions whenever the new artifact hash differs from the previously persisted one. The cache is then guaranteed correct: a verdict only sticks for the artifact it was produced against. (2) BLOCKING — Empty Ralph artifact silently skipped ``_evaluate_or_complete`` checked ``if ralph_result_text:`` which treats ``""`` as falsy. A run whose output is intentionally empty (or whose stdout was filtered out) therefore skipped EVALUATE entirely and the session transitioned straight to COMPLETE — a false-pass that violates the "complete_product + evaluator → graded COMPLETE" contract. The Ralph adapter result also flowed through ``_optional_str`` which collapses ``""`` to None — another layer of the same bug. Two changes: - ``_evaluate_or_complete`` now checks ``ralph_result_text is not None`` - a new helper ``_artifact_text`` preserves ``""`` (only returns None for non-string inputs); both ralph terminal call sites use it instead of ``_optional_str`` for the artifact field (3) WARNING — ``resume_capability()`` ignored EVALUATE After an evaluator timeout/transient error the session is BLOCKED with ``tool_name="evaluator"``. ``_recoverable_phase_for_tool("evaluator")`` correctly returns ``AutoPhase.EVALUATE``, but ``resume_capability()`` had no branch for that phase and fell through to NONE. MCP/CLI surfaces therefore reported the session as non-resumable even though the resume machinery did re-enter EVALUATE — a public-contract mismatch. Fix: add an EVALUATE branch: - ``state.evaluate_artifact`` present → RESUME (evaluator can re-grade) - ``evaluate_artifact_hash + last_qa_verdict`` both present → RESUME (cache-hit short-circuit will drive the resume to its terminal state) - neither → NONE Tests added (5, total 25 in test_pipeline_evaluate.py): - ``test_new_artifact_does_not_inherit_stale_verdict`` — A passes, B times out, verdict cleared, hash bumped - ``test_empty_ralph_artifact_still_enters_evaluate`` — empty artifact reaches the evaluator and produces an explicit verdict - ``test_resume_capability_advertises_evaluate_as_resumable`` — EVALUATE blocked + artifact persisted → RESUME - ``test_resume_capability_evaluate_blocked_without_artifact_is_none`` — EVALUATE blocked + nothing persisted → NONE - ``test_resume_capability_evaluate_blocked_with_cached_verdict_is_resumable`` — cache-hit-only case still resumable Refs: Q00#825 (review by ouroboros-agent[bot]), Q00#809 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auto): empty-artifact contract leaks through Ralph adapters + state Two BLOCKING findings from ouroboros-agent[bot] review #4 on b5e1855. Both are residues of the same empty-string-is-valid-artifact contract: adjacent code paths still collapse ``""`` to None, leaking the false-pass through other entry points. (1) Ralph adapters re-collapsed ``__result_text__`` via ``_optional_str`` ``HandlerRalphStarter`` and ``HandlerRalphPoller`` both routed the artifact through ``_optional_str(terminal_meta.get("__result_text__"))``, which returns None for ``""``. Production Ralph runs with intentionally empty output therefore reached ``_evaluate_or_complete`` with ``ralph_result_text=None`` and silently skipped EVALUATE — exactly the false-pass the previous commit claimed to fix on the pipeline side. Fix: add ``_artifact_text`` helper in ``auto/adapters.py`` mirroring the one already in pipeline.py — preserves ``""`` verbatim, only returns None for non-string inputs. Both adapters use it for ``result_text``. Other fields (``lineage_id``, ``status``, etc.) keep ``_optional_str`` because empty strings there really do mean "missing". (2) ``resume_capability()`` used truthy ``self.evaluate_artifact`` A session blocked after persisting an empty-but-valid artifact would report ``NONE`` capability — CLI/MCP surfaces would omit the resume hint even though ``_run_evaluate`` can fully re-grade ``""`` on resume. Fix: ``is not None`` rather than truthiness. Tests added (3, total 28 in test_pipeline_evaluate.py): - ``test_resume_capability_evaluate_blocked_with_empty_artifact_is_resumable`` — state with ``evaluate_artifact=""`` reports RESUME (not NONE) - ``test_handler_ralph_starter_preserves_empty_result_text`` — production starter adapter propagates ``""`` as a real string into the pipeline - ``test_handler_ralph_poller_preserves_empty_result_text`` — same contract on the resume poller path Refs: Q00#825 (review by ouroboros-agent[bot]), Q00#809 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auto): EVALUATE respects top-level pipeline deadline BLOCKING finding from ouroboros-agent[bot] review #5 on 6ba4a85: > The new EVALUATE phase ignores the top-level pipeline deadline and > always waits for the full per-phase timeout. Every other long-running > phase now uses _deadline_capped_timeout(...), but _run_evaluate() > calls asyncio.wait_for(..., timeout=state.phase_timeout_seconds(...)) > directly. If Ralph finishes with only a few seconds left before > deadline_at, the QA call can still block for up to 90s, violating > the pipeline timeout contract and returning an "evaluator timed out" > blocker instead of the expected pipeline_timeout. Fix: mirror the pattern used in INTERVIEW, SEED_GENERATION, REVIEW, REPAIR, RUN, and RALPH_HANDOFF — wrap the phase timeout in ``self._deadline_capped_timeout(state, phase_timeout)`` before calling ``asyncio.wait_for``, and on TimeoutError consult ``self._enforce_deadline(state)`` so a real deadline trip surfaces the canonical ``pipeline_timeout`` blocker (``tool_name="pipeline_deadline"``) instead of the per-phase "evaluator timed out" message. Test added (1, total 29): - ``test_evaluator_respects_top_level_pipeline_deadline`` — sets ``state.deadline_at = now + 0.1s``, phase timeout 90s, hanging evaluator. Asserts the deadline trip uses ``PIPELINE_DEADLINE_TOOL_NAME`` and ``pipeline_timeout`` rather than ``"evaluator"`` / ``"evaluator timed out"``. Refs: Q00#825 (review by ouroboros-agent[bot]), Q00#809, Q00#779 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auto): cache pass flag separately from verdict string BLOCKING finding from ouroboros-agent[bot] review #6 on 9e6fbb7: > The EVALUATE resume cache reconstructs `passed` from > `last_qa_verdict == "pass"` instead of from the QA contract's > persisted pass condition. QAHandler records `passed` as > `score >= pass_threshold`, so a run that originally completed with > `passed=True` but `verdict="revise"`/`"fail"` will resume down the > cache-hit path and be reclassified as blocked. Real determinism break: the QA contract treats ``passed`` as ``score >= pass_threshold`` — independent of the LLM's free-form ``verdict`` text. A score of 0.85 against a 0.80 threshold can yield ``passed=True, verdict="revise"`` when the LLM is conservative on the categorical label. The prior cache key (``verdict == "pass"``) would have flipped that resume to BLOCKED. Fix: persist ``last_qa_passed: bool | None`` on AutoPipelineState as the canonical pass flag. Set it from ``eval_result.passed`` on every evaluator call. The cache hit predicate now keys on ``last_qa_passed is not None`` (the boolean is present iff a verdict was committed for this artifact hash) and returns ``passed = bool(state.last_qa_passed)`` verbatim. ``last_qa_verdict`` remains for human display only. State changes: - new field ``last_qa_passed: bool | None = None`` - ``setdefault("last_qa_passed", None)`` in from_dict for legacy state - type-validated in _validate_loaded - ``resume_capability()`` EVALUATE branch keys on ``last_qa_passed`` rather than ``last_qa_verdict`` Pipeline changes: - cache-hit predicate uses ``state.last_qa_passed is not None`` - pass flag is persisted alongside verdict and score - artifact-changed cleanup clears ``last_qa_passed`` too Test added (1, total 30): - ``test_evaluate_cache_reuses_passed_flag_for_revise_verdict``: evaluator returns ``passed=True, verdict="revise"``. First call → COMPLETE. Simulated resume → cache hit returns COMPLETE (not BLOCKED), evaluator not re-invoked. Locks in the resume-determinism contract. Refs: Q00#825 (review by ouroboros-agent[bot]), Q00#809 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(qa): allow empty artifact strings * fix(auto): allow evaluate resume through run gate * fix(auto): keep evaluate qa synchronous in plugin mode --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Hermes Agent <hermes-agent@users.noreply.github.com>
…P2.2) (Q00#829) Add Phase 2.2 advisory layer: when EVALUATE deterministically fails, dispatch a persona via lateral_routing to surface human-readable advice in the AC ledger as [from-auto][lateral_repair]. This stays advisory — no automated recovery loop yet (that lands in P2.2b). Includes the full set of follow-up fixes from bot review iterations: - Resume allowlist covers EVALUATE/UNSTUCK_LATERAL (idempotent handlers) - Plugin-mode delegation: skip evaluator/lateral wiring when opencode_mode is "plugin", falling back to the pre-Phase-2.1 RALPH_HANDOFF path - Empty-artifact contract enforced through Ralph adapters and state - Symmetric evaluator-not-wired guard on EVALUATE resume - Lateral cache + resume capability covering the full QA-fail spectrum Rebased onto main after Q00#825 (P2.1) squash-merged. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…P3, PR 1/6) Pillar A of Q00#809 moves domain-specific knowledge (vague terms, verifiable predicates, intent classifiers, safe defaults) out of the hard-coded coding assumptions in answerer.py and safe_defaults.py and into a per-domain DomainProfile. This PR is purely observational: it defines the contracts only — no existing caller is modified. Public surface introduced: - VerifiablePredicate (Protocol) — code, matches(), repair_template() - IntentClassifier (Protocol) — classify(), supported_intents() - RepoContextExtractor (Protocol) — extract(cwd) - DomainProfile (frozen dataclass) — name, predicates, classifier, extractor, vague_terms, safe_defaults, detector, find_verifiable_predicate() - DomainProfileRegistry — register(), get(), all(), detect_best(), union_predicates() - DEFAULT_REGISTRY — module-level singleton (empty until PR-2) Follow-up stack: - PR-2: coding DomainProfile registered into DEFAULT_REGISTRY - PR-3: CLI activation routes through DomainProfileRegistry - PR-4: AutoAnswerer routed through intent_classifier - PR-5: safe_defaults migrated to DomainProfile.safe_defaults - PR-6: second built-in profile demonstrating plurality Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… 5/6) Introduces `active_profile: DomainProfile | None = None` parameter to `finalize_safe_defaultable_gaps`. When a profile is supplied its `safe_defaults` dict is consulted first for each ledger section; missing keys fall through to the hardcoded `_SAFE_DEFAULTS` coding-domain dict so the pre-existing behavior is fully preserved when `active_profile=None`. Profile values may be `_DefaultSpec`, plain `dict`, or plain `str` — all three forms are coerced into `_DefaultSpec` via the new `_resolve_spec` helper. The helper is also used by a forthcoming `build_safe_default_synthesis` extension path. `AutoPipelineState` gains an `active_domain_profile_name: str | None` field (defaults to `None` for backwards-compatible state file loading). `AutoInterviewDriver.run` resolves the name against `DEFAULT_REGISTRY` and threads the resulting profile into `finalize_safe_defaultable_gaps`. Stacks on PR Q00#849 (PR-4 intent classifier routing). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one. |
|
Re-opening against upstream Q00/ouroboros — wrong target. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4eb0d35730
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "if a stricter answer is required.", | ||
| ] | ||
| for section in finalization.defaulted_sections: | ||
| spec = _SAFE_DEFAULTS.get(section) |
There was a problem hiding this comment.
Use resolved profile defaults when building synthesis text
When finalize_safe_defaultable_gaps runs with an active domain profile, ledger entries are written from _resolve_spec(...), but build_safe_default_synthesis always reads from _SAFE_DEFAULTS. In max-round finalization flows this makes the persisted interview transcript describe coding defaults even when the ledger used profile overrides, so downstream seed generation (which consumes transcript answers) can ignore the selected profile and produce inconsistent output.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Code Review
This pull request introduces several significant enhancements to the ooo auto pipeline, including the implementation of a top-level pipeline deadline, support for chaining RUN to RALPH_HANDOFF via a new complete_product flag, and a new EVALUATE phase with persona-driven lateral thinking for verification failures. It also adds robust idempotency handling for run handoffs and updates the plugin layer with a new trust-subject contract and artifact digest verification. My feedback focuses on refining the resume_capability logic in state.py to accurately reflect the pipeline's retry capabilities for run handoffs, ensuring users receive correct guidance on whether a session can be retried or is terminally blocked.
| if self.run_start_attempted: | ||
| return AutoResumeCapability.NONE |
There was a problem hiding this comment.
The current logic for determining resume_capability for the RUN phase is too simplistic and can provide incorrect guidance to the user. It currently returns NONE if run_start_attempted is true, but the pipeline logic in pipeline.py actually allows for one retry if the handoff status is unknown_no_handle or unknown_timeout.
This logic should be updated to correctly identify when a retry is possible, so that the user is correctly informed that they can RETRY the operation.
| if self.run_start_attempted: | |
| return AutoResumeCapability.NONE | |
| if self.run_start_attempted: | |
| if self.run_handoff_status in {"unknown_no_handle", "unknown_timeout"}: | |
| # The "retried once" phrase is from AutoPipeline and signals that the | |
| # single retry attempt has been exhausted. | |
| if self.run_handoff_guidance and "retried once with idempotency key" in self.run_handoff_guidance: | |
| return AutoResumeCapability.NONE | |
| return AutoResumeCapability.RETRY | |
| return AutoResumeCapability.NONE |
Summary
active_profile: DomainProfile | None = Noneparameter tofinalize_safe_defaultable_gaps; profile'ssafe_defaultsis consulted first, missing keys fall through to the hardcoded_SAFE_DEFAULTScoding-domain dict.active_domain_profile_name: str | Nonefield toAutoPipelineState(defaultsNonefor backwards-compatible state file loading).AutoInterviewDriver.runresolves the name againstDEFAULT_REGISTRYand threads the profile intofinalize_safe_defaultable_gaps.Test plan
test_profile_default_overrides_hardcoded— profile value wins over hardcodedtest_missing_key_falls_back_to_hardcoded— missing profile key uses_SAFE_DEFAULTStest_no_profile_uses_hardcoded_dict_unchanged—active_profile=Nonepreserves existing behavior exactlytest_profile_dict_value_is_coerced_to_defaultspec— plain dict coercedtest_profile_str_value_is_coerced_to_defaultspec— plain string coercedtest_active_profile_threading_via_state—AutoPipelineState.active_domain_profile_namepersists and round-tripsAll 150 existing + new tests pass. Ruff clean.
Stacks on PR-4 (
feat/p3-intent-classifier-routing).🤖 Generated with Claude Code