[codex] Add AI workflow context to PM interview prompt#1201
Conversation
There was a problem hiding this comment.
Review — ouroboros-agent[bot]
Verdict: APPROVE
Reviewing commit
2f9475bfor PR #1201
Review record:
5cd7c3c6-92f8-4e14-8f28-3985589956bc
Blocking Findings
No in-scope blocking findings remained after policy filtering.
Non-blocking Suggestions
None.
Design Notes
Unable to assess architecture or implementation scope because the source snapshot and diff were inaccessible in this environment.
Policy Notes
- Omitted 1 finding(s) that referenced files outside the current PR changed-files scope.
Recovery Notes
First recoverable review artifact generated from codex analysis log.
Reviewed by ouroboros-agent[bot] via Codex deep analysis
|
Warden note: this PR is not AgentOS/#961 tier-gated and current checks are green. It does touch the same PM interview prompt area as the active #1153/#1154 PM guidance lane, so please make sure it is reconciled with that in-flight guidance/pause-resume discussion before merge. No roadmap gate label or review action is needed from me. Posted by agentos-roadmap-warden — bot. Reply with |
There was a problem hiding this comment.
Review — ouroboros-agent[bot]
Verdict: REQUEST_CHANGES
PR #1201
Branch: codex/pm-ai-workflow-steering | 1 files, +1/-0 | CI: Bridge TypeScript pass 11s https://github.com/Q00/ouroboros/actions/runs/26332490936/job/77520743667
Scope: architecture-level
HEAD checked: 2f9475b2c2a143b88ef695a82a972c2756192512
What Improved
- Added explicit PM-interview steering that frames the PRD as input to downstream AI planning, implementation, and verification workflows.
- Added broad runtime/plugin/config documentation and substantial tests around new runtime, PM, plugin, persistence, and workflow surfaces.
Issue #N/A Requirements
| Requirement | Status |
|---|---|
| Add PM interview steering for downstream AI workflow use | Implemented in src/ouroboros/bigbang/pm_interview.py. |
| Keep claimed scope narrow | Not met: PR metadata says 1 changed file and 1 addition, but current diff spans hundreds of files and multiple runtime/plugin/persistence surfaces. |
| Preserve PM MCP session/replay contracts | Blocked by the select_repos session-id continuity issue above. |
| Meaningful tests for newly added logic | Partially met: many targeted tests exist and the sampled suites pass, but the broken in-process two-step select_repos session continuity needs direct coverage. |
Prior Findings Status
| Prior Finding | Status |
|---|---|
| Prior review context | MODIFIED — No prior review thread was provided in the artifacts I inspected, so no prior concerns can be confirmed as maintained, modified, or withdrawn. |
Blockers
| # | File:Line | Severity | Confidence | Finding |
|---|---|---|---|---|
| 1 | src/ouroboros/mcp/tools/pm_handler.py:1014 | High | 85% | The in-process select_repos continuation discards the session_id it required from step 1 and calls _handle_start(...), which creates a fresh interview via ask_opening_and_start without passing the existing ID. That breaks the two-step PM contract described in this same method: callers that selected repos for an existing pre-start session get a new session instead, leaving the original session/meta as the handle they were told to continue with and making resume/idempotency ambiguous across the PM MCP boundary. |
Follow-ups
| # | File:Line | Priority | Confidence | Suggestion |
|---|---|---|---|---|
| — | — | — | — | None. |
Test Coverage
- Verified:
uv run pytest tests/unit/bigbang/test_pm_interview.py -qpassed, 59 tests. - Verified:
uv run pytest tests/unit/orchestrator/test_runtime_factory.py tests/unit/test_kiro_adapters.py tests/unit/orchestrator/test_copilot_cli_runtime.py -qpassed, 123 tests. - Verified:
uv run pytest tests/unit/config/test_loader.py tests/unit/config/test_loader_env.py tests/unit/config/test_models.py tests/unit/orchestrator/test_workflow_state.py tests/unit/persistence/test_event_store.py -qpassed, 317 tests. - Verified:
uv run python -m compileall -q src testspassed.
Design / Roadmap Gate
Affected-boundary review: this PR is not just a prompt tweak. It changes PM interview flow, MCP handler contracts, plugin/runtime configuration, package extras, workflow release behavior, and persistence/resume metadata. The main design blocker is at the PM MCP boundary: a multi-turn state machine action that requires an existing session_id must continue that same durable session. Current HEAD can silently transition to a newly created session during select_repos, so replay/idempotency and consumer handles are not stable.
Merge Recommendation
Do not merge until the in-process PM select_repos path preserves the original session identity or explicitly returns a safe migration/alias contract, with regression coverage for start → select_repos → resume/generate using the same session handle.
Review-Metadata:
verdict: REQUEST_CHANGES
github_event: COMMENT
review_kind: full
merge_eligible: false
head_sha: 2f9475b
source_read_ok: true
diff_read_ok: true
blocking_count: 0
There was a problem hiding this comment.
Review — ouroboros-agent[bot]
Verdict: APPROVE
Metadata
| Field | Value |
|### Recovery Notes
First recoverable review artifact generated from codex analysis log.
---|---|
| PR | #1201 |
| HEAD checked | 2f9475b2c2a143b88ef695a82a972c2756192512 |
| Request ID | req_manual_requeue_container_token_1779694376_1201 |
| Review record | 5e822705-0be3-4c72-a6c3-04fe4db0954f |
What Improved
- Strengthens the PM interview system prompt so generated product requirements are precise enough for downstream AI planning, implementation, and verification.
Issue Requirements
| Requirement | Status |
|---|---|
| No linked issue requirement captured | N/A |
Prior Findings Status
No inline review comments were present. The only prior context was a warden note to reconcile this PM prompt-area change with active PM guidance work; kept as a non-blocking follow-up because no current-head contract failure was evident.
Blockers
No in-scope blocking findings remained after policy filtering.
Follow-up Findings
| # | File:Line | Priority | Confidence | Suggestion |
|---|---|---|---|---|
| 1 | src/ouroboros/bigbang/pm_interview.py:61 | Low | Medium | Maintainer note flags nearby in-flight PM guidance work (#1153/#1154); reconcile wording before merge if those PRs establish a more specific prompt contract. |
Non-blocking Suggestions
| 1 | None | None | None. |
Test Coverage Notes
- Reviewed changed file, unified diff, PR comments, review comments, PM interview tests, PM handler boundary, PM skill guidance, and PM document/extraction path.
- Could not execute focused tests:
uvis not installed;pythonis unavailable;python3 -m pytestfailed because pytest is not installed.
Design Notes
This is a narrow prompt-only change in the PM interview steering prefix. It remains in-memory and does not leak into persisted interview initial context, matching the existing design.
Design / Roadmap Gate
Affected boundary is PM question generation only: _PM_SYSTEM_PROMPT_PREFIX is installed by _install_pm_steering() over the base InterviewEngine prompt and is intentionally not persisted into InterviewState.initial_context. Extraction, PM document generation, metadata persistence, and MCP handler contracts are unchanged. Prompt length impact is negligible relative to existing prompt-budget tests and limits.
Directional Notes
Memory and project docs emphasize specification-first clarity and avoiding prompt drift. I focused on whether this sentence creates a second implementation path, persistence leak, or runtime contract change; I found none.
Test Coverage
- Reviewed changed file, unified diff, PR comments, review comments, PM interview tests, PM handler boundary, PM skill guidance, and PM document/extraction path.
- Could not execute focused tests:
uvis not installed;pythonis unavailable;python3 -m pytestfailed because pytest is not installed.
Merge Recommendation
APPROVE. No blocking runtime, persistence, API, or design-contract issue was found in the current snapshot. Test execution was unavailable in this container, but the change is a single prompt sentence and existing focused tests cover the relevant wrapper/persistence boundary.
Review-Metadata:
verdict: APPROVE
head_sha: 2f9475b
request_id: req_manual_requeue_container_token_1779694376_1201
review_profile: memory-aware-zero-trust-v2
advisory_memory_only: true
Reviewed by ouroboros-agent[bot] via Codex deep analysis
Summary
Adds one sentence to the PM interview steering prompt so the interviewer assumes the generated product requirements document will drive downstream AI workflows.
Impact
This nudges PM interviews to elicit decisions precise enough for autonomous planning, implementation, and verification, while keeping the existing PM-focused prompt structure intact.
Validation
uv run pytest tests/unit/bigbang/test_pm_interview.py