guard reviews against issue rediscovery
This commit is contained in:
@@ -183,7 +183,14 @@ class CodexClient:
|
||||
output_path.unlink(missing_ok=True) # noqa: ASYNC240
|
||||
|
||||
def _environment(self) -> dict[str, str]:
|
||||
allowed = {"PATH", "LANG", "LC_ALL", "SSL_CERT_FILE", "CODEX_CA_CERTIFICATE"}
|
||||
allowed = {
|
||||
"PATH",
|
||||
"LANG",
|
||||
"LC_ALL",
|
||||
"SSL_CERT_FILE",
|
||||
"CODEX_CA_CERTIFICATE",
|
||||
"XDG_CONFIG_HOME",
|
||||
}
|
||||
environment = {key: value for key, value in os.environ.items() if key in allowed}
|
||||
environment["CODEX_HOME"] = str(self.codex_home)
|
||||
if self.context7_api_key:
|
||||
|
||||
@@ -1,11 +1,26 @@
|
||||
Independently review the current uncommitted implementation against the issue context. Inspect the
|
||||
working tree and diff yourself. Do not edit files. Focus on correctness, regressions, security,
|
||||
missing tests, and whether the requested behavior is actually complete.
|
||||
Independently review only the proposed pull request, the issue it fixes, and the canonical plan
|
||||
from that issue. Inspect the pull request diff yourself, but do not inspect or review unrelated
|
||||
repository files, history, branches, issues, or other repository-wide concerns. You may read a
|
||||
changed file only as needed to understand a changed diff hunk. Do not edit files. Focus on concrete
|
||||
correctness, regressions, security, missing tests, and whether the requested behavior is complete.
|
||||
|
||||
<issue_context>
|
||||
$context
|
||||
$issue_context
|
||||
</issue_context>
|
||||
|
||||
<canonical_plan>
|
||||
$artifact
|
||||
</canonical_plan>
|
||||
|
||||
<pull_request_context>
|
||||
$pull_context
|
||||
</pull_request_context>
|
||||
|
||||
Before reporting any finding, run
|
||||
`tea issues list --repo OWNER/REPO --state all --keyword "FOCUSED KEYWORDS"` for the repository
|
||||
named by the issue context. This duplicate search is the only permitted access to other issues. If
|
||||
a matching issue already tracks the finding, cite its number in the finding. Do not use `tea` to
|
||||
create, edit, close, reopen, comment on, or otherwise mutate issues or pull requests.
|
||||
|
||||
Return a structured review. Use `blocking` only when the result cannot safely be proposed, `major`
|
||||
for a material defect, and `minor` for a non-blocking improvement.
|
||||
|
||||
|
||||
@@ -1,6 +1,14 @@
|
||||
You are the independent reviewer for a proposed implementation plan. Inspect the repository
|
||||
yourself and compare the plan to the issue. Do not edit files. Identify concrete correctness,
|
||||
security, compatibility, missing-decision, and testing problems. Do not invent speculative work.
|
||||
You are the independent reviewer for a proposed implementation plan. Review only the proposed
|
||||
plan against the issue context below. Do not inspect the repository, working tree, Git history,
|
||||
or any other repository material, and do not edit files. Identify concrete correctness, security,
|
||||
compatibility, missing-decision, and testing problems. Do not invent speculative work.
|
||||
|
||||
Before reporting any finding, run
|
||||
`tea issues list --repo OWNER/REPO --state all --keyword "FOCUSED KEYWORDS"` for the repository
|
||||
named by the issue context. This duplicate search is the only permitted access to repository
|
||||
material outside the issue context and plan. If a matching issue already tracks the finding, cite
|
||||
its number in the finding. Do not use `tea` to create, edit, close, reopen, comment on, or otherwise
|
||||
mutate issues or pull requests.
|
||||
|
||||
<issue_context>
|
||||
$context
|
||||
@@ -12,4 +20,3 @@ $artifact
|
||||
|
||||
Return a structured review. Use `blocking` only when work cannot safely proceed, `major` for a
|
||||
material defect or unresolved implementation decision, and `minor` for a non-blocking improvement.
|
||||
|
||||
|
||||
@@ -12,7 +12,8 @@ class CodeReviewLoop:
|
||||
self,
|
||||
job: Job,
|
||||
workflow: Workflow,
|
||||
context: str,
|
||||
issue_context: str,
|
||||
plan: str,
|
||||
result: AgentResult,
|
||||
) -> tuple[AgentResult, ReviewReport]:
|
||||
report = ReviewReport(summary="", findings=[])
|
||||
@@ -22,7 +23,15 @@ class CodeReviewLoop:
|
||||
stage=f"reviewing implementation {round_index + 1}/"
|
||||
f"{self.deps.settings.implement_review_rounds}",
|
||||
)
|
||||
report = await self.once(workflow, context)
|
||||
report = await self.once(
|
||||
workflow,
|
||||
issue_context=issue_context,
|
||||
plan=plan,
|
||||
pull_context=(
|
||||
"The proposed pull request is the current uncommitted working-tree diff. "
|
||||
"Review only that diff."
|
||||
),
|
||||
)
|
||||
workflow.artifact = result.model_dump_json()
|
||||
workflow.review_json = report_json(report)
|
||||
await self.deps.storage.update_workflow(workflow)
|
||||
@@ -46,15 +55,27 @@ class CodeReviewLoop:
|
||||
)
|
||||
return result, report
|
||||
|
||||
async def once(self, workflow: Workflow, context: str) -> ReviewReport:
|
||||
prompt = self.deps.prompts.render("implementation_review", context=context)
|
||||
async def once(
|
||||
self,
|
||||
workflow: Workflow,
|
||||
*,
|
||||
issue_context: str,
|
||||
plan: str,
|
||||
pull_context: str,
|
||||
) -> ReviewReport:
|
||||
prompt = self.deps.prompts.render(
|
||||
"implementation_review",
|
||||
issue_context=issue_context,
|
||||
artifact=plan,
|
||||
pull_context=pull_context,
|
||||
)
|
||||
if workflow.reviewer_session_id:
|
||||
return await self.deps.codex.resume(
|
||||
session_id=workflow.reviewer_session_id,
|
||||
prompt=prompt,
|
||||
model=self.deps.settings.implement_model,
|
||||
reasoning=self.deps.settings.implement_reasoning,
|
||||
permission="agentci-read",
|
||||
permission="agentci-review",
|
||||
workspace=workflow.workspace_path,
|
||||
schema_name="review.json",
|
||||
result_type=ReviewReport,
|
||||
@@ -64,7 +85,7 @@ class CodeReviewLoop:
|
||||
prompt=prompt,
|
||||
model=self.deps.settings.implement_model,
|
||||
reasoning=self.deps.settings.implement_reasoning,
|
||||
permission="agentci-read",
|
||||
permission="agentci-review",
|
||||
schema_name="review.json",
|
||||
result_type=ReviewReport,
|
||||
)
|
||||
|
||||
@@ -83,7 +83,13 @@ class ImplementWorkflow:
|
||||
workflow.primary_session_id = session_id
|
||||
workflow.artifact = result.model_dump_json()
|
||||
await self.deps.storage.update_workflow(workflow)
|
||||
result, report = await self.review.run(job, workflow, context, result)
|
||||
result, report = await self.review.run(
|
||||
job,
|
||||
workflow,
|
||||
context,
|
||||
plan.artifact if plan and plan.artifact else "(no canonical plan)",
|
||||
result,
|
||||
)
|
||||
sha = await self.changes.commit_and_push(
|
||||
job,
|
||||
workspace,
|
||||
|
||||
@@ -176,7 +176,7 @@ class PlanWorkflow:
|
||||
prompt=prompt,
|
||||
model=self.deps.settings.plan_model,
|
||||
reasoning=self.deps.settings.plan_reasoning,
|
||||
permission="agentci-read",
|
||||
permission="agentci-review",
|
||||
workspace=workflow.workspace_path,
|
||||
schema_name="review.json",
|
||||
result_type=ReviewReport,
|
||||
@@ -186,7 +186,7 @@ class PlanWorkflow:
|
||||
prompt=prompt,
|
||||
model=self.deps.settings.plan_model,
|
||||
reasoning=self.deps.settings.plan_reasoning,
|
||||
permission="agentci-read",
|
||||
permission="agentci-review",
|
||||
schema_name="review.json",
|
||||
result_type=ReviewReport,
|
||||
)
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
from __future__ import annotations
|
||||
|
||||
from agentci.domain.models import AgentResult, Job, WorkflowStatus
|
||||
from agentci.domain.models import AgentResult, Job, WorkflowKind, WorkflowStatus
|
||||
from agentci.workflows.change_set import ChangeSet, result_comment
|
||||
from agentci.workflows.code_review import CodeReviewLoop
|
||||
from agentci.workflows.common import (
|
||||
@@ -58,7 +58,18 @@ class PullRequestWorkflow:
|
||||
schema_name="agent_result.json",
|
||||
result_type=AgentResult,
|
||||
)
|
||||
report = await self.review.once(workflow, context)
|
||||
issue_context = await self.deps.context.issue_context(
|
||||
job.repo_owner, job.repo_name, workflow.issue_number
|
||||
)
|
||||
plan = await self.deps.storage.latest_workflow(
|
||||
job.repo_owner, job.repo_name, workflow.issue_number, WorkflowKind.PLAN
|
||||
)
|
||||
report = await self.review.once(
|
||||
workflow,
|
||||
issue_context=issue_context,
|
||||
plan=plan.artifact if plan and plan.artifact else "(no canonical plan)",
|
||||
pull_context=context,
|
||||
)
|
||||
sha = await self.changes.commit_and_push(
|
||||
job,
|
||||
workflow.workspace_path,
|
||||
|
||||
Reference in New Issue
Block a user