Skip to content

feat(core): introduce Directive StrEnum for control-plane decisions#477

Open
shaun0927 wants to merge 2 commits intoQ00:mainfrom
shaun0927:feat/core-directive-vocabulary
Open

feat(core): introduce Directive StrEnum for control-plane decisions#477
shaun0927 wants to merge 2 commits intoQ00:mainfrom
shaun0927:feat/core-directive-vocabulary

Conversation

@shaun0927
Copy link
Copy Markdown
Collaborator

Summary

  • Introduce a shared Directive StrEnum for control-plane decisions (continue, evaluate, evolve, unstuck, retry, compact, wait, cancel, converge).
  • Pure addition — no decision site is modified in this PR. Subsequent migrations will land as separate, independently reviewable PRs.

Motivation

First step of #472. Post-#280, docs/guides/mcp-bridge.md records two Known Limitations: evolve_step not receiving the bridge manager, and no dynamic upstream-server addition. #471 frames these as control-plane symptoms — decision sites across evaluation/, evolution/, resilience/, orchestrator/, and observability/ each emit ad-hoc signals (enums, booleans, literal strings) with no shared vocabulary, which is why new plumbing is required on every handler added post-#280.

#472 proposes introducing that shared vocabulary. This PR lands only the type and its invariants, with no caller modifications, matching the surgical-PR pattern established by recent merges (e.g., #470, #438, #444). Downstream migrations of individual decision sites become their own review units.

Changes

  • src/ouroboros/core/directive.py — new Directive StrEnum. Each member has a short docstring stating precondition and effect. Terminality is exposed through is_terminal, backed by a module-level _TERMINAL_DIRECTIVES frozenset. Exactly two terminal members: CANCEL, CONVERGE.
  • src/ouroboros/core/__init__.py — lazy re-export of Directive alongside Result and the core type aliases, preserving the existing lazy-loader pattern used for every other core symbol.
  • tests/unit/core/test_directive.py — 10 unit tests covering:
    • string-value stability (lowercased names, uniqueness, StrEnum substitutability)
    • terminal-set closure (exactly CANCEL and CONVERGE; everything else is non-terminal)
    • required vocabulary membership (guards accidental rename/removal)
    • Directive(value) round-trip
    • lazy re-export through ouroboros.core

Not changed

No existing decision site is migrated in this PR. Migration is deferred so each call site becomes its own small, reviewable change:

  • evaluation/ terminal branches — follow-up PR
  • evolution/ convergence / resilience/ stagnation handlers — follow-up PRs
  • orchestrator/ routing — follow-up PR

Verification

uv run ruff check src/ouroboros/core/directive.py src/ouroboros/core/__init__.py tests/unit/core/test_directive.py
uv run ruff format --check src/ouroboros/core/directive.py src/ouroboros/core/__init__.py tests/unit/core/test_directive.py
uv run pytest tests/unit/core/ -q

Result: 296 passed (10 new), ruff clean, format clean.

References

Add ouroboros.core.directive as an additive, caller-free vocabulary for
control-plane decisions (continue / evaluate / evolve / unstuck / retry /
compact / wait / cancel / converge).

Decision sites are currently distributed across evaluation/, evolution/,
resilience/, orchestrator/, and observability/, each with its own ad-hoc
signals. This commit introduces only the shared type and its terminal
set; no decision site is modified. Follow-up changes will migrate sites
incrementally so each migration is independently reviewable.

- src/ouroboros/core/directive.py: Directive StrEnum, per-member docstrings,
  and a module-level _TERMINAL_DIRECTIVES frozenset exposed through
  is_terminal (CANCEL, CONVERGE).
- src/ouroboros/core/__init__.py: lazy re-export of Directive.
- tests/unit/core/test_directive.py: value/terminality/membership/re-export
  coverage (10 tests).

Verification:
- uv run ruff check src/ouroboros/core/directive.py src/ouroboros/core/__init__.py tests/unit/core/test_directive.py
- uv run ruff format --check <same paths>
- uv run pytest tests/unit/core/ -q  # 296 passed
Copy link
Copy Markdown
Contributor

@ouroboros-agent ouroboros-agent Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review — ouroboros-agent[bot]

Verdict: APPROVE

Reviewing commit f3a9ab0 for PR #477

Review record: 959e4035-4e28-4cfa-b010-8b6c0197f02f

Blocking Findings

No in-scope blocking findings remained after policy filtering.

Non-blocking Suggestions

None.

Design Notes

The change is narrowly scoped and internally consistent: it adds a small Directive vocabulary module, exposes it through the existing lazy-export boundary, and covers the public API/terminality contract with focused unit tests. I did not find a diff-scoped runtime or API issue. The local environment does not have pytest installed, so full test execution was not possible here; direct import validation of the new module/export path succeeded.

Recovery Notes

First recoverable review artifact generated from codex analysis log.


Reviewed by ouroboros-agent[bot] via Codex deep analysis

@shaun0927
Copy link
Copy Markdown
Collaborator Author

Aligning with #476 (Phase 2 framing): this PR is in scope and unchanged. The Directive type as defined here matches the vocabulary enumerated in that RFC. Placement in core/ is the intended Phase 2 home per my answer at #476#issuecomment-4294892689 (Q2) — relocatable via a one-line lazy re-export change if a dedicated runtime/ module is introduced later.

No code change needed. ouroboros-agent APPROVED, all CI green.

Expand the module docstring to connect Directive explicitly to the Agent
OS layering introduced by the Phase 2 RFC (Q00#476): directives occupy the
control layer and are the runtime-level syscalls through which decision
sites express control flow.

Also record two posture decisions that were implicit before:

- Directives describe workflow control, not capability policy. Capability
  visibility/execution decisions stay in the policy layer and in
  policy.* events, never interleaved with directive semantics.
- Existing local enums (e.g. StepAction) are mapped into this vocabulary
  incrementally rather than replaced in a single change. The docstring
  records the reference StepAction -> Directive mapping so callers and
  reviewers can see the migration path at the type's site of truth.

No behavior or API change.
@shaun0927
Copy link
Copy Markdown
Collaborator Author

Correction to my earlier alignment note — ouroboros-agent APPROVED covers style and diff-scoped correctness, not conceptual alignment with #476. Re-reading the module docstring against the Phase 2 RFC surfaced two gaps:

  1. Framing: Directive was described as "control-plane decisions" but not positioned inside the Agent OS layer stack. The RFC treats directives as runtime-level syscalls in the Control Layer; the docstring now says so, next to the other four layer names so future readers see where it sits.
  2. Migration posture: the RFC explicitly calls for mapping existing enums (StepAction example) rather than replacing them. That stance was implicit in the PR body but missing from the module itself; the docstring now records the reference StepAction → Directive mapping at the type's site of truth.

New commit: 1606c6e docs(core): tie Directive module to Phase 2 control-plane framing — docstring only, no behavior change, existing tests pass (10/10).

Copy link
Copy Markdown
Contributor

@ouroboros-agent ouroboros-agent Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review — ouroboros-agent[bot]

Verdict: APPROVE

Reviewing commit 1606c6e for PR #477

Review record: 3f8074cc-a3d7-4d48-a8e2-03002eb8e82e

Blocking Findings

No in-scope blocking findings remained after policy filtering.

Non-blocking Suggestions

| 1 | src/ouroboros/core/directive.py:13 | Documentation | The module docstring’s Directive layer bullet has malformed emphasis markup (**...* ...)*), which makes the rendered layering explanation harder to read even though it does not affect runtime behavior. |
| 2 | tests/unit/core/test_directive.py:53 | Nice-to-have test | test_required_members_present only checks that the expected names are a subset of the enum. If the intent is to lock the initial vocabulary exactly, an equality assertion would catch accidental additions as well as removals/renames. |

Design Notes

The change is structurally sound: Directive is introduced as an isolated StrEnum, re-exported through the existing lazy core boundary, and covered by focused unit tests for value stability, terminality, and package export behavior. I did not find a diff-scoped runtime or API-contract regression.

Recovery Notes

First recoverable review artifact generated from codex analysis log.


Reviewed by ouroboros-agent[bot] via Codex deep analysis

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant