Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 64 additions & 0 deletions backend/templates/prompts/build/build_workflow/_contract.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
{% if build.ticket_ref %}
**MANDATORY FIRST ACTION — fetch the ticket. This is not a suggestion.** Your very first tool call MUST be to fetch `{{ build.ticket_ref }}` from {{ build.source | default('the tracker', true) | capitalize }} using your available tools, then read its full description and **every** comment before you read the codebase, write a plan, edit a file, or emit any output. Do not begin from the ticket reference, title, or the rendered plan alone — those are derived; the ticket and its operator comments are the binding source of truth, and frequently carry exact decisions you must honor verbatim. The ONLY acceptable reason to proceed without the ticket's full text is a genuine tool failure, which you must report as a blocker — never guess or fabricate the requirements. If the source materially contradicts a plan or acceptance criteria rendered below, surface the conflict rather than silently proceeding.

{% endif %}
{% if build.journal.plan.plan_markdown %}
## Current plan

{{ build.journal.plan.plan_markdown }}

{% endif %}
{% if build.journal.plan.acceptance_criteria %}
## Acceptance criteria

{% for ac in build.journal.plan.acceptance_criteria %}
### {{ ac.id }}

**Description:** {{ ac.description.strip() }}

{% if ac.verification %}
**Verification:** {{ ac.verification.strip() }}

{% endif %}
{% endfor %}
{% endif %}
{% if build.journal.plan.rejected_approaches %}
## Ruled out

{% for approach in build.journal.plan.rejected_approaches %}
- {{ approach.strip() }}
{% endfor %}

{% endif %}
{% if build.journal.evaluations %}
## Prior implementation review

{% for review in build.journal.evaluations %}
### Round {{ loop.index }} — verdict: {{ review.verdict.value if review.verdict.value is defined else review.verdict }}

{% if review.body %}
{{ review.body.strip() }}

{% endif %}
{% endfor %}
{% endif %}
{% if build.journal.human_feedback %}
## Human feedback

{% for fb in build.journal.human_feedback %}
### {{ fb.reviewer }}

{% if fb.body %}
**Body:** {{ fb.body.strip() }}

{% endif %}
{% if fb.question %}
**Question:** {{ fb.question.strip() }}

{% endif %}
{% if fb.implementation_instructions %}
**Implementation instructions:** {{ fb.implementation_instructions.strip() }}

{% endif %}
{% endfor %}
{% endif %}
64 changes: 1 addition & 63 deletions backend/templates/prompts/build/build_workflow/_header.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@
- **Repo:** {{ build.repo }} · branch `{{ build.branch or '(none)' }}` · PR #{{ build.pr_number or '?' }}{% if build.issue_number %} · issue #{{ build.issue_number }}{% endif %}
{% endif %}
{% if build.ticket_ref %}
- **Ticket:** {{ build.ticket_ref }}

**MANDATORY FIRST ACTION — fetch the ticket. This is not a suggestion.** Your very first tool call MUST be to fetch `{{ build.ticket_ref }}` from {{ build.source | default('the tracker', true) | capitalize }} using your available tools, then read its full description and **every** comment before you read the codebase, write a plan, edit a file, or emit any output. Do not begin from the ticket reference, title, or the rendered plan alone — those are derived; the ticket and its operator comments are the binding source of truth, and frequently carry exact decisions you must honor verbatim. The ONLY acceptable reason to proceed without the ticket's full text is a genuine tool failure, which you must report as a blocker — never guess or fabricate the requirements. If the source materially contradicts a plan or acceptance criteria rendered below, surface the conflict rather than silently proceeding.
- **Ticket:** {{ build.ticket_ref }} on {{ build.source | default('the tracker', true) | capitalize }}
{% endif %}
- **Plan revision:** {{ build.journal.plan_revision }}
- **Implementation revision:** {{ build.journal.implementation_revision }}{% if build.journal.implementation_revision == 0 %} (first attempt){% endif %}
Expand All @@ -26,63 +24,3 @@
- ``workspace_root``: `{{ workspace.workspace_root }}` — clone related repos you need as ``workspace_root/related/<name>``

{{ verification }}
{% if build.journal.plan.plan_markdown %}
## Current plan

{{ build.journal.plan.plan_markdown }}

{% endif %}
{% if build.journal.plan.acceptance_criteria %}
## Acceptance criteria

{% for ac in build.journal.plan.acceptance_criteria %}
### {{ ac.id }}

**Description:** {{ ac.description.strip() }}

{% if ac.verification %}
**Verification:** {{ ac.verification.strip() }}

{% endif %}
{% endfor %}
{% endif %}
{% if build.journal.plan.rejected_approaches %}
## Ruled out

{% for approach in build.journal.plan.rejected_approaches %}
- {{ approach.strip() }}
{% endfor %}

{% endif %}
{% if build.journal.evaluations %}
## Prior implementation review

{% for review in build.journal.evaluations %}
### Round {{ loop.index }} — verdict: {{ review.verdict.value if review.verdict.value is defined else review.verdict }}

{% if review.body %}
{{ review.body.strip() }}

{% endif %}
{% endfor %}
{% endif %}
{% if build.journal.human_feedback %}
## Human feedback

{% for fb in build.journal.human_feedback %}
### {{ fb.reviewer }}

{% if fb.body %}
**Body:** {{ fb.body.strip() }}

{% endif %}
{% if fb.question %}
**Question:** {{ fb.question.strip() }}

{% endif %}
{% if fb.implementation_instructions %}
**Implementation instructions:** {{ fb.implementation_instructions.strip() }}

{% endif %}
{% endfor %}
{% endif %}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ plan should have asked for. You verify what it did ask for, exhaustively, in a s
AWARENESS rule below.

{% include "build/build_workflow/_header.md" %}
{% include "build/build_workflow/_contract.md" %}
{% include "build/build_workflow/_related_repos.md" %}
{% include "build/build_workflow/_skills.md" %}
Evaluate the implementation against the **Current plan** above, the issue, and the current PR diff. When `base_sha` and `head_sha` are listed in the **Workflow context** section above, use them as the authoritative diff range and evaluate `head_sha` against `base_sha`. If branch names disagree with those SHAs, trust the SHAs and mention metadata drift only when it affects the PR. Return blocked if an authoritative SHA is unavailable locally after fetching. Evaluate every acceptance criterion from the PR state and report one result per criterion. Inspection commands such as git diff/show, rg, and sed are allowed for review. Verification commands are different: run only the configured verification profile commands when feasible and report their results in checks. Do not invent repo-specific smoke tests or package install commands. Return exactly one final result object. Return pass only when the work is ready for a human final PR review. Return fail for actionable implementation changes.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ your acceptance criteria. Wrong shape here compounds into every step that follow
- Preserve the source's explicit out-of-scope statements near-verbatim.

{% include "build/build_workflow/_header.md" %}
{% include "build/build_workflow/_contract.md" %}
{% include "build/build_workflow/_related_repos.md" %}
{% include "build/build_workflow/_skills.md" %}
{% if answered_questions %}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ acceptance criterion — nothing more, nothing less.
catch it and the loop cost is the same — except the operator never sees your explanation.

{% include "build/build_workflow/_header.md" %}
{% include "build/build_workflow/_contract.md" %}
{% include "build/build_workflow/_related_repos.md" %}
{% include "build/build_workflow/_skills.md" %}
Implement the approved plan (rendered above as **Current plan**) on the work branch (see "Delivering your work" below). If the **Human feedback** section above carries an entry with implementation instructions, apply those instructions as the current revision request. Do not run ad hoc install, lint, test, build, or smoke commands during implementation unless the plan explicitly requires changing those commands or generated outputs. The evaluator runs and adjudicates the verification profile — you do not run or judge it; report every check as structured evidence (`not_run` or an observed baseline failure). Never leave dependency lockfile, generated, or cache changes unless they are part of the requested implementation. Return structured evidence for every acceptance criterion, every check you ran or intentionally did not run, changed files, and known risks. If a check was not run, include status not_run and a reason.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ write it?
there is at least one medium or high finding.

{% include "build/build_workflow/_header.md" %}

**MANDATORY FIRST ACTION — read the diff. This is not a suggestion.** Your very first tool call MUST be `git diff <base_sha>..<head_sha>` using the SHAs rendered above. Then read every changed file END TO END — the whole file, not the changed hunks — before writing any finding. You are reviewing code, not a plan: you have no plan and no acceptance criteria here by design, because correctness against the contract was already adjudicated by the evaluator. Fetch the ticket when you need its scope to judge a finding, and to file the follow-up sub-issue below.

{% include "build/build_workflow/_related_repos.md" %}
{% include "build/build_workflow/_skills.md" %}
Review the implementation as a code reviewer, AFTER the evaluator has already passed on correctness against the acceptance criteria. Your job is the holistic quality pass: would you be happy maintaining this code in 6 months?
Expand All @@ -43,15 +46,15 @@ WHAT TO LOOK FOR — beyond AC correctness, which is the evaluator's job, not yo
- Foot-guns: surprising default values, unsafe casts, swallowed exceptions, missing input validation on a public boundary, log-then-continue patterns where the caller can't tell something failed.
- Secret leaks, obvious injection paths, log lines that include sensitive data.
- Comments and naming that lie or mislead — drift between what the comment claims and what the code does.
- Edits outside the ticket scope: a file the change touches that the ticket never asked for is a finding; name it by file path.

WHAT NOT TO FLAG:
- Anything the evaluator already covered (AC correctness). Don't relitigate.
- Style nits that don't change behavior or maintainability (whitespace, alphabetical import order, single vs double quote unless the repo enforces one).
- "I would have done this differently" without a concrete reason tied to maintainability, performance, or correctness.
- Pre-existing issues in unchanged code. You're reviewing the diff, not the codebase.

SEVERITY CALIBRATION:
- high: correctness bug, security flaw, data loss path, or a duplicate of an existing helper that's already used elsewhere in the repo (someone will fix the other usages later and miss this one).
- high: correctness bug, security flaw, data loss path, an out-of-scope edit, or a duplicate of an existing helper that's already used elsewhere in the repo (someone will fix the other usages later and miss this one).
- medium: test shape problem (asserting against implementation details), missing test for a non-trivial new behavior, idiomatic mismatch that will confuse future readers, log-then-continue pattern, unsafe default.
- low: naming clarity, comment drift, a small refactor that would simplify the next change but isn't required now.

Expand All @@ -63,7 +66,7 @@ You do the writing yourself — druks records only your one-line `summary`.

2. **Post one PR comment** on PR #{{ build.pr_number }} (`gh pr comment {{ build.pr_number }} --body ...`; the checkout is authenticated): `Code review: <your one-sentence summary, verbatim>`.

3. **File a follow-up sub-issue — only when step 1 found real work.** Open it on the same tracker as the parent ticket (the one you fetched at the start), as a child of that ticket, using the same tracker tools. Give it a concise verb-first title (e.g. "Extract duplicate validation helper", "Add integration test for refund path"). In the body write one section per finding — the follow-up implementer reads it as spec — each covering:
3. **File a follow-up sub-issue — only when step 1 found real work.** Open it on the same tracker as the parent ticket named in **Workflow context**, as a child of that ticket, using the same tracker tools. Give it a concise verb-first title (e.g. "Extract duplicate validation helper", "Add integration test for refund path"). In the body write one section per finding — the follow-up implementer reads it as spec — each covering:
- severity: high / medium / low
- what's wrong, why it matters, and what good would look like
- the file path and anchor line it applies to, when it has one
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ implementation — your verdict is the gate.
- Do not require changes beyond what the issue and the existing codebase support.

{% include "build/build_workflow/_header.md" %}
{% include "build/build_workflow/_contract.md" %}
{% include "build/build_workflow/_related_repos.md" %}
{% include "build/build_workflow/_skills.md" %}
Review the current plan in one complete pass. Batch every blocking issue and every required clarification into a single response — there is no second automatic round.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ everything the original plan got right.
- Do not add scope beyond what the human feedback introduced.

{% include "build/build_workflow/_header.md" %}
{% include "build/build_workflow/_contract.md" %}
{% include "build/build_workflow/_related_repos.md" %}
{% include "build/build_workflow/_skills.md" %}
The human reviewer's feedback contradicts the current acceptance criteria. Revise the plan and acceptance criteria to incorporate the feedback while preserving the original issue intent. The latest triaged human feedback is in the **Human feedback** section above; the current acceptance criteria are in the **Acceptance criteria** section above. Return the full revised plan markdown, the complete updated acceptance criteria list, and concise implementation instructions describing what changed so the implementer knows what to redo.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ without ambiguity.
- Do not edit files.

{% include "build/build_workflow/_header.md" %}
{% include "build/build_workflow/_contract.md" %}
{% include "build/build_workflow/_related_repos.md" %}
{% include "build/build_workflow/_skills.md" %}
Triage the latest pending entry in the **Human feedback** section above. Decide whether the feedback requires code changes, is incorrect or already addressed, needs a follow-up question, or means the PR should be closed/cancelled. Do not edit files. Use action `no_change` when no code change is needed and explain why in body. Use `change_required` only for actionable implementation work and put precise instructions for the implementer in implementation_instructions. Use `contract_change_required` when the feedback contradicts or invalidates one or more acceptance criteria — for example the human rejects a design choice that the criteria explicitly required. In that case put revised instructions in implementation_instructions explaining what changed. Use `question` when human input is needed before acting and put the exact question in question. Use `close` only when the correct action is to stop the PR.
Expand Down
73 changes: 69 additions & 4 deletions backend/tests/test_build_prompts.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@
"triage_human_feedback.md",
]

# The roles that carry the plan, the acceptance criteria, and the ticket behind
# them. The code reviewer is deliberately outside this set: it grades the diff.
_CONTRACT_TEMPLATES = [name for name in _OP_TEMPLATES if name != "review_code.md"]

# The kwargs the workflow passes at each template's agent call site.
_CALL_KWARGS = {
"generate_plan.md": {"answered_questions": [], "operator_note": "", "reviewer_notes": ""},
Expand Down Expand Up @@ -156,11 +160,12 @@ async def test_the_planner_resolves_the_assignee_not_the_reviewer():
assert "assignee_github_login" not in reviewer


@pytest.mark.parametrize("template", _OP_TEMPLATES)
@pytest.mark.parametrize("template", _CONTRACT_TEMPLATES)
async def test_build_prompt_orders_the_ticket_fetch(template):
"""Every build agent is ordered to fetch the ticket from its source before
acting — a mandatory first step, not a suggestion. Regression guard for the
silently-skipped-fetch bug (agents working off the ticket ref alone)."""
"""Every agent that works the contract is ordered to fetch the ticket from its
source before acting — a mandatory first step, not a suggestion. Regression
guard for the silently-skipped-fetch bug (agents working off the ticket ref
alone). The code reviewer is ordered to read the diff instead."""
build = _build()
build.source = "linear"
output = await render_prompt(
Expand Down Expand Up @@ -193,6 +198,66 @@ async def test_review_code_prompt_owns_its_followup_subissue():
assert '"summary"' in output # the only thing it returns


async def test_contract_context_is_omitted_only_from_code_review():
build = _build()
build.journal = SimpleNamespace(
plan_revision=1,
implementation_revision=1,
last_implementation=SimpleNamespace(base_sha="base123", head_sha="head456"),
plan=SimpleNamespace(
plan_markdown="PLAN-MARKER",
acceptance_criteria=[
SimpleNamespace(
id="AC-1",
description="AC-MARKER",
verification="VERIFY-MARKER",
)
],
rejected_approaches=["RULED-OUT-MARKER"],
),
evaluations=[SimpleNamespace(verdict="pass", body="EVALUATION-MARKER")],
human_feedback=[
SimpleNamespace(
reviewer="REVIEWER-MARKER",
body="FEEDBACK-MARKER",
question="QUESTION-MARKER",
implementation_instructions="INSTRUCTIONS-MARKER",
)
],
)
headings = (
"## Current plan",
"## Acceptance criteria",
"## Prior implementation review",
"## Human feedback",
)
markers = headings + (
"MANDATORY FIRST ACTION — fetch the ticket",
"PLAN-MARKER",
"AC-MARKER",
"RULED-OUT-MARKER",
"EVALUATION-MARKER",
"FEEDBACK-MARKER",
"QUESTION-MARKER",
"INSTRUCTIONS-MARKER",
)
for template in _OP_TEMPLATES:
output = await render_prompt(
f"build/build_workflow/{template}",
build=build,
verification="VERIFICATION-BLOCK",
workspace=_workspace(),
**_CALL_KWARGS.get(template, {}),
)
if template == "review_code.md":
assert all(marker not in output for marker in markers)
assert "`git diff <base_sha>..<head_sha>`" in output
else:
positions = [output.index(heading) for heading in headings]
assert positions == sorted(positions)
assert all(marker in output for marker in markers)


def test_build_prompt_context_covers_template_attrs():
# Every build prompt reads build.<attr>; assert BuildPromptContext carries them
# all, so a template ref can never outrun the context contract.
Expand Down
2 changes: 2 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ extend-exclude = ["backend/migrations/versions"]

[tool.ruff.lint]
select = ["E", "F", "I", "UP", "B", "SIM"]
# SIM108 rewrites if/else into a ternary; formatter-wrapped ternaries are banned here, and plain if/else is the house form.
ignore = ["SIM108"]

[tool.ruff.lint.flake8-bugbear]
# FastAPI dependency markers are the documented way to declare injected
Expand Down