stack(pr138): integrate D7 implementation on top of evidence-first gate - #146
stack(pr138): integrate D7 implementation on top of evidence-first gate#146mindfn wants to merge 10 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9583b868f
ℹ️ 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".
| self._state.plan_description = str(plan_description) | ||
|
|
||
| if steps is not None: | ||
| old_by_id = {step.step_id: _step_state(step) for step in self._state.steps} |
There was a problem hiding this comment.
ReviseCurrentPlanTool.execute now builds old_by_id via step.step_id, which raises AttributeError when PlannerState.steps contains legacy dict entries (a shape this commit still treats as compatible in PlannerState helpers/tests). In that migrated-state scenario, revise_current_plan crashes instead of returning a deterministic ToolResult, so users cannot revise active plans after loading older snapshots.
Useful? React with 👍 / 👎.
| if not self._state.steps: | ||
| return ToolResult(success=False, output=None, error="no active plan to finish") | ||
|
|
||
| pending = [step.step_id for step in _pending_steps(self._state)] |
There was a problem hiding this comment.
FinishPlanTool.execute computes pending with step.step_id from _pending_steps(self._state), so any legacy dict-backed step causes an immediate AttributeError before tool-level validation runs. This breaks finish_plan for migrated sessions and bypasses the expected deterministic failure contract (caller gets an exception, not a structured tool error).
Useful? React with 👍 / 👎.
b9583b8 to
aa192d0
Compare
Sync D7 acceptance-pack language to latest #142 contract updates. Changes: - Contract Delta now uses error semantics wording instead of error_code-only wording. - Observability locator wording now requires fixed IDs plus one-of error locator fields. Rationale: keep #146 evidence contract consistent with DARE's mixed runtime error representation model (ToolResult.error + ToolError code + hook error_code/error_type).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa192d033e
ℹ️ 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".
|
|
||
| def _step_state(step: Step) -> PlanStateName: | ||
| """Read a step lifecycle state with fallback for legacy objects.""" | ||
| raw = getattr(step, "status", "todo") |
There was a problem hiding this comment.
_step_state only checks getattr(step, "status", "todo"), so legacy dict-backed steps are always treated as todo even when they store "done"/"abandoned". In migrated sessions this breaks new state-machine paths: for example SubAgentTool.execute can miss the terminal-step guard and then call transition_step(..., "in_progress"), which raises ValueError outside the tool's try block and escapes as an exception instead of a structured ToolResult.
Useful? React with 👍 / 👎.
|
同步更新已完成并推送到 #146 头分支（ 本次只改了 1 个文件：
对齐内容：
并已重跑：
|
b4301eb to
f8519f3
Compare
Sync D7 acceptance-pack language to latest #142 contract updates. Changes: - Contract Delta now uses error semantics wording instead of error_code-only wording. - Observability locator wording now requires fixed IDs plus one-of error locator fields. Rationale: keep #146 evidence contract consistent with DARE's mixed runtime error representation model (ToolResult.error + ToolError code + hook error_code/error_type).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f8519f326a
ℹ️ 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".
| continue | ||
| step_id = _step_id(step) | ||
| if step_id is not None: | ||
| completed.add(step_id) | ||
| self.completed_step_ids = completed |
There was a problem hiding this comment.
PlannerState.sync_completed_step_ids now recomputes completed_step_ids only from steps whose runtime status is "done" and then overwrites the compatibility set. In migrated pre-D7 sessions, steps may legitimately have no status field while completed_step_ids already contains the true progress; after any tool path that calls this helper (for example validate_plan via _format_critical_block), previously completed steps are dropped back to pending, which can cause repeated execution and incorrect NEXT guidance.
Useful? React with 👍 / 👎.
| for step in self._state.steps: | ||
| if _step_state(step) in _PENDING_STATES: | ||
| self._state.transition_step(step.step_id, "abandoned") |
There was a problem hiding this comment.
The abandoned path in FinishPlanTool.execute transitions by step_id while iterating step objects, but transition_step resolves by first matching id. If duplicate step_ids are present (currently accepted by plan creation/revision inputs), the first matching step is transitioned repeatedly and later duplicates can remain pending, yet the tool still succeeds and sets plan_status to terminal; this leaves an internally inconsistent plan where not all non-terminal steps were abandoned.
Useful? React with 👍 / 👎.
8d9d778 to
e8a6d8f
Compare
Introduce an Evidence-first governance contract that blocks merge when acceptance-pack evidence is incomplete or semantically weak. Key changes: - Extend governance evidence gate to require Contract Delta, Golden Cases, Regression Summary, Observability + locator fields, and Structured Review Report. - Add semantic checks for schema/error_code/retry, pass/fail/skip summaries, and required observability markers (start/tool_call/end/fail). - Require review-link records to include Intent PR + Implementation PR markers and at least one concrete review/discussion link. - Expand gate scope to active + in_review feature docs so open PRs are covered by policy checks. - Update PR template, workflow labeling, and docs/skills constraints to align reviewers with evidence-first + risk-targeted sampling. - Add Acceptance Pack specification under docs/governance as a canonical checklist for authors and reviewers. Rationale: reduce reviewer load from full code-line reading to evidence-first verification while preserving deterministic merge quality signals.
Replace hard error_code requirement with framework-aligned error semantics while keeping Evidence-first rigor. Context: - DARE tool/runtime paths are mixed: ToolResult.error string is canonical, many tools emit output.code via ToolError, and hook paths emit error_code/error_type-style signals. - Requiring only error_code as mandatory evidence causes unnecessary friction for local agent-framework modules that do not expose cloud-style cross-service codes. Key changes: - Gate script now validates Contract Delta error semantics via one-of: error_code / error_type / exception_class / ToolResult.error mapping. - Observability checks now require run_id/tool_call_id/capability_id/attempt/trace_id plus at least one error locator from the same one-of set. - Docs/templates/skills updated to treat schema+retry as required declaration dimensions (changed OR none/n.a + reason), not mandatory change payloads. - Acceptance pack spec and SOP wording updated accordingly. Verification: - ./scripts/ci/check_governance_evidence_truth.sh (pass) - openspec validate --changes enhance-doc-governance-traceability (pass) - openspec status --change enhance-doc-governance-traceability --json (pass)
Address review findings in governance-evidence-truth gate for robustness under set -euo pipefail. Changes: - Prevent premature script termination when PR links are absent by guarding PR link count command substitution with a fallback. - Replace greedy GitHub PR URL regex with owner/repo scoped pattern to avoid undercount when multiple links exist on one line. - Relax Golden Cases file matcher to allow extensionless file names (still requires explicit file token or none-with-reason). Validation: - Local shell check confirms zero-link scenario now reports count=0 without abort. - ./scripts/ci/check_governance_evidence_truth.sh passes.
Deliver AgentScope D7 slice by introducing explicit plan/step lifecycle state management in plan_v2, adding missing plan revision/finish tools, and syncing docs/OpenSpec evidence under documentation-first workflow. Key changes: - add plan_v2 lifecycle state model (todo/in_progress/done/abandoned) with transition guards in types.py - add compatibility synchronization between step statuses and completed_step_ids - add ReviseCurrentPlanTool and FinishPlanTool with deterministic validation and terminal-state rules - update CreatePlanTool/ValidatePlanTool/SubAgentTool to drive state transitions and keep critical_block aligned - update planner tool registration, plan_v2 exports, and plan prompt guidance for revise/finish workflow - add D7 regression test matrix in tests/unit/test_plan_v2_tools.py - add docs-first artifacts: plan_v2 module design doc, D7 feature evidence doc, OpenSpec proposal/design/specs/tasks, and TODO claim/evidence sync Verification: - /Users/lang/workspace/github/Deterministic-Agent-Runtime-Engine/.venv/bin/pytest -q tests/unit/test_plan_v2_tools.py - /Users/lang/workspace/github/Deterministic-Agent-Runtime-Engine/.venv/bin/pytest -q tests/unit/test_plan_v2_tools.py tests/unit/test_react_agent_gateway_injection.py tests/unit/test_dare_agent_step_driven_mode.py - /Users/lang/workspace/github/Deterministic-Agent-Runtime-Engine/.venv/bin/pytest -q - openspec validate --changes "agentscope-d7-plan-state-tools" - openspec instructions apply --change "agentscope-d7-plan-state-tools" --json - ./scripts/ci/check_governance_evidence_truth.sh Rationale: This closes Gap-P1/P2/P3 in the AgentScope migration backlog while preserving runtime compatibility. Explicit lifecycle states and finish/revise tools remove ambiguous progress tracking and make critical-block guidance auditable and deterministic.
Update D7 feature and claim ledgers after PR creation so governance records match current execution state. Changes:\n- mark D7 feature doc as in_review and add PR/review links\n- update D7 claim notes in overall/domain TODO ledgers from pending PR to PR #138 in review\n- keep claim scope and owner mapping unchanged
sync_completed_step_ids previously accessed step.status and step.step_id directly, which can raise AttributeError when PlannerState is restored with legacy Step-like objects missing status.\n\nThis change introduces Step-like access helpers with dict/object fallback, then rebuilds completed_step_ids using normalized state and safe step_id extraction. Invalid/blank step ids are ignored instead of crashing the sync path.\n\nA regression test covers mixed modern + legacy + dict-backed steps to ensure sync remains stable and only includes valid terminal step ids.
The design-doc-drift CI gate enforces that every docs/design/modules/*/README.md includes the section header:\n"## 能力状态（landed / partial / planned）".\n\nThe plan_v2 module README lacked this section, causing ci-gate/design-doc-drift to fail on PR #138.\n\nThis commit adds the required section and documents landed/partial/planned status for plan_v2 so governance checks can validate module status consistency.
A new Codex review item on PR #138 identified that transition_step read step.status directly, which can raise AttributeError for restored/pre-D7 step objects lacking a status attribute.\n\nThis change aligns transition_step with the existing compatibility read path by using _step_state() for current-state evaluation, broadens get_step() lookup to use _step_id() fallback, and writes the normalized status via dict/object-compatible assignment.\n\nA regression test now verifies that transitioning a legacy step without status succeeds and persists the new lifecycle state.
Upgrade PR #138 feature evidence to the new Evidence-first acceptance-pack contract without changing runtime implementation. Key changes: - Add Contract Delta section (schema/error_code/retry semantics). - Add Golden Cases section with concrete updated files. - Add Regression Summary with runner commands and pass/fail/skip totals. - Add Observability and Failure Localization section with start/tool_call/end/fail markers and locator fields. - Add Structured Review Report covering module boundaries, new state, concurrency/timeout/retry, side effects, and residual risk. - Add Intent PR and Implementation PR markers in review links to satisfy merge-gate link contract. Rationale: make #138 reviewable under the new gate model and provide side-by-side comparison with old evidence style.
Sync D7 acceptance-pack language to latest #142 contract updates. Changes: - Contract Delta now uses error semantics wording instead of error_code-only wording. - Observability locator wording now requires fixed IDs plus one-of error locator fields. Rationale: keep #146 evidence contract consistent with DARE's mixed runtime error representation model (ToolResult.error + ToolError code + hook error_code/error_type).
f8519f3 to
3163516
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31635166fe
ℹ️ 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".
| completed = sorted(state.completed_step_ids) | ||
| pending = [s.step_id for s in state.steps if s.step_id not in state.completed_step_ids] | ||
| state.sync_completed_step_ids() | ||
| completed = [step.step_id for step in state.steps if _step_state(step) == "done"] |
There was a problem hiding this comment.
_format_critical_block still dereferences steps as dataclass objects (step.step_id / st.params) even though this change introduces legacy dict compatibility in PlannerState helpers and tests; when a migrated session contains dict-backed steps, any tool path that refreshes critical_block will raise AttributeError instead of returning a deterministic ToolResult. This makes normal flows like create/validate/sub-agent updates crash for legacy state snapshots.
Useful? React with 👍 / 👎.
| normalized_next = _normalize_state(next_state, default="todo") | ||
| if not is_valid_state_transition(current, normalized_next): |
There was a problem hiding this comment.
transition_plan and transition_step normalize unknown next_state values to "todo", so invalid inputs (for example "completed") are silently accepted whenever the current state is already todo instead of being rejected as illegal transitions. This violates the state-machine contract for deterministic rejection of invalid transitions and can hide caller/tool bugs by reporting success on malformed state names.
Useful? React with 👍 / 👎.
969df9e to
d59f169
Compare
Update D7 lifecycle status across project/agentscope boards and feature aggregation evidence to reflect merged delivery.\n\nChanges:\n- Mark CLM-20260302-D7 and CLM-20260302-AG3 as done with merged PR evidence (#138).\n- Align D7 priority table/stage annotations from active closeout to completed baseline.\n- Promote feature doc docs/features/agentscope-d7-plan-state-tools.md to status=done and add review/merge links (#138/#143/#146).\n- Activate next fix claim CLM-20260304-AG10 for T0-6 (search_file path contract regression) and move T0-6 status to doing.\n\nWhy:\n- Keep claim ledger, dependency board, and feature lifecycle synchronized with actual merged state.\n- Start next highest-priority, dependency-free P0 fix immediately after D7 closure.\n\nVerification:\n- ./scripts/ci/check_governance_evidence_truth.sh => passed\n- ./scripts/ci/check_governance_traceability.sh => passed\n- ./scripts/ci/check_design_doc_drift.sh => passed
1 participant
Goal
Provide the exact integration shape you requested:
So this PR represents:
138 code + 142 gate + retrofit docs.Stack relation
codex/evidence-first-gates-v2)codex/pr138-on-evidence-firstIncluded commits
Validation
./scripts/ci/check_governance_evidence_truth.sh=> passedReview guidance