From d62300a0e14bd051ce6e21ca89c4fc63b748e64c Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Sat, 12 Sep 2026 10:58:34 +0000 Subject: [PATCH 1/7] Devin: deliver authorized context packets to Devin roles and hosted builder input Closes #908 Co-Authored-By: bot_apk --- README.md | 2 +- docs/context-delivery.md | 35 +++++-- docs/sessions.md | 4 +- src/code_mower/context_command.py | 10 +- src/code_mower/context_delivery.py | 12 ++- src/code_mower/context_guided.py | 11 ++- src/code_mower/context_prepare.py | 5 +- src/code_mower/devin_work_orders.py | 72 +++++++++++++-- tests/test_context_delivery.py | 42 ++++++++- tests/test_context_guided.py | 69 ++++++++++++++ tests/test_devin_work_orders.py | 136 +++++++++++++++++++++++++++- 11 files changed, 356 insertions(+), 42 deletions(-) diff --git a/README.md b/README.md index 8bf957ee..03ed809c 100644 --- a/README.md +++ b/README.md @@ -116,7 +116,7 @@ execution remains an explicit handoff or provider-specific transport. See ## Optional Organizational Context The base installation works without an organizational-memory provider. The -optional Coworker integration gives approved Claude and Codex roles the same +optional Coworker integration gives approved Claude, Codex, and Devin roles the same bounded, cited evidence through an explicitly selected private account. Account identity and credentials stay outside the repository. diff --git a/docs/context-delivery.md b/docs/context-delivery.md index 17f00dcb..84daf16f 100644 --- a/docs/context-delivery.md +++ b/docs/context-delivery.md @@ -1,7 +1,7 @@ -# Share optional evidence with Claude and Codex +# Share optional evidence with Claude, Codex, and Devin An approved context packet can now accompany a work order and its independent -review. Both hosts use the same evidence renderer and authorization checks. +review. Every host uses the same evidence renderer and authorization checks. The calling host remains the orchestrator; a context provider gains no builder, reviewer, tracker-write, or merge authority. Ordinary sessions without context keep their existing behavior and do not load the optional Coworker SDK. @@ -10,9 +10,10 @@ Start with a verified [private connection](context-connections.md). The guided path below fetches once for the work item and retains the opaque packet handle; the lower-level workflow can still fetch it explicitly. Every delivery verifies authorization online; it does not repeat the search. -Only Claude and Codex orchestrator, builder, and reviewer roles are supported -for private delivery in this release. Each role must be explicitly approved in -the private connection configuration. +Only Claude, Codex, and Devin orchestrator, builder, and reviewer roles are +supported for private delivery in this release. Each role must be explicitly +approved in the private connection configuration as `:`; a +connection without `devin:*` recipients never delivers to Devin. ## Guided session path @@ -53,8 +54,8 @@ and diagnostics. `session context deliver` derives the repository, work item, connection, policy, packet, and selected builder from protected state. It writes the private evidence to stdout for the builder's prompt; keep that output out of tracked -files and public logs. The selected builder must be Claude or Codex in this -release. +files and public logs. The selected builder must be Claude, Codex, or Devin in +this release. ## Work order and builder @@ -74,6 +75,26 @@ Use the same scope and policy as the original fetch. Evidence is written to stdout for the approved participant's prompt. Keep it out of public terminal logs, tracked files, PR descriptions, and shared artifacts. +### Hosted Devin builder + +A trusted hosted work order can carry the packet to `devin:builder` without a +local prompt file. The embedding binds one packet with +`devin_work_orders.packet_context(...)` and passes the resulting callable as +`context=` to `dispatch`, `clarify`, or `fix`. Immediately before each paid +create or message write, and never in preview, the callable performs a new +online authorization for `devin:builder` and renders the common evidence +payload. That text is appended only to the provider input; the local work-order +record and remote-session record keep only their existing digests, and status, +collect, and cancel reject a context argument. + +Wrong account, revoked or expired authorization, a repository or recipient +outside the connection, a refreshed or invalidated packet, or a payload without +the packet identity fails closed as `context_unavailable` before any local +round or provider write. Omitting `context=` is the only way to degrade to a +code-only work order, and the remote session's dispatch fingerprint prevents a +later dispatch from silently replaying with different input. The combined input +is bounded at 64 KiB (`context_budget_exceeded`). + ## Attach evidence to independent review After creating the PR, attach the packet to its current code head without a diff --git a/docs/sessions.md b/docs/sessions.md index 2b5abb5e..60806a8d 100644 --- a/docs/sessions.md +++ b/docs/sessions.md @@ -136,7 +136,7 @@ different participant; review lanes then exclude that builder. `--output` accepts a repository-relative path only. The command rechecks the session lease and trusted context policy, authorizes -the calling Claude or Codex host, fetches through the existing bounded provider +the calling Claude, Codex, or Devin host, fetches through the existing bounded provider contract, drafts through the existing work-order contract, and saves only a request hash plus opaque packet and work-order references in private session state. Its output does not contain the work-item identity, query, connection, @@ -182,7 +182,7 @@ Running `prepare` while publication is pending or uncertain preserves the saved intent and directs the session back to `attach`; it never drops the revision or repeats provider authorization while publication recovery is unresolved. -After a selected Claude or Codex reviewer finishes, give its private findings +After a selected Claude, Codex, or Devin reviewer finishes, give its private findings to the selected builder without copying the revision: ```bash diff --git a/src/code_mower/context_command.py b/src/code_mower/context_command.py index 20416041..dcde73b3 100644 --- a/src/code_mower/context_command.py +++ b/src/code_mower/context_command.py @@ -13,7 +13,7 @@ from . import context_review from .claude_audit_pr import _decision_authorities_for_repo from .context_contract import ContextError, ContextRequest, _object, normalize_policy -from .context_delivery import attach, deliver, read_binding, render_evidence +from .context_delivery import SUPPORTED_HOSTS, SUPPORTED_RECIPIENTS, attach, deliver, read_binding, render_evidence from .context_packets import load_authorized from .context_store import ContextStore, strict_json from .provider_runners import fetch_issue_comments, fetch_pull_request, post_pr_comment @@ -39,7 +39,7 @@ def main(argv=None): attach_parser.add_argument("--connection", required=True) attach_parser.add_argument("--unavailable", action="store_true", help="Explicitly declare unavailable context; optional work may continue with a fresh code-only review") attach_parser.add_argument("--request-stdin", action="store_true", required=True) - attach_parser.add_argument("--host", choices=("claude", "codex"), default=os.environ.get("CODE_MOWER_HOST")) + attach_parser.add_argument("--host", choices=SUPPORTED_HOSTS, default=os.environ.get("CODE_MOWER_HOST")) for verb in ("deliver", "feedback"): command = sub.add_parser(verb, help="Output private evidence or findings only after authorization") command.add_argument("--revision", help="Previously attached PR input revision") @@ -48,7 +48,7 @@ def main(argv=None): command.add_argument("--request-stdin", action="store_true") command.add_argument("--recipient", required=True, help="Approved host:role, such as codex:builder") if verb == "feedback": - command.add_argument("--reviewer", choices=("claude", "codex"), required=True) + command.add_argument("--reviewer", choices=SUPPORTED_HOSTS, required=True) for command in (attach_parser, *[sub.choices[name] for name in ("deliver", "feedback")]): command.add_argument("--state-dir", type=Path) command.add_argument("--repo-path", type=Path, default=Path.cwd(), help="Target repository checkout for trusted base configuration") @@ -60,7 +60,7 @@ def main(argv=None): if args.revision or not args.connection or not args.request_stdin: raise ContextError("packet delivery requires a connection and private request on stdin") spec = _object(_private_spec(), {"repository", "work_item", "policy"}) - if args.recipient not in ("claude:orchestrator", "codex:orchestrator", "claude:builder", "codex:builder"): + if args.recipient not in SUPPORTED_RECIPIENTS or args.recipient.endswith(":reviewer"): raise ContextError("independent reviewers consume an attached review revision") packet = load_authorized(store, args.connection, args.packet, spec["policy"], ContextRequest(spec["repository"], spec["work_item"], args.recipient)) @@ -76,7 +76,7 @@ def main(argv=None): policy = normalize_policy(spec['policy']) if policy is None or policy['connection'] != args.connection: raise ContextError('select the connection named by the work-item policy') - if args.host not in ("claude", "codex"): + if args.host not in SUPPORTED_HOSTS: raise ContextError("supply the calling host when attaching context") if type(spec["pr"]) is not int or spec["pr"] < 1: raise ContextError("context attachment requires a PR number") diff --git a/src/code_mower/context_delivery.py b/src/code_mower/context_delivery.py index fde4739a..e53bc5e7 100644 --- a/src/code_mower/context_delivery.py +++ b/src/code_mower/context_delivery.py @@ -13,8 +13,10 @@ SCHEMA = "code_mower.contextDelivery.v1" MAX_DELIVERY_BYTES = 80_000 -SUPPORTED_RECIPIENTS = frozenset(f"{host}:{role}" for host in ("claude", "codex") - for role in ("orchestrator", "builder", "reviewer")) +SUPPORTED_HOSTS = ("claude", "codex", "devin") +SUPPORTED_ROLES = ("orchestrator", "builder", "reviewer") +SUPPORTED_RECIPIENTS = frozenset(f"{host}:{role}" for host in SUPPORTED_HOSTS for role in SUPPORTED_ROLES) +SUPPORTED_PROVIDERS = ("Claude", "Codex", "Devin") def render_evidence(packet: ValidatedPacket, revision: str) -> str: @@ -59,7 +61,7 @@ def _binding(value): if metadata["revision"] != value["revision"] or metadata["required"] != policy["required"]: raise ContextError("context delivery metadata does not match its binding") feedback = value["feedback"] - if (not isinstance(feedback, dict) or feedback.keys() - {"claude", "codex"} + if (not isinstance(feedback, dict) or feedback.keys() - set(SUPPORTED_HOSTS) or any(not isinstance(v, str) or len(v.encode("utf-8")) > 100_000 for v in feedback.values())): raise ContextError("context feedback exceeds its private storage budget") return dict(value) @@ -237,7 +239,7 @@ def deliver(store, revision, *, repository, pr, head, recipient, current, backen def save_feedback(store, delivery: Delivery, host, prose): """Only credential-free verdict prose is retained, under connection cleanup.""" - if host not in ("claude", "codex") or not isinstance(prose, str) or len(prose.encode()) > 100_000: + if host not in SUPPORTED_HOSTS or not isinstance(prose, str) or len(prose.encode()) > 100_000: raise ContextError("private review feedback exceeds its supported budget") with store.locked(delivery.binding["connection"]) as locked: artifact = locked.artifact("d-" + delivery.metadata["revision"]) @@ -251,7 +253,7 @@ def save_feedback(store, delivery: Delivery, host, prose): def public_verdict(delivery, *, provider, head, verdict, counts, trailer, actions_run_id=None, merge_authority=True): """Never publish model-authored prose when private context was supplied.""" - if provider not in ("Claude", "Codex") or verdict not in ("PASS", "BLOCKED", "UNKNOWN", "STALE"): + if provider not in SUPPORTED_PROVIDERS or verdict not in ("PASS", "BLOCKED", "UNKNOWN", "STALE"): raise ContextError("unsupported context review metadata") if len(counts) != 4 or any(type(value) is not int or not 0 <= value <= 1000 for value in counts): raise ContextError("invalid context review counts") diff --git a/src/code_mower/context_guided.py b/src/code_mower/context_guided.py index d05a02dc..aee2504b 100644 --- a/src/code_mower/context_guided.py +++ b/src/code_mower/context_guided.py @@ -11,6 +11,7 @@ from .claude_audit_pr import _decision_authorities_for_repo from .context_contract import ContextError, ContextRequest from .context_delivery import ( + SUPPORTED_HOSTS, SUPPORTED_RECIPIENTS, abandon_attachment, deliver, @@ -197,8 +198,8 @@ def attach_session( raise ContextError("prepare the selected context before attaching it") if record["stage"] not in {"prepared", "attached", "reviewed"}: raise ContextError("prepare the selected context before attaching it") - if record["host"] not in {"claude", "codex"}: - raise ContextError("guided private context currently supports Claude and Codex hosts") + if record["host"] not in SUPPORTED_HOSTS: + raise ContextError("guided private context currently supports Claude, Codex, and Devin hosts") token, authorities = _github_access(repo_path, base_ref) with association_store.locked(_workflow_key(record)): @@ -344,7 +345,7 @@ def _builder_recipient(record: Mapping[str, Any]) -> str: recipient = builder + ":builder" if recipient not in SUPPORTED_RECIPIENTS: raise ContextError( - "the selected builder cannot consume private context in this release; choose Claude or Codex" + "the selected builder cannot consume private context in this release; choose Claude, Codex, or Devin" ) return recipient @@ -416,12 +417,12 @@ def feedback_session( record = context_session.validate(record) reviewer = participant_id(reviewer) if ( - reviewer not in {"claude", "codex"} + reviewer not in SUPPORTED_HOSTS or reviewer not in record["participants"] or reviewer == record["builder"] or PARTICIPANTS[reviewer].review_lane is None ): - raise ContextError("--reviewer must name a selected independent Claude or Codex reviewer") + raise ContextError("--reviewer must name a selected independent Claude, Codex, or Devin reviewer") if record["attachment_state"] != "published": raise ContextError("attach context and complete the independent review before reading feedback") recipient = _builder_recipient(record) diff --git a/src/code_mower/context_prepare.py b/src/code_mower/context_prepare.py index 37e42aa8..328e19b8 100644 --- a/src/code_mower/context_prepare.py +++ b/src/code_mower/context_prepare.py @@ -9,6 +9,7 @@ from . import context_packets, context_session, work_orders from .context_contract import ContextError, ContextRequest, _text +from .context_delivery import SUPPORTED_HOSTS from .context_store import ContextStore from .participants import PARTICIPANTS, participant_id @@ -202,8 +203,8 @@ def prepare( dependent_work="usable", next_action="Continue the ordinary workflow or configure an optional context connection.", ), 0 - if record["host"] not in {"claude", "codex"}: - raise ContextError("guided private context currently supports Claude and Codex hosts") + if record["host"] not in SUPPORTED_HOSTS: + raise ContextError("guided private context currently supports Claude, Codex, and Devin hosts") selected_builder = participant_id(builder or record["builder"] or record["host"]) if ( selected_builder not in record["participants"] diff --git a/src/code_mower/devin_work_orders.py b/src/code_mower/devin_work_orders.py index c4df0e7f..6f8ea473 100644 --- a/src/code_mower/devin_work_orders.py +++ b/src/code_mower/devin_work_orders.py @@ -11,8 +11,11 @@ import re from dataclasses import asdict, dataclass, field from pathlib import Path -from typing import Protocol +from typing import Callable, Protocol +from .context_contract import ContextError, ContextRequest +from .context_delivery import render_evidence +from .context_packets import load_authorized from .context_store import ContextStore from .devin_sessions import REPO, DevinClient from .provider_capabilities import resolve_transport @@ -33,6 +36,8 @@ "head_sha": {"type": "string", "pattern": "^[0-9a-f]{40}$"}, }, } +MAX_INPUT_BYTES = 65536 +CONTEXT_RECIPIENT = "devin:builder" SHA = re.compile(r"[0-9a-f]{40}\Z") LOGIN = re.compile(r"[A-Za-z0-9][A-Za-z0-9-]{0,38}(?:\[bot\])?\Z") @@ -131,6 +136,22 @@ def candidates(self, repository: str, branch: str, *, limit: int) -> Candidates: def read(self, repository: str, number: int) -> PullRequest: ... +def packet_context(store: ContextStore, name: str, handle: str, policy, *, repository: str, + work_item: str, backend=None) -> Callable[[], str]: + """Bind one authorized packet to the hosted builder before its PR exists. + + Each call performs a new online authorization for ``devin:builder`` and + renders the common evidence payload; nothing is cached or written. + """ + request = ContextRequest(repository, work_item, CONTEXT_RECIPIENT) + + def render() -> str: + packet = load_authorized(store, name, handle, policy, request, backend=backend) + return render_evidence(packet, handle) + + return render + + def _github_call(method, *args, **kwargs): try: return method(*args, **kwargs) @@ -174,6 +195,33 @@ def _prompt(order, round_number=0): + "\nApproved work order:\n" + order.body ) + @staticmethod + def _evidence(context): + """Render freshly reauthorized evidence for exactly one create/message input. + + ``context`` renders the authorized packet for ``devin:builder`` through + the ordinary online authorization path. It runs immediately before the + paid provider write, never in preview, and its text is never persisted. + """ + if context is None: + return None + try: + evidence = context() + except ContextError: + raise RemoteError("context_unavailable") from None + if not isinstance(evidence, str) or "Packet identity: " not in evidence: + raise RemoteError("context_unavailable") + return evidence + + @staticmethod + def _with_evidence(prose, evidence): + if evidence is None: + return prose + combined = prose + "\n" + evidence + if len(combined.encode()) > MAX_INPUT_BYTES: + raise RemoteError("context_budget_exceeded") + return combined + def _verify(self, order, claim, round_number): if (not isinstance(claim, dict) or set(claim) != set(COMPLETION_JSON_SCHEMA["required"]) or claim.get("schema") != COMPLETION_SCHEMA @@ -207,7 +255,8 @@ def _verify(self, order, claim, round_number): def run(self, command: str, order: WorkOrder, *, apply: bool = False, request: str = "", prose: str = "", reviewed_head: str = "", - acknowledge_delivered: bool = False) -> dict: + acknowledge_delivered: bool = False, + context: Callable[[], str] | None = None) -> dict: if command not in {"dispatch", "status", "collect", "clarify", "fix", "cancel"}: raise RemoteError("invalid_request") # Library equivalent of --apply: no reads, writes or provider calls in preview. @@ -234,8 +283,11 @@ def run(self, command: str, order: WorkOrder, *, apply: bool = False, branch_lock.write({"issue_key": key}) remote_command = command kwargs = {} + if context is not None and command not in {"dispatch", "fix", "clarify"}: + raise RemoteError("invalid_request") if command == "dispatch": - kwargs.update(prose=self._prompt(order), repo=order.repository, limit=order.acu_limit) + kwargs.update(prose=self._with_evidence(self._prompt(order), self._evidence(context)), + repo=order.repository, limit=order.acu_limit) elif command in {"fix", "clarify"}: if (not isinstance(request, str) or not request.strip() or len(request) > 128 or not isinstance(prose, str) or not prose.strip() or len(prose.encode()) > 48000): @@ -258,6 +310,7 @@ def run(self, command: str, order: WorkOrder, *, apply: bool = False, raise RemoteError("fix_requires_reviewed_head") if record["round"] >= 100: raise RemoteError("request_limit_reached") + evidence = self._evidence(context) # Before any local or remote mutation. record["round"] += 1 record.update(claim=None, evidence=None, message={ "request": request, "fingerprint": fingerprint, "pending": True}) @@ -265,12 +318,15 @@ def run(self, command: str, order: WorkOrder, *, apply: bool = False, locked.write(record) # Invalidate evidence before any remote mutation. elif previous["fingerprint"] != fingerprint: raise RemoteError("request_conflict") + else: + evidence = None if acknowledge_delivered else self._evidence(context) remote_command = "message" - kwargs.update(prose=(f"Continue the same work order, repository and sole writer branch. " - f"Completion round: {record['round']}. " - f"Reviewed head: {reviewed_head or 'none'}. " - "Before editing, stop if the branch head differs from the reviewed head " - "when supplied. Keep the original ACU cap and completion schema.\n" + prose)) + message = (f"Continue the same work order, repository and sole writer branch. " + f"Completion round: {record['round']}. " + f"Reviewed head: {reviewed_head or 'none'}. " + "Before editing, stop if the branch head differs from the reviewed head " + "when supplied. Keep the original ACU cap and completion schema.\n" + prose) + kwargs.update(prose=self._with_evidence(message, evidence)) result = self.remote.run(remote_command, key, apply=apply, request=request, acknowledge_delivered=acknowledge_delivered, **kwargs) if command in {"fix", "clarify"}: diff --git a/tests/test_context_delivery.py b/tests/test_context_delivery.py index b73fc9cc..ee5367b9 100644 --- a/tests/test_context_delivery.py +++ b/tests/test_context_delivery.py @@ -1,4 +1,4 @@ -"""One approved evidence payload for both hosts and independent recipients.""" +"""One approved evidence payload for every host and independent recipient.""" import json import hashlib @@ -9,7 +9,8 @@ from code_mower.context_connections import connect, disconnect from code_mower.context_contract import ContextError, ContextRequest, load_packet -from code_mower.context_delivery import attach, deliver, public_verdict, read_binding, render_evidence, save_feedback +from code_mower.context_delivery import (SUPPORTED_HOSTS, SUPPORTED_RECIPIENTS, attach, deliver, public_verdict, + read_binding, render_evidence, save_feedback) from code_mower.context_packets import fetch from code_mower.context_store import ContextStore from test_context_connections import MemoryVault @@ -25,7 +26,8 @@ def setUp(self): self.store = ContextStore(Path(tmp.name).resolve() / 'private', vault=MemoryVault()) self.backend = RetrievalBackend() self.head = 'a' * 40 - self.recipients = [f'{host}:{role}' for host in ('claude', 'codex') for role in ('orchestrator', 'builder', 'reviewer')] + self.recipients = [f'{host}:{role}' for host in ('claude', 'codex', 'devin') + for role in ('orchestrator', 'builder', 'reviewer')] connect(self.store, 'example', {'principal': 'one@example.invalid', 'workspace': 'example', 'repositories': ['owner/repo'], 'recipients': self.recipients}, backend=self.backend) self.spec = {'repository': 'owner/repo', 'work_item': 'EXAMPLE-1', 'recipient': 'codex:orchestrator', @@ -42,8 +44,12 @@ def delivery(self, current, recipient='claude:reviewer', **kwargs): return deliver(self.store, current['revision'], repository='owner/repo', pr=42, head=self.head, recipient=recipient, current=current, backend=self.backend, **kwargs) - def test_either_host_delivers_identical_evidence_to_builder_and_reviewer(self): - for host in ('claude', 'codex'): + def test_devin_roles_are_supported_recipients(self): + self.assertEqual(SUPPORTED_HOSTS, ('claude', 'codex', 'devin')) + self.assertEqual(SUPPORTED_RECIPIENTS, frozenset(self.recipients)) + + def test_every_host_delivers_identical_evidence_to_builder_and_reviewer(self): + for host in SUPPORTED_HOSTS: current = self.attach(host) received = [self.delivery(current, recipient) for recipient in self.recipients] self.assertEqual(len({item.text for item in received}), 1) @@ -71,6 +77,10 @@ def test_changed_input_recipient_revocation_and_disconnected_cache_are_rejected( self.delivery(invalid) with self.assertRaises(ContextError): self.delivery(current, 'unsupported:reviewer') + self.backend.wrong_identity = True + with self.assertRaises(ContextError): + self.delivery(current, 'devin:reviewer') + self.backend.wrong_identity = False self.backend.revoked = True with self.assertRaises(ContextError): self.delivery(current) @@ -79,6 +89,20 @@ def test_changed_input_recipient_revocation_and_disconnected_cache_are_rejected( with self.assertRaises(ContextError): self.delivery(current) + def test_connection_without_devin_recipients_never_delivers_to_devin(self): + disconnect(self.store, 'example', backend=self.backend) + narrowed = [recipient for recipient in self.recipients if not recipient.startswith('devin:')] + connect(self.store, 'example', {'principal': 'one@example.invalid', 'workspace': 'example', + 'repositories': ['owner/repo'], 'recipients': narrowed}, backend=self.backend) + self.result = fetch(self.store, 'example', self.spec, backend=self.backend) + with self.assertRaises(ContextError): + self.attach('devin') + current = self.attach() + for role in ('orchestrator', 'builder', 'reviewer'): + with self.assertRaises(ContextError): + self.delivery(current, f'devin:{role}') + self.assertTrue(self.delivery(current, 'codex:builder').text) + def test_retrieval_refresh_deletes_old_binding_and_feedback(self): current = self.attach() delivery = self.delivery(current) @@ -100,6 +124,14 @@ def test_public_verdict_has_only_metadata_and_never_model_authored_findings(self self.assertNotIn('one@example.invalid', body) self.assertIn(current['revision'], body) self.assertIn('Claude Audit: BLOCKED', body) + devin = self.delivery(current, 'devin:reviewer') + save_feedback(self.store, devin, 'devin', prose) + body = public_verdict(devin, provider='Devin', head=self.head, verdict='PASS', + counts=[0, 0, 0, 0], trailer='') + self.assertNotIn(prose, body) + self.assertIn('Devin Audit: PASS', body) + with self.assertRaises(ContextError): + save_feedback(self.store, devin, 'unsupported', prose) def test_local_repository_graph_uses_common_renderer_without_oauth_identity(self): from test_context_contract import NOW, connection, packet, policy diff --git a/tests/test_context_guided.py b/tests/test_context_guided.py index 0d6f8374..5dc7bb04 100644 --- a/tests/test_context_guided.py +++ b/tests/test_context_guided.py @@ -157,6 +157,75 @@ def test_builder_delivery_and_review_feedback_need_no_private_request_or_revisio context_session.read(self.associations, self.session["id"])["stage"], "reviewed" ) + def test_devin_host_builder_and_reviewer_use_the_common_packet(self): + self.session.update(host="devin", orchestrator="devin", + participants=[{"id": "claude"}, {"id": "devin"}]) + selected = context_session.create( + self.associations, {**self.session, "id": "e" * 32}, work_item="EXAMPLE-1", + policy=self.fixture.spec["policy"], + ) + self.session["id"] = "e" * 32 + self.record = context_session.update( + self.associations, self.session["id"], expected_generation=selected["generation"], + changes={"stage": "prepared", "builder": "devin", "query_mode": "work_item", + "request_hash": "c" * 64, "packet": self.fixture.result["packet_handle"], + "work_order": ".code-mower/work-orders/example.md"}, + ) + before = context_guided.deliver_session( + self.associations, self.fixture.store, self.record, repo_path=self.root, + backend=self.fixture.backend, + ) + self.assertIn("Private evidence", before) + report, code = self.attach() + self.assertEqual((report["status"], code), ("attached", 0)) + saved = context_session.read(self.associations, self.session["id"]) + current = read_binding(self.fixture.store, saved["revision"])["metadata"] + texts = { + deliver(self.fixture.store, saved["revision"], repository="owner/repo", pr=42, head=self.head, + recipient=recipient, current=current, backend=self.fixture.backend).text + for recipient in ("devin:orchestrator", "devin:builder", "devin:reviewer", "claude:reviewer") + } + self.assertEqual(len(texts), 1) + review = deliver(self.fixture.store, saved["revision"], repository="owner/repo", pr=42, + head=self.head, recipient="devin:reviewer", current=current, + backend=self.fixture.backend) + save_feedback(self.fixture.store, review, "devin", "Private Devin finding.") + with self.patches()[0], self.patches()[1], self.patches()[2]: + with self.assertRaises(ContextError): + context_guided.feedback_session( + self.associations, self.fixture.store, saved, repo_path=self.root, + reviewer="devin", backend=self.fixture.backend, + ) + for private in ("Private Devin finding.", "one@example.invalid", saved["packet"]): + self.assertNotIn(private, self.comments[0]["body"]) + + def test_devin_reviewer_feedback_reaches_a_different_builder(self): + self.session.update(participants=[{"id": "codex"}, {"id": "devin"}]) + selected = context_session.create( + self.associations, {**self.session, "id": "f" * 32}, work_item="EXAMPLE-1", + policy=self.fixture.spec["policy"], + ) + self.session["id"] = "f" * 32 + context_session.update( + self.associations, self.session["id"], expected_generation=selected["generation"], + changes={"stage": "prepared", "builder": "codex", "query_mode": "work_item", + "request_hash": "c" * 64, "packet": self.fixture.result["packet_handle"], + "work_order": ".code-mower/work-orders/example.md"}, + ) + self.attach() + saved = context_session.read(self.associations, self.session["id"]) + current = read_binding(self.fixture.store, saved["revision"])["metadata"] + review = deliver(self.fixture.store, saved["revision"], repository="owner/repo", pr=42, + head=self.head, recipient="devin:reviewer", current=current, + backend=self.fixture.backend) + save_feedback(self.fixture.store, review, "devin", "Private Devin finding.") + with self.patches()[0], self.patches()[1], self.patches()[2]: + feedback = context_guided.feedback_session( + self.associations, self.fixture.store, saved, repo_path=self.root, + reviewer="devin", backend=self.fixture.backend, + ) + self.assertEqual(feedback, "Private Devin finding.") + def test_lost_comment_response_reconciles_without_a_duplicate(self): self.fail_comment = RuntimeError("response lost") report, code = self.attach() diff --git a/tests/test_devin_work_orders.py b/tests/test_devin_work_orders.py index c5626308..f9ea0414 100644 --- a/tests/test_devin_work_orders.py +++ b/tests/test_devin_work_orders.py @@ -1,5 +1,6 @@ """Offline trusted-work-order delivery and hostile builder evidence fixtures.""" import json +import os import sys import tempfile import unittest @@ -9,12 +10,14 @@ sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "src")) +from code_mower.context_packets import fetch from code_mower.devin_sessions import DevinClient from code_mower.devin_work_orders import ( - COMPLETION_SCHEMA, Candidates, DevinWorkOrders, PullRequest, WorkOrder, + COMPLETION_SCHEMA, MAX_INPUT_BYTES, Candidates, DevinWorkOrders, PullRequest, WorkOrder, packet_context, ) from code_mower.remote_session import FakeProvider, RemoteError, RemoteSessions, _key from code_mower.work_orders import WORK_ORDER_SCHEMA +import test_context_delivery as fixtures CANARY = "PRIVATE_PROSE_SOURCE_DIFF_CREDENTIAL_RESULT" HEAD = "a" * 40 @@ -37,7 +40,7 @@ def read(self, repository, number): return self.reads.pop(0) if self.reads else self.pr -class DeliveryTests(unittest.TestCase): +class WorkOrderCase(unittest.TestCase): def setUp(self): self.tmp = tempfile.TemporaryDirectory() self.addCleanup(self.tmp.cleanup) @@ -68,6 +71,8 @@ def claim(self, **kwargs): def complete(self, **kwargs): self.provider.set_state(self.binding(), "complete", result=self.claim(**kwargs)) + +class DeliveryTests(WorkOrderCase): def test_preview_and_manifest_binding(self): self.assertTrue(self.service.run("dispatch", self.order)["apply_required"]) self.assertFalse((self.root / "builder").exists()) @@ -288,5 +293,132 @@ def runner(method, url, body, headers): self.assertTrue(calls[0][1].endswith("/consumption/daily/sessions/devin-one")) +CONTEXT_CANARY = "PRIVATE_CONTEXT_PACKET_CANARY_TEXT" + + +@unittest.skipUnless(os.name == "posix", "private packets need POSIX protections") +class ContextInjectionTests(WorkOrderCase): + """The hosted builder receives the common packet only inside create/message input.""" + + def setUp(self): + super().setUp() + self.fixture = fixtures.ContextDeliveryTests(methodName="runTest") + self.fixture.setUp() + self.addCleanup(self.fixture.doCleanups) + self.backend = self.fixture.backend + self.backend.result["result"]["results"][0]["text"] = CONTEXT_CANARY + self.result = fetch(self.fixture.store, "example", self.fixture.spec, backend=self.backend, refresh=True) + self.context = packet_context( + self.fixture.store, "example", self.result["packet_handle"], self.fixture.spec["policy"], + repository="owner/repo", work_item="EXAMPLE-1", backend=self.backend) + + def private_files(self): + return [path for path in self.root.rglob("*") if path.is_file()] + + def assert_not_persisted(self, *outputs): + serialized = json.dumps(outputs) + for private in (CONTEXT_CANARY, "Packet identity", "one@example.invalid", self.result["packet_handle"]): + self.assertNotIn(private, serialized) + for path in self.private_files(): + self.assertNotIn(CONTEXT_CANARY.encode(), path.read_bytes()) + + def test_preview_never_retrieves_context(self): + calls = [] + def spy(): + calls.append(1) + return self.context() + for command in ("dispatch", "clarify", "fix"): + output = self.service.run(command, self.order, context=spy, request="r", prose="p") + self.assertTrue(output["apply_required"]) + self.assertEqual(calls, []) + self.assertFalse((self.root / "builder").exists()) + + def test_create_and_message_receive_the_common_evidence_once_each(self): + outputs = [] + with patch.object(self.provider, "create", wraps=self.provider.create) as create, \ + patch.object(self.provider, "message", wraps=self.provider.message) as message: + outputs.append(self.run_order("dispatch", context=self.context)) + prompt = create.call_args.args[0] + self.assertIn(CANARY, prompt) + self.assertIn(CONTEXT_CANARY, prompt) + self.assertIn("Packet identity: " + self.result["packet_handle"], prompt) + self.assertNotIn("one@example.invalid", prompt) + self.assertEqual(prompt.split("\n").count("Private evidence for this work item. " + "Packet identity: " + self.result["packet_handle"]), 1) + outputs.append(self.run_order("dispatch", context=self.context)) + self.assertEqual(create.call_count, 1) + with self.assertRaisesRegex(RemoteError, "request_conflict"): + self.run_order("dispatch") # Changed create input, even by omission, never redispatches. + self.assertEqual(create.call_count, 1) + outputs.append(self.run_order("clarify", request="c-1", prose=CANARY, context=self.context)) + self.assertIn(CONTEXT_CANARY, message.call_args.args[1]) + self.assertIn(CANARY, message.call_args.args[1]) + outputs.append(self.run_order("clarify", request="c-2", prose=CANARY)) + self.assertNotIn(CONTEXT_CANARY, message.call_args.args[1]) + self.complete(round=2) + outputs.append(self.run_order("collect")) + outputs.append(self.run_order("fix", request="fix-1", prose=CANARY, reviewed_head=HEAD, + context=self.context)) + self.assertIn(CONTEXT_CANARY, message.call_args.args[1]) + self.assertEqual(message.call_count, 3) + self.assert_not_persisted(*outputs) + with self.remote.store.locked(_key(self.key)) as locked: + self.assertNotIn(CONTEXT_CANARY, json.dumps(locked.read())) + + def test_status_collect_and_cancel_reject_context(self): + self.run_order("dispatch") + for command in ("status", "collect", "cancel"): + with self.subTest(command=command), self.assertRaisesRegex(RemoteError, "invalid_request"): + self.run_order(command, request="cancel-1", context=self.context) + + def test_authorization_is_rechecked_and_fails_closed_without_a_provider_write(self): + cases = { + "wrong_identity": lambda: setattr(self.backend, "wrong_identity", True), + "revoked": lambda: setattr(self.backend, "revoked", True), + "wrong_repository": lambda: setattr( + self, "context", packet_context( + self.fixture.store, "example", self.result["packet_handle"], self.fixture.spec["policy"], + repository="other/repo", work_item="EXAMPLE-1", backend=self.backend)), + "changed_evidence": lambda: fetch(self.fixture.store, "example", self.fixture.spec, + backend=self.backend, refresh=True), + "garbage": lambda: setattr(self, "context", lambda: CANARY), + } + for name, arrange in cases.items(): + with self.subTest(case=name): + self.setUp() + arrange() + with patch.object(self.provider, "create", wraps=self.provider.create) as create: + with self.assertRaisesRegex(RemoteError, "context_unavailable"): + self.run_order("dispatch", context=self.context) + self.assertEqual(create.call_count, 0) + self.assertNotIn(CONTEXT_CANARY.encode(), b"".join(p.read_bytes() for p in self.private_files())) + self.setUp() + self.run_order("dispatch", context=self.context) + with patch.object(self.provider, "message", wraps=self.provider.message) as message: + with self.assertRaisesRegex(RemoteError, "context_unavailable"): + self.run_order("clarify", request="c-1", prose=CANARY, context=lambda: CANARY) + self.assertEqual(message.call_count, 0) + self.assertEqual(self.run_order("status")["round"], 0) # No local round was consumed. + self.assertEqual(self.run_order("clarify", request="c-1", prose=CANARY, + context=self.context)["round"], 1) + self.backend.revoked = True + with patch.object(self.provider, "message", wraps=self.provider.message) as message: + with self.assertRaisesRegex(RemoteError, "context_unavailable"): + self.run_order("clarify", request="c-2", prose=CANARY, context=self.context) + self.assertEqual(message.call_count, 0) + self.assertEqual(self.run_order("status")["round"], 1) + + def test_optional_context_is_omitted_only_by_explicit_choice(self): + with patch.object(self.provider, "create", wraps=self.provider.create) as create: + self.run_order("dispatch", context=None) + self.assertNotIn("Packet identity", create.call_args.args[0]) + + def test_combined_input_is_bounded(self): + with patch.object(self.provider, "create", wraps=self.provider.create) as create: + with self.assertRaisesRegex(RemoteError, "context_budget_exceeded"): + self.run_order("dispatch", context=lambda: "Packet identity: x\n" + "y" * MAX_INPUT_BYTES) + self.assertEqual(create.call_count, 0) + + if __name__ == "__main__": unittest.main() From bcf5aa18ce4d1317d343bce07a8d67f6a886cced Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Sat, 12 Sep 2026 11:26:55 +0000 Subject: [PATCH 2/7] Bind hosted Devin context to the work order, honor required policy, and declare context capability - Validate the combined follow-up plus context size before any local mutation. - packet_context derives its request from the WorkOrder; mismatched bindings fail closed. - Required policy fails closed; optional policy degrades explicitly and reports it. - public_verdict(provider=Devin) is always informational. - Devin transports declare context as local_runner/agent_handoff; doctor/session text updated. Co-Authored-By: bot_apk --- docs/context-delivery.md | 39 +++--- docs/sessions.md | 2 +- src/code_mower/context_delivery.py | 2 + src/code_mower/devin_work_orders.py | 92 +++++++++++---- src/code_mower/doctor_checks/providers.py | 2 +- src/code_mower/provider_capabilities.py | 4 +- .../provider_capabilities.schema.json | 10 +- src/code_mower/templates/providers.yml | 4 +- templates/providers.yml | 4 +- templates/providers/devin.yml | 2 +- templates/providers/devin_cli.yml | 2 +- tests/test_context_delivery.py | 7 ++ tests/test_devin_capabilities.py | 7 +- tests/test_devin_work_orders.py | 111 +++++++++++++++--- 14 files changed, 213 insertions(+), 75 deletions(-) diff --git a/docs/context-delivery.md b/docs/context-delivery.md index 84daf16f..2d06b073 100644 --- a/docs/context-delivery.md +++ b/docs/context-delivery.md @@ -78,22 +78,29 @@ logs, tracked files, PR descriptions, and shared artifacts. ### Hosted Devin builder A trusted hosted work order can carry the packet to `devin:builder` without a -local prompt file. The embedding binds one packet with -`devin_work_orders.packet_context(...)` and passes the resulting callable as -`context=` to `dispatch`, `clarify`, or `fix`. Immediately before each paid -create or message write, and never in preview, the callable performs a new -online authorization for `devin:builder` and renders the common evidence -payload. That text is appended only to the provider input; the local work-order -record and remote-session record keep only their existing digests, and status, -collect, and cancel reject a context argument. - -Wrong account, revoked or expired authorization, a repository or recipient -outside the connection, a refreshed or invalidated packet, or a payload without -the packet identity fails closed as `context_unavailable` before any local -round or provider write. Omitting `context=` is the only way to degrade to a -code-only work order, and the remote session's dispatch fingerprint prevents a -later dispatch from silently replaying with different input. The combined input -is bounded at 64 KiB (`context_budget_exceeded`). +local prompt file. The embedding binds one packet to the exact work order with +`devin_work_orders.packet_context(store, name, handle, policy, order=order)` and +passes the resulting `PacketContext` as `context=` to `dispatch`, `clarify`, or +`fix`. The packet request is derived from the order's repository and issue +number, never from caller-supplied identity, and a context bound to another +order (or a bare callable) is rejected as `context_binding_mismatch`. +Immediately before each paid create or message write, and never in preview, the +context performs a new online authorization for `devin:builder` and renders the +common evidence payload. That text is appended only to the provider input; the +local work-order record and remote-session record keep only their existing +digests, and status, collect, and cancel reject a context argument. Each +outcome reports `context` as `delivered`, `degraded`, or `omitted`. + +The trusted policy's `required` flag decides what happens when the packet cannot +be reauthorized. Wrong account, revoked or expired authorization, a packet for +another repository or work item, a refreshed or invalidated packet, or a payload +without the packet identity fails a required context closed as +`context_unavailable` before any local round or provider write; the work order +pauses with its prior state intact. Only an explicitly optional policy degrades +to a code-only input, and that degradation is reported. Omitting `context=` +sends a code-only work order, and the remote session's input fingerprint +prevents a later replay with different input. The combined input, validated +before any local mutation, is bounded at 64 KiB (`context_budget_exceeded`). ## Attach evidence to independent review diff --git a/docs/sessions.md b/docs/sessions.md index 60806a8d..435f4476 100644 --- a/docs/sessions.md +++ b/docs/sessions.md @@ -280,7 +280,7 @@ readiness, and explicit gaps. The modes describe maintained Code Mower paths: | Review | Local runner | Evidence only | | Message | Unavailable | Unavailable | | Cancel | Unavailable | Unavailable | -| Authorized context delivery | Unavailable | Unavailable | +| Authorized context delivery | Local runner (attach to the packet) | Agent handoff (authorized packet in hosted builder input) | | Structured results | Local runner | Release campaign only | These declarations describe the current integration; they do not launch a diff --git a/src/code_mower/context_delivery.py b/src/code_mower/context_delivery.py index e53bc5e7..fc8eab5b 100644 --- a/src/code_mower/context_delivery.py +++ b/src/code_mower/context_delivery.py @@ -257,6 +257,8 @@ def public_verdict(delivery, *, provider, head, verdict, counts, trailer, raise ContextError("unsupported context review metadata") if len(counts) != 4 or any(type(value) is not int or not 0 <= value <= 1000 for value in counts): raise ContextError("invalid context review counts") + if provider == "Devin": + merge_authority = False # Devin review is informational in every lane. metadata = delivery.metadata if isinstance(delivery, Delivery) else delivery from .provider_runners.comments import format_audit_comment_header header = format_audit_comment_header(provider_name=provider, head_sha=head, diff --git a/src/code_mower/devin_work_orders.py b/src/code_mower/devin_work_orders.py index 6f8ea473..acf399e4 100644 --- a/src/code_mower/devin_work_orders.py +++ b/src/code_mower/devin_work_orders.py @@ -13,7 +13,7 @@ from pathlib import Path from typing import Callable, Protocol -from .context_contract import ContextError, ContextRequest +from .context_contract import ContextError, ContextRequest, normalize_policy from .context_delivery import render_evidence from .context_packets import load_authorized from .context_store import ContextStore @@ -136,20 +136,42 @@ def candidates(self, repository: str, branch: str, *, limit: int) -> Candidates: def read(self, repository: str, number: int) -> PullRequest: ... -def packet_context(store: ContextStore, name: str, handle: str, policy, *, repository: str, - work_item: str, backend=None) -> Callable[[], str]: +@dataclass(frozen=True, repr=False) +class PacketContext: + """Authorized packet bound to exactly one work order's repository and issue. + + ``required`` mirrors the trusted context policy: required context fails the + paid write closed when it cannot be reauthorized; optional context degrades + to a code-only input and reports ``degraded`` in the returned metadata. + """ + repository: str + issue: int + required: bool + render: Callable[[], str] = field(repr=False) + + +def packet_context(store: ContextStore, name: str, handle: str, policy, *, order: WorkOrder, + backend=None) -> PacketContext: """Bind one authorized packet to the hosted builder before its PR exists. - Each call performs a new online authorization for ``devin:builder`` and - renders the common evidence payload; nothing is cached or written. + The packet request is derived from the work order (repository and issue + number as the work item), never from caller-supplied identity. Each render + performs a new online authorization for ``devin:builder`` and returns the + common evidence payload; nothing is cached or written. """ - request = ContextRequest(repository, work_item, CONTEXT_RECIPIENT) + try: + normalized = normalize_policy(policy) + except ContextError: + raise RemoteError("invalid_request") from None + if normalized is None: + raise RemoteError("invalid_request") + request = ContextRequest(order.repository, str(order.issue), CONTEXT_RECIPIENT) def render() -> str: packet = load_authorized(store, name, handle, policy, request, backend=backend) return render_evidence(packet, handle) - return render + return PacketContext(order.repository, order.issue, normalized["required"], render) def _github_call(method, *args, **kwargs): @@ -196,22 +218,39 @@ def _prompt(order, round_number=0): ) @staticmethod - def _evidence(context): + def _evidence(order, context): """Render freshly reauthorized evidence for exactly one create/message input. ``context`` renders the authorized packet for ``devin:builder`` through the ordinary online authorization path. It runs immediately before the paid provider write, never in preview, and its text is never persisted. + Returns ``(evidence, state)`` where state is ``delivered``, ``degraded`` + (optional context unavailable) or ``omitted`` (no context supplied). """ if context is None: - return None + return None, "omitted" + if (not isinstance(context, PacketContext) or context.repository != order.repository + or context.issue != order.issue): + raise RemoteError("context_binding_mismatch") try: - evidence = context() + evidence = context.render() + if not isinstance(evidence, str) or "Packet identity: " not in evidence: + raise ContextError("malformed context evidence") except ContextError: - raise RemoteError("context_unavailable") from None - if not isinstance(evidence, str) or "Packet identity: " not in evidence: - raise RemoteError("context_unavailable") - return evidence + if context.required: + raise RemoteError("context_unavailable") from None + return None, "degraded" + return evidence, "delivered" + + @classmethod + def _message(cls, round_number, reviewed_head, prose, evidence): + return cls._with_evidence( + f"Continue the same work order, repository and sole writer branch. " + f"Completion round: {round_number}. " + f"Reviewed head: {reviewed_head or 'none'}. " + "Before editing, stop if the branch head differs from the reviewed head " + "when supplied. Keep the original ACU cap and completion schema.\n" + prose, + evidence) @staticmethod def _with_evidence(prose, evidence): @@ -256,7 +295,7 @@ def _verify(self, order, claim, round_number): def run(self, command: str, order: WorkOrder, *, apply: bool = False, request: str = "", prose: str = "", reviewed_head: str = "", acknowledge_delivered: bool = False, - context: Callable[[], str] | None = None) -> dict: + context: PacketContext | None = None) -> dict: if command not in {"dispatch", "status", "collect", "clarify", "fix", "cancel"}: raise RemoteError("invalid_request") # Library equivalent of --apply: no reads, writes or provider calls in preview. @@ -283,10 +322,12 @@ def run(self, command: str, order: WorkOrder, *, apply: bool = False, branch_lock.write({"issue_key": key}) remote_command = command kwargs = {} + context_state = None if context is not None and command not in {"dispatch", "fix", "clarify"}: raise RemoteError("invalid_request") if command == "dispatch": - kwargs.update(prose=self._with_evidence(self._prompt(order), self._evidence(context)), + evidence, context_state = self._evidence(order, context) + kwargs.update(prose=self._with_evidence(self._prompt(order), evidence), repo=order.repository, limit=order.acu_limit) elif command in {"fix", "clarify"}: if (not isinstance(request, str) or not request.strip() or len(request) > 128 @@ -310,7 +351,9 @@ def run(self, command: str, order: WorkOrder, *, apply: bool = False, raise RemoteError("fix_requires_reviewed_head") if record["round"] >= 100: raise RemoteError("request_limit_reached") - evidence = self._evidence(context) # Before any local or remote mutation. + # Render and bound the full input before any local or remote mutation. + evidence, context_state = self._evidence(order, context) + message = self._message(record["round"] + 1, reviewed_head, prose, evidence) record["round"] += 1 record.update(claim=None, evidence=None, message={ "request": request, "fingerprint": fingerprint, "pending": True}) @@ -319,14 +362,13 @@ def run(self, command: str, order: WorkOrder, *, apply: bool = False, elif previous["fingerprint"] != fingerprint: raise RemoteError("request_conflict") else: - evidence = None if acknowledge_delivered else self._evidence(context) + if acknowledge_delivered: + evidence, context_state = None, "omitted" + else: + evidence, context_state = self._evidence(order, context) + message = self._message(record["round"], reviewed_head, prose, evidence) remote_command = "message" - message = (f"Continue the same work order, repository and sole writer branch. " - f"Completion round: {record['round']}. " - f"Reviewed head: {reviewed_head or 'none'}. " - "Before editing, stop if the branch head differs from the reviewed head " - "when supplied. Keep the original ACU cap and completion schema.\n" + prose) - kwargs.update(prose=self._with_evidence(message, evidence)) + kwargs.update(prose=message) result = self.remote.run(remote_command, key, apply=apply, request=request, acknowledge_delivered=acknowledge_delivered, **kwargs) if command in {"fix", "clarify"}: @@ -354,4 +396,4 @@ def run(self, command: str, order: WorkOrder, *, apply: bool = False, "session": result, "round": record["round"], "acu_limit": order.acu_limit, "observed_acu": record["observed_acu"], "verified_pr": record["evidence"] if command == "collect" else None, - "merge_authority": False} + "context": context_state, "merge_authority": False} diff --git a/src/code_mower/doctor_checks/providers.py b/src/code_mower/doctor_checks/providers.py index 55995fcd..5313d445 100644 --- a/src/code_mower/doctor_checks/providers.py +++ b/src/code_mower/doctor_checks/providers.py @@ -203,7 +203,7 @@ def check_lane_runtime( name="provider.capabilities", status=STATUS_WARN, lane=lane_id, message=f"{transport.product} via {transport.transport}: unavailable capabilities: " + ", ".join(transport.brief()["capability_gaps"]), detail=transport.brief(), - remediation="Use only declared capability modes; session messaging, cancellation, and authorized context delivery are unavailable. Selection does not grant merge authority.", + remediation="Use only declared capability modes; session messaging and cancellation are unavailable; authorized context is delivered only through the declared mode. Selection does not grant merge authority.", )) driver = str(lane.get("driver", "")) skip_local_cli_runtime = ( diff --git a/src/code_mower/provider_capabilities.py b/src/code_mower/provider_capabilities.py index 14991262..79bf598a 100644 --- a/src/code_mower/provider_capabilities.py +++ b/src/code_mower/provider_capabilities.py @@ -59,7 +59,7 @@ def brief(self) -> dict[str, Any]: "devin", "devin_cli", "local_cli", "devin_cli", Capabilities( coordinate="agent_handoff", build="local_runner", review="local_runner", - message="unavailable", cancel="unavailable", context="unavailable", + message="unavailable", cancel="unavailable", context="local_runner", structured_results="local_runner", ), ), @@ -67,7 +67,7 @@ def brief(self) -> dict[str, Any]: "devin", "devin_api_v3", "hosted_bridge", "devin", Capabilities( coordinate="unavailable", build="agent_handoff", review="evidence_only", - message="unavailable", cancel="unavailable", context="unavailable", + message="unavailable", cancel="unavailable", context="agent_handoff", structured_results="campaign_only", ), ), diff --git a/src/code_mower/provider_capabilities.schema.json b/src/code_mower/provider_capabilities.schema.json index 92b7a487..90dcca97 100644 --- a/src/code_mower/provider_capabilities.schema.json +++ b/src/code_mower/provider_capabilities.schema.json @@ -31,7 +31,7 @@ "review": "local_runner", "message": "unavailable", "cancel": "unavailable", - "context": "unavailable", + "context": "local_runner", "structured_results": "local_runner" } }, @@ -41,8 +41,7 @@ "capability_gaps": { "const": [ "message", - "cancel", - "context" + "cancel" ] } } @@ -75,7 +74,7 @@ "review": "evidence_only", "message": "unavailable", "cancel": "unavailable", - "context": "unavailable", + "context": "agent_handoff", "structured_results": "campaign_only" } }, @@ -86,8 +85,7 @@ "const": [ "coordinate", "message", - "cancel", - "context" + "cancel" ] } } diff --git a/src/code_mower/templates/providers.yml b/src/code_mower/templates/providers.yml index c69113bc..b85ec3a9 100644 --- a/src/code_mower/templates/providers.yml +++ b/src/code_mower/templates/providers.yml @@ -268,7 +268,7 @@ provider_templates: review: "evidence_only" message: "unavailable" cancel: "unavailable" - context: "unavailable" + context: "agent_handoff" structured_results: "campaign_only" driver: "hosted_bridge" type: "audit" @@ -305,7 +305,7 @@ provider_templates: review: "local_runner" message: "unavailable" cancel: "unavailable" - context: "unavailable" + context: "local_runner" structured_results: "local_runner" driver: "local_cli" type: "audit" diff --git a/templates/providers.yml b/templates/providers.yml index c69113bc..b85ec3a9 100644 --- a/templates/providers.yml +++ b/templates/providers.yml @@ -268,7 +268,7 @@ provider_templates: review: "evidence_only" message: "unavailable" cancel: "unavailable" - context: "unavailable" + context: "agent_handoff" structured_results: "campaign_only" driver: "hosted_bridge" type: "audit" @@ -305,7 +305,7 @@ provider_templates: review: "local_runner" message: "unavailable" cancel: "unavailable" - context: "unavailable" + context: "local_runner" structured_results: "local_runner" driver: "local_cli" type: "audit" diff --git a/templates/providers/devin.yml b/templates/providers/devin.yml index eb8ceda1..8e0d18ad 100644 --- a/templates/providers/devin.yml +++ b/templates/providers/devin.yml @@ -8,7 +8,7 @@ devin: review: "evidence_only" message: "unavailable" cancel: "unavailable" - context: "unavailable" + context: "agent_handoff" structured_results: "campaign_only" driver: "hosted_bridge" type: "audit" diff --git a/templates/providers/devin_cli.yml b/templates/providers/devin_cli.yml index 218ff266..54336cb3 100644 --- a/templates/providers/devin_cli.yml +++ b/templates/providers/devin_cli.yml @@ -8,7 +8,7 @@ devin_cli: review: "local_runner" message: "unavailable" cancel: "unavailable" - context: "unavailable" + context: "local_runner" structured_results: "local_runner" driver: "local_cli" type: "audit" diff --git a/tests/test_context_delivery.py b/tests/test_context_delivery.py index ee5367b9..2fa6d446 100644 --- a/tests/test_context_delivery.py +++ b/tests/test_context_delivery.py @@ -130,6 +130,13 @@ def test_public_verdict_has_only_metadata_and_never_model_authored_findings(self counts=[0, 0, 0, 0], trailer='') self.assertNotIn(prose, body) self.assertIn('Devin Audit: PASS', body) + self.assertIn('## Devin audit (informational only)', body) + forced = public_verdict(devin, provider='Devin', head=self.head, verdict='PASS', counts=[0, 0, 0, 0], + trailer='', merge_authority=True) + self.assertIn('## Devin audit (informational only)', forced) + self.assertIn('## Claude audit (merge-authority lane)', public_verdict( + delivery, provider='Claude', head=self.head, verdict='PASS', counts=[0, 0, 0, 0], + trailer='')) with self.assertRaises(ContextError): save_feedback(self.store, devin, 'unsupported', prose) diff --git a/tests/test_devin_capabilities.py b/tests/test_devin_capabilities.py index 2a15d328..7f3f26fd 100644 --- a/tests/test_devin_capabilities.py +++ b/tests/test_devin_capabilities.py @@ -165,8 +165,10 @@ def test_session_briefs_preserve_transport_aliases_and_capability_gaps(self): self.assertTrue(member["reviewer"]["informational"]) rendered = session.render_session(brief) self.assertIn(expected, rendered) - for capability in ("message", "cancel", "context"): + for capability in ("message", "cancel"): self.assertIn(capability + "=unavailable", rendered) + self.assertIn("context=" + TRANSPORTS[expected].capabilities.context, rendered) + self.assertNotIn("context=unavailable", rendered) def test_hosted_transport_cannot_coordinate(self): with self.assertRaisesRegex(config.ConfigError, "cannot coordinate"): @@ -220,7 +222,8 @@ def test_doctor_reports_static_gaps_without_a_live_probe(self): checks = check_lane_runtime(transport.review_lane, participants.reference_review_config(transport.review_lane), probe_runtime=False, http_timeout=1, adoption_posture="orchestrator-only") check = next(check for check in checks if check.name == "provider.capabilities") self.assertEqual(check.detail, transport.brief()) - self.assertIn("message, cancel, context", check.message) + self.assertIn("message, cancel", check.message) + self.assertNotIn("context", check.message) self.assertEqual(check.status, "warn") def test_shipped_schema_matches_each_complete_transport_contract(self): diff --git a/tests/test_devin_work_orders.py b/tests/test_devin_work_orders.py index f9ea0414..6fd7f6f6 100644 --- a/tests/test_devin_work_orders.py +++ b/tests/test_devin_work_orders.py @@ -10,6 +10,7 @@ sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "src")) +from code_mower.context_contract import ContextError from code_mower.context_packets import fetch from code_mower.devin_sessions import DevinClient from code_mower.devin_work_orders import ( @@ -307,10 +308,14 @@ def setUp(self): self.addCleanup(self.fixture.doCleanups) self.backend = self.fixture.backend self.backend.result["result"]["results"][0]["text"] = CONTEXT_CANARY - self.result = fetch(self.fixture.store, "example", self.fixture.spec, backend=self.backend, refresh=True) - self.context = packet_context( - self.fixture.store, "example", self.result["packet_handle"], self.fixture.spec["policy"], - repository="owner/repo", work_item="EXAMPLE-1", backend=self.backend) + self.policy = self.fixture.spec["policy"] + self.spec = {**self.fixture.spec, "work_item": str(self.order.issue)} + self.result = fetch(self.fixture.store, "example", self.spec, backend=self.backend, refresh=True) + self.context = self.bind() + + def bind(self, order=None, policy=None, handle=None): + return packet_context(self.fixture.store, "example", handle or self.result["packet_handle"], + policy or self.policy, order=order or self.order, backend=self.backend) def private_files(self): return [path for path in self.root.rglob("*") if path.is_file()] @@ -322,13 +327,24 @@ def assert_not_persisted(self, *outputs): for path in self.private_files(): self.assertNotIn(CONTEXT_CANARY.encode(), path.read_bytes()) + def test_packet_context_binds_the_work_order_identity_and_policy(self): + self.assertEqual((self.context.repository, self.context.issue, self.context.required), + (self.order.repository, self.order.issue, True)) + self.assertFalse(self.bind(policy={**self.policy, "required": False}).required) + self.assertNotIn(CONTEXT_CANARY, repr(self.context)) + for policy in (None, {}, {**self.policy, "required": "yes"}): + with self.subTest(policy=policy), self.assertRaisesRegex(RemoteError, "invalid_request"): + packet_context(self.fixture.store, "example", self.result["packet_handle"], policy, + order=self.order, backend=self.backend) + def test_preview_never_retrieves_context(self): calls = [] def spy(): calls.append(1) - return self.context() + return self.context.render() + context = replace(self.context, render=spy) for command in ("dispatch", "clarify", "fix"): - output = self.service.run(command, self.order, context=spy, request="r", prose="p") + output = self.service.run(command, self.order, context=context, request="r", prose="p") self.assertTrue(output["apply_required"]) self.assertEqual(calls, []) self.assertFalse((self.root / "builder").exists()) @@ -338,6 +354,7 @@ def test_create_and_message_receive_the_common_evidence_once_each(self): with patch.object(self.provider, "create", wraps=self.provider.create) as create, \ patch.object(self.provider, "message", wraps=self.provider.message) as message: outputs.append(self.run_order("dispatch", context=self.context)) + self.assertEqual(outputs[-1]["context"], "delivered") prompt = create.call_args.args[0] self.assertIn(CANARY, prompt) self.assertIn(CONTEXT_CANARY, prompt) @@ -351,12 +368,15 @@ def test_create_and_message_receive_the_common_evidence_once_each(self): self.run_order("dispatch") # Changed create input, even by omission, never redispatches. self.assertEqual(create.call_count, 1) outputs.append(self.run_order("clarify", request="c-1", prose=CANARY, context=self.context)) + self.assertEqual(outputs[-1]["context"], "delivered") self.assertIn(CONTEXT_CANARY, message.call_args.args[1]) self.assertIn(CANARY, message.call_args.args[1]) outputs.append(self.run_order("clarify", request="c-2", prose=CANARY)) + self.assertEqual(outputs[-1]["context"], "omitted") self.assertNotIn(CONTEXT_CANARY, message.call_args.args[1]) self.complete(round=2) outputs.append(self.run_order("collect")) + self.assertIsNone(outputs[-1]["context"]) outputs.append(self.run_order("fix", request="fix-1", prose=CANARY, reviewed_head=HEAD, context=self.context)) self.assertIn(CONTEXT_CANARY, message.call_args.args[1]) @@ -371,17 +391,37 @@ def test_status_collect_and_cancel_reject_context(self): with self.subTest(command=command), self.assertRaisesRegex(RemoteError, "invalid_request"): self.run_order(command, request="cancel-1", context=self.context) + def test_context_must_be_bound_to_this_work_order(self): + other_issue = replace(self.order, issue=908) + other_repo = replace(self.order, repository="other/repo") + cases = { + "other_issue": self.bind(order=other_issue), + "other_repository": self.bind(order=other_repo), + "unbound_callable": self.context.render, + "forged_binding": replace(self.bind(order=other_issue), repository="other/repo"), + } + for name, context in cases.items(): + with self.subTest(case=name), patch.object(self.provider, "create", wraps=self.provider.create) as create: + with self.assertRaisesRegex(RemoteError, "context_binding_mismatch"): + self.run_order("dispatch", context=context) + self.assertEqual(create.call_count, 0) + self.run_order("dispatch", context=self.context) + with patch.object(self.provider, "message", wraps=self.provider.message) as message: + with self.assertRaisesRegex(RemoteError, "context_binding_mismatch"): + self.run_order("clarify", request="c-1", prose=CANARY, context=self.bind(order=other_issue)) + self.assertEqual(message.call_count, 0) + self.assertEqual(self.run_order("status")["round"], 0) + def test_authorization_is_rechecked_and_fails_closed_without_a_provider_write(self): cases = { "wrong_identity": lambda: setattr(self.backend, "wrong_identity", True), "revoked": lambda: setattr(self.backend, "revoked", True), - "wrong_repository": lambda: setattr( - self, "context", packet_context( - self.fixture.store, "example", self.result["packet_handle"], self.fixture.spec["policy"], - repository="other/repo", work_item="EXAMPLE-1", backend=self.backend)), - "changed_evidence": lambda: fetch(self.fixture.store, "example", self.fixture.spec, + "packet_for_another_work_item": lambda: setattr(self, "context", self.bind( + handle=fetch(self.fixture.store, "example", self.fixture.spec, + backend=self.backend, refresh=True)["packet_handle"])), + "changed_evidence": lambda: fetch(self.fixture.store, "example", self.spec, backend=self.backend, refresh=True), - "garbage": lambda: setattr(self, "context", lambda: CANARY), + "garbage": lambda: setattr(self, "context", replace(self.context, render=lambda: CANARY)), } for name, arrange in cases.items(): with self.subTest(case=name): @@ -396,7 +436,8 @@ def test_authorization_is_rechecked_and_fails_closed_without_a_provider_write(se self.run_order("dispatch", context=self.context) with patch.object(self.provider, "message", wraps=self.provider.message) as message: with self.assertRaisesRegex(RemoteError, "context_unavailable"): - self.run_order("clarify", request="c-1", prose=CANARY, context=lambda: CANARY) + self.run_order("clarify", request="c-1", prose=CANARY, + context=replace(self.context, render=lambda: CANARY)) self.assertEqual(message.call_count, 0) self.assertEqual(self.run_order("status")["round"], 0) # No local round was consumed. self.assertEqual(self.run_order("clarify", request="c-1", prose=CANARY, @@ -408,16 +449,54 @@ def test_authorization_is_rechecked_and_fails_closed_without_a_provider_write(se self.assertEqual(message.call_count, 0) self.assertEqual(self.run_order("status")["round"], 1) - def test_optional_context_is_omitted_only_by_explicit_choice(self): + def test_optional_context_degrades_explicitly_and_required_context_fails_closed(self): + optional = self.bind(policy={**self.policy, "required": False}) with patch.object(self.provider, "create", wraps=self.provider.create) as create: - self.run_order("dispatch", context=None) + output = self.run_order("dispatch", context=None) + self.assertEqual(output["context"], "omitted") self.assertNotIn("Packet identity", create.call_args.args[0]) + def fail(): + raise ContextError("context search unavailable") + unavailable = {"render": fail} + with patch.object(self.provider, "message", wraps=self.provider.message) as message: + with self.assertRaisesRegex(RemoteError, "context_unavailable"): + self.run_order("clarify", request="c-1", prose=CANARY, context=replace(self.context, **unavailable)) + self.assertEqual(message.call_count, 0) + output = self.run_order("clarify", request="c-1", prose=CANARY, context=replace(optional, **unavailable)) + self.assertEqual((output["context"], output["round"]), ("degraded", 1)) + self.assertNotIn(CONTEXT_CANARY, message.call_args.args[1]) + self.assertIn(CANARY, message.call_args.args[1]) + with patch.object(self.provider, "message", wraps=self.provider.message) as message: + with self.assertRaisesRegex(RemoteError, "request_conflict"): + self.run_order("clarify", request="c-1", prose=CANARY, context=optional) # Input changed. + output = self.run_order("clarify", request="c-1", prose=CANARY, context=replace(optional, **unavailable)) + self.assertEqual((output["context"], output["round"]), ("degraded", 1)) + output = self.run_order("clarify", request="c-2", prose=CANARY, context=optional) + self.assertEqual((output["context"], output["round"]), ("delivered", 2)) + self.assertIn(CONTEXT_CANARY, message.call_args.args[1]) + self.assert_not_persisted(output) def test_combined_input_is_bounded(self): + oversized = replace(self.context, render=lambda: "Packet identity: x\n" + "y" * MAX_INPUT_BYTES) with patch.object(self.provider, "create", wraps=self.provider.create) as create: with self.assertRaisesRegex(RemoteError, "context_budget_exceeded"): - self.run_order("dispatch", context=lambda: "Packet identity: x\n" + "y" * MAX_INPUT_BYTES) + self.run_order("dispatch", context=oversized) self.assertEqual(create.call_count, 0) + self.run_order("dispatch", context=self.context) + self.complete() + self.run_order("collect") + with patch.object(self.provider, "message", wraps=self.provider.message) as message: + with self.assertRaisesRegex(RemoteError, "context_budget_exceeded"): + self.run_order("fix", request="fix-1", prose=CANARY, reviewed_head=HEAD, context=oversized) + self.assertEqual(message.call_count, 0) + self.assertEqual(self.run_order("status")["round"], 0) # Oversized input leaves the record unchanged. + with self.service.store.locked(self.key) as locked: + record = locked.read() + self.assertIsNone(record["message"]) + self.assertIsNotNone(record["claim"]) + self.assertEqual(record["requests"], []) + self.assertEqual(self.run_order("fix", request="fix-1", prose=CANARY, reviewed_head=HEAD, + context=self.context)["round"], 1) if __name__ == "__main__": From a6305bdb67e5646cca6e11c44f9a663a018814a7 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Sat, 12 Sep 2026 11:45:11 +0000 Subject: [PATCH 3/7] Bind hosted Devin context policy and packet identity to the work order - Narrow devin_cli context capability back to unavailable (not wired into the local reviewer). - Render/validate initial context before the work-order record or branch reservation is written. - PacketContext carries only a render closure; the loaded packet's own binding is checked against the WorkOrder. - WorkOrder.context_policy (none/optional/required) decides the outcome, including context=None. - Persist and report the safe context-state enum for dispatch and each message; acknowledge preserves it. Co-Authored-By: bot_apk --- docs/context-delivery.md | 51 +++--- docs/sessions.md | 2 +- src/code_mower/devin_work_orders.py | 99 +++++++----- src/code_mower/doctor_checks/providers.py | 2 +- src/code_mower/provider_capabilities.py | 2 +- .../provider_capabilities.schema.json | 5 +- src/code_mower/templates/providers.yml | 2 +- templates/providers.yml | 2 +- templates/providers/devin_cli.yml | 2 +- tests/test_devin_capabilities.py | 4 +- tests/test_devin_work_orders.py | 153 +++++++++++------- 11 files changed, 196 insertions(+), 128 deletions(-) diff --git a/docs/context-delivery.md b/docs/context-delivery.md index 2d06b073..ac55ff71 100644 --- a/docs/context-delivery.md +++ b/docs/context-delivery.md @@ -78,29 +78,36 @@ logs, tracked files, PR descriptions, and shared artifacts. ### Hosted Devin builder A trusted hosted work order can carry the packet to `devin:builder` without a -local prompt file. The embedding binds one packet to the exact work order with -`devin_work_orders.packet_context(store, name, handle, policy, order=order)` and -passes the resulting `PacketContext` as `context=` to `dispatch`, `clarify`, or -`fix`. The packet request is derived from the order's repository and issue -number, never from caller-supplied identity, and a context bound to another -order (or a bare callable) is rejected as `context_binding_mismatch`. +local prompt file. The trusted context decision is part of the work order: +`WorkOrder.context_policy` is `none`, `optional`, or `required`, comes from +dispatcher policy, and is included in the durable binding. The embedding binds +one packet with `devin_work_orders.packet_context(store, name, handle, policy, +order=order)`, which derives the packet request from the order's repository and +issue number, requires the trusted policy's `required` flag to agree with the +order, and returns a `PacketContext` holding only a render closure. That value +is passed as `context=` to `dispatch`, `clarify`, or `fix`. + Immediately before each paid create or message write, and never in preview, the -context performs a new online authorization for `devin:builder` and renders the -common evidence payload. That text is appended only to the provider input; the -local work-order record and remote-session record keep only their existing -digests, and status, collect, and cancel reject a context argument. Each -outcome reports `context` as `delivered`, `degraded`, or `omitted`. - -The trusted policy's `required` flag decides what happens when the packet cannot -be reauthorized. Wrong account, revoked or expired authorization, a packet for -another repository or work item, a refreshed or invalidated packet, or a payload -without the packet identity fails a required context closed as -`context_unavailable` before any local round or provider write; the work order -pauses with its prior state intact. Only an explicitly optional policy degrades -to a code-only input, and that degradation is reported. Omitting `context=` -sends a code-only work order, and the remote session's input fingerprint -prevents a later replay with different input. The combined input, validated -before any local mutation, is bounded at 64 KiB (`context_budget_exceeded`). +context performs a new online authorization for `devin:builder`, and the loaded +packet's own repository and work-item binding is compared with the order; a +packet for another ticket, a relabelled wrapper, a bare callable, or any context +on a `none` order fails as `context_binding_mismatch`. Rendering and the 64 KiB +combined-size check (`context_budget_exceeded`) happen before the work-order +record, branch reservation, or a new round is written, so a rejected input +leaves no undispatched reservation and consumes no round. Evidence text is +appended only to the provider input; records keep only digests plus the safe +state enum, and status, collect, and cancel reject a context argument. + +A `required` order fails closed as `context_unavailable` when no packet is +supplied or it cannot be reauthorized (wrong account, revoked or expired +authorization, refreshed or invalidated packet, payload without the packet +identity); the work order pauses with its prior state intact. Only an `optional` +order degrades to a code-only input (`degraded`) or runs without a packet +(`omitted`), and the remote session's input fingerprint prevents a later replay +with different input. The state chosen for the create input and for each +message intent is persisted and reported by every command as +`context: {policy, dispatch, message}`; replay, acknowledgement, status, and +collect return the saved state rather than recomputing it. ## Attach evidence to independent review diff --git a/docs/sessions.md b/docs/sessions.md index 435f4476..4aaca3ba 100644 --- a/docs/sessions.md +++ b/docs/sessions.md @@ -280,7 +280,7 @@ readiness, and explicit gaps. The modes describe maintained Code Mower paths: | Review | Local runner | Evidence only | | Message | Unavailable | Unavailable | | Cancel | Unavailable | Unavailable | -| Authorized context delivery | Local runner (attach to the packet) | Agent handoff (authorized packet in hosted builder input) | +| Authorized context delivery | Unavailable | Agent handoff (authorized packet in hosted builder input) | | Structured results | Local runner | Release campaign only | These declarations describe the current integration; they do not launch a diff --git a/src/code_mower/devin_work_orders.py b/src/code_mower/devin_work_orders.py index acf399e4..b30694e1 100644 --- a/src/code_mower/devin_work_orders.py +++ b/src/code_mower/devin_work_orders.py @@ -13,7 +13,7 @@ from pathlib import Path from typing import Callable, Protocol -from .context_contract import ContextError, ContextRequest, normalize_policy +from .context_contract import ContextError, ContextRequest, ValidatedPacket, normalize_policy from .context_delivery import render_evidence from .context_packets import load_authorized from .context_store import ContextStore @@ -38,6 +38,8 @@ } MAX_INPUT_BYTES = 65536 CONTEXT_RECIPIENT = "devin:builder" +CONTEXT_POLICIES = ("none", "optional", "required") +CONTEXT_STATES = ("delivered", "degraded", "omitted") SHA = re.compile(r"[0-9a-f]{40}\Z") LOGIN = re.compile(r"[A-Za-z0-9][A-Za-z0-9-]{0,38}(?:\[bot\])?\Z") @@ -74,18 +76,20 @@ class WorkOrder: author_login: str acu_limit: int body: str = field(repr=False) + context_policy: str = "none" @classmethod def from_manifest(cls, manifest: dict, body: str, *, repository: str, issue: int, branch: str, base: str, author_id: int, author_login: str, - acu_limit: int = 10) -> WorkOrder: + acu_limit: int = 10, context_policy: str = "none") -> WorkOrder: source = manifest.get("source", {}) if (manifest.get("schema") != WORK_ORDER_SCHEMA or manifest.get("repo") != repository or not isinstance(source, dict) or source.get("repo") != repository or str(source.get("issue_number")) != str(issue)): raise RemoteError("work_order_binding_mismatch") - return cls(repository, issue, branch, base, author_id, author_login, acu_limit, body) + return cls(repository, issue, branch, base, author_id, author_login, acu_limit, body, + context_policy) def __post_init__(self): if (not isinstance(self.repository, str) or len(self.repository) > 256 @@ -95,7 +99,7 @@ def __post_init__(self): or not LOGIN.fullmatch(self.author_login) or type(self.acu_limit) is not int or not 1 <= self.acu_limit <= 100 or not isinstance(self.body, str) or not self.body.strip() - or len(self.body.encode()) > 48000): + or len(self.body.encode()) > 48000 or self.context_policy not in CONTEXT_POLICIES): raise RemoteError("invalid_work_order") @@ -138,16 +142,13 @@ def read(self, repository: str, number: int) -> PullRequest: ... @dataclass(frozen=True, repr=False) class PacketContext: - """Authorized packet bound to exactly one work order's repository and issue. + """Renders one authorized packet; carries no caller-editable identity or policy. - ``required`` mirrors the trusted context policy: required context fails the - paid write closed when it cannot be reauthorized; optional context degrades - to a code-only input and reports ``degraded`` in the returned metadata. + The trusted context decision lives on ``WorkOrder.context_policy``; the loaded + packet's own repository/work-item binding is checked against the order at + every render, so this wrapper cannot redirect another ticket's packet. """ - repository: str - issue: int - required: bool - render: Callable[[], str] = field(repr=False) + render: Callable[[], tuple[ValidatedPacket, str]] = field(repr=False) def packet_context(store: ContextStore, name: str, handle: str, policy, *, order: WorkOrder, @@ -155,23 +156,25 @@ def packet_context(store: ContextStore, name: str, handle: str, policy, *, order """Bind one authorized packet to the hosted builder before its PR exists. The packet request is derived from the work order (repository and issue - number as the work item), never from caller-supplied identity. Each render - performs a new online authorization for ``devin:builder`` and returns the - common evidence payload; nothing is cached or written. + number as the work item), never from caller-supplied identity, and the + trusted policy's ``required`` flag must agree with the order's declared + context policy. Each render performs a new online authorization for + ``devin:builder``; nothing is cached or written. """ try: normalized = normalize_policy(policy) except ContextError: raise RemoteError("invalid_request") from None - if normalized is None: + if normalized is None or order.context_policy == "none" \ + or normalized["required"] != (order.context_policy == "required"): raise RemoteError("invalid_request") request = ContextRequest(order.repository, str(order.issue), CONTEXT_RECIPIENT) - def render() -> str: + def render() -> tuple[ValidatedPacket, str]: packet = load_authorized(store, name, handle, policy, request, backend=backend) - return render_evidence(packet, handle) + return packet, render_evidence(packet, handle) - return PacketContext(order.repository, order.issue, normalized["required"], render) + return PacketContext(render) def _github_call(method, *args, **kwargs): @@ -224,22 +227,34 @@ def _evidence(order, context): ``context`` renders the authorized packet for ``devin:builder`` through the ordinary online authorization path. It runs immediately before the paid provider write, never in preview, and its text is never persisted. - Returns ``(evidence, state)`` where state is ``delivered``, ``degraded`` - (optional context unavailable) or ``omitted`` (no context supplied). + The order's trusted ``context_policy`` decides the outcome: required + context fails closed, optional context degrades to code-only, and a + missing packet is ``omitted`` only when the policy allows it. Returns + ``(evidence, state)`` with state in ``CONTEXT_STATES``. """ + required = order.context_policy == "required" if context is None: + if required: + raise RemoteError("context_unavailable") return None, "omitted" - if (not isinstance(context, PacketContext) or context.repository != order.repository - or context.issue != order.issue): + if order.context_policy == "none" or not isinstance(context, PacketContext): raise RemoteError("context_binding_mismatch") try: - evidence = context.render() - if not isinstance(evidence, str) or "Packet identity: " not in evidence: - raise ContextError("malformed context evidence") + packet, evidence = context.render() except ContextError: - if context.required: + if required: raise RemoteError("context_unavailable") from None return None, "degraded" + if not isinstance(packet, ValidatedPacket) or not isinstance(evidence, str): + raise RemoteError("context_binding_mismatch") + binding = packet.private_payload().get("binding") + if (not isinstance(binding, dict) or binding.get("repository") != order.repository + or binding.get("work_item") != str(order.issue)): + raise RemoteError("context_binding_mismatch") + if "Packet identity: " not in evidence: + if required: + raise RemoteError("context_unavailable") + return None, "degraded" return evidence, "delivered" @classmethod @@ -301,14 +316,23 @@ def run(self, command: str, order: WorkOrder, *, apply: bool = False, # Library equivalent of --apply: no reads, writes or provider calls in preview. if command != "status" and not apply: return {"schema": EVIDENCE_SCHEMA, "mode": "dry_run", "apply_required": True} + if context is not None and command not in {"dispatch", "fix", "clarify"}: + raise RemoteError("invalid_request") key = self._key(order) + context_state = None with self.store.locked(key) as locked: record = locked.read() + if command == "dispatch": + # Render and bound the full create input before any reservation is written, + # so a rejected context never binds an undispatched order or its branch. + evidence, context_state = self._evidence(order, context) + dispatch_prose = self._with_evidence(self._prompt(order), evidence) if record is None: if command != "dispatch": raise RemoteError("work_order_not_found") record = {"binding": self._binding(order), "round": 0, "claim": None, - "evidence": None, "message": None, "observed_acu": None, "pr_number": None, "requests": []} + "evidence": None, "message": None, "observed_acu": None, "pr_number": None, + "requests": [], "context": context_state} locked.write(record) # Stable identity survives remote create uncertainty. if record["binding"] != self._binding(order): raise RemoteError("work_order_binding_mismatch") @@ -322,13 +346,8 @@ def run(self, command: str, order: WorkOrder, *, apply: bool = False, branch_lock.write({"issue_key": key}) remote_command = command kwargs = {} - context_state = None - if context is not None and command not in {"dispatch", "fix", "clarify"}: - raise RemoteError("invalid_request") if command == "dispatch": - evidence, context_state = self._evidence(order, context) - kwargs.update(prose=self._with_evidence(self._prompt(order), evidence), - repo=order.repository, limit=order.acu_limit) + kwargs.update(prose=dispatch_prose, repo=order.repository, limit=order.acu_limit) elif command in {"fix", "clarify"}: if (not isinstance(request, str) or not request.strip() or len(request) > 128 or not isinstance(prose, str) or not prose.strip() or len(prose.encode()) > 48000): @@ -356,16 +375,14 @@ def run(self, command: str, order: WorkOrder, *, apply: bool = False, message = self._message(record["round"] + 1, reviewed_head, prose, evidence) record["round"] += 1 record.update(claim=None, evidence=None, message={ - "request": request, "fingerprint": fingerprint, "pending": True}) + "request": request, "fingerprint": fingerprint, "pending": True, + "context": context_state}) record["requests"].append(request) locked.write(record) # Invalidate evidence before any remote mutation. elif previous["fingerprint"] != fingerprint: raise RemoteError("request_conflict") else: - if acknowledge_delivered: - evidence, context_state = None, "omitted" - else: - evidence, context_state = self._evidence(order, context) + evidence = None if acknowledge_delivered else self._evidence(order, context)[0] message = self._message(record["round"], reviewed_head, prose, evidence) remote_command = "message" kwargs.update(prose=message) @@ -396,4 +413,6 @@ def run(self, command: str, order: WorkOrder, *, apply: bool = False, "session": result, "round": record["round"], "acu_limit": order.acu_limit, "observed_acu": record["observed_acu"], "verified_pr": record["evidence"] if command == "collect" else None, - "context": context_state, "merge_authority": False} + "context": {"policy": order.context_policy, "dispatch": record.get("context"), + "message": (record["message"] or {}).get("context")}, + "merge_authority": False} diff --git a/src/code_mower/doctor_checks/providers.py b/src/code_mower/doctor_checks/providers.py index 5313d445..07e62f96 100644 --- a/src/code_mower/doctor_checks/providers.py +++ b/src/code_mower/doctor_checks/providers.py @@ -203,7 +203,7 @@ def check_lane_runtime( name="provider.capabilities", status=STATUS_WARN, lane=lane_id, message=f"{transport.product} via {transport.transport}: unavailable capabilities: " + ", ".join(transport.brief()["capability_gaps"]), detail=transport.brief(), - remediation="Use only declared capability modes; session messaging and cancellation are unavailable; authorized context is delivered only through the declared mode. Selection does not grant merge authority.", + remediation="Use only declared capability modes; session messaging and cancellation are unavailable, and authorized context reaches only the hosted builder input (never the local CLI reviewer). Selection does not grant merge authority.", )) driver = str(lane.get("driver", "")) skip_local_cli_runtime = ( diff --git a/src/code_mower/provider_capabilities.py b/src/code_mower/provider_capabilities.py index 79bf598a..c756d358 100644 --- a/src/code_mower/provider_capabilities.py +++ b/src/code_mower/provider_capabilities.py @@ -59,7 +59,7 @@ def brief(self) -> dict[str, Any]: "devin", "devin_cli", "local_cli", "devin_cli", Capabilities( coordinate="agent_handoff", build="local_runner", review="local_runner", - message="unavailable", cancel="unavailable", context="local_runner", + message="unavailable", cancel="unavailable", context="unavailable", structured_results="local_runner", ), ), diff --git a/src/code_mower/provider_capabilities.schema.json b/src/code_mower/provider_capabilities.schema.json index 90dcca97..f3921126 100644 --- a/src/code_mower/provider_capabilities.schema.json +++ b/src/code_mower/provider_capabilities.schema.json @@ -31,7 +31,7 @@ "review": "local_runner", "message": "unavailable", "cancel": "unavailable", - "context": "local_runner", + "context": "unavailable", "structured_results": "local_runner" } }, @@ -41,7 +41,8 @@ "capability_gaps": { "const": [ "message", - "cancel" + "cancel", + "context" ] } } diff --git a/src/code_mower/templates/providers.yml b/src/code_mower/templates/providers.yml index b85ec3a9..0222daa8 100644 --- a/src/code_mower/templates/providers.yml +++ b/src/code_mower/templates/providers.yml @@ -305,7 +305,7 @@ provider_templates: review: "local_runner" message: "unavailable" cancel: "unavailable" - context: "local_runner" + context: "unavailable" structured_results: "local_runner" driver: "local_cli" type: "audit" diff --git a/templates/providers.yml b/templates/providers.yml index b85ec3a9..0222daa8 100644 --- a/templates/providers.yml +++ b/templates/providers.yml @@ -305,7 +305,7 @@ provider_templates: review: "local_runner" message: "unavailable" cancel: "unavailable" - context: "local_runner" + context: "unavailable" structured_results: "local_runner" driver: "local_cli" type: "audit" diff --git a/templates/providers/devin_cli.yml b/templates/providers/devin_cli.yml index 54336cb3..218ff266 100644 --- a/templates/providers/devin_cli.yml +++ b/templates/providers/devin_cli.yml @@ -8,7 +8,7 @@ devin_cli: review: "local_runner" message: "unavailable" cancel: "unavailable" - context: "local_runner" + context: "unavailable" structured_results: "local_runner" driver: "local_cli" type: "audit" diff --git a/tests/test_devin_capabilities.py b/tests/test_devin_capabilities.py index 7f3f26fd..6f5f3d30 100644 --- a/tests/test_devin_capabilities.py +++ b/tests/test_devin_capabilities.py @@ -168,7 +168,7 @@ def test_session_briefs_preserve_transport_aliases_and_capability_gaps(self): for capability in ("message", "cancel"): self.assertIn(capability + "=unavailable", rendered) self.assertIn("context=" + TRANSPORTS[expected].capabilities.context, rendered) - self.assertNotIn("context=unavailable", rendered) + self.assertEqual(expected == "devin_api_v3", "context=agent_handoff" in rendered) def test_hosted_transport_cannot_coordinate(self): with self.assertRaisesRegex(config.ConfigError, "cannot coordinate"): @@ -223,7 +223,7 @@ def test_doctor_reports_static_gaps_without_a_live_probe(self): check = next(check for check in checks if check.name == "provider.capabilities") self.assertEqual(check.detail, transport.brief()) self.assertIn("message, cancel", check.message) - self.assertNotIn("context", check.message) + self.assertEqual(transport.transport == "devin_cli", "context" in check.message) self.assertEqual(check.status, "warn") def test_shipped_schema_matches_each_complete_transport_contract(self): diff --git a/tests/test_devin_work_orders.py b/tests/test_devin_work_orders.py index 6fd7f6f6..810e9273 100644 --- a/tests/test_devin_work_orders.py +++ b/tests/test_devin_work_orders.py @@ -14,7 +14,8 @@ from code_mower.context_packets import fetch from code_mower.devin_sessions import DevinClient from code_mower.devin_work_orders import ( - COMPLETION_SCHEMA, MAX_INPUT_BYTES, Candidates, DevinWorkOrders, PullRequest, WorkOrder, packet_context, + COMPLETION_SCHEMA, MAX_INPUT_BYTES, Candidates, DevinWorkOrders, PacketContext, PullRequest, WorkOrder, + packet_context, ) from code_mower.remote_session import FakeProvider, RemoteError, RemoteSessions, _key from code_mower.work_orders import WORK_ORDER_SCHEMA @@ -297,26 +298,38 @@ def runner(method, url, body, headers): CONTEXT_CANARY = "PRIVATE_CONTEXT_PACKET_CANARY_TEXT" +def _fail(): + raise ContextError("context search unavailable") + + @unittest.skipUnless(os.name == "posix", "private packets need POSIX protections") class ContextInjectionTests(WorkOrderCase): """The hosted builder receives the common packet only inside create/message input.""" def setUp(self): super().setUp() + self.order = replace(self.order, context_policy="required") + self.key = self.service._key(self.order) + self.optional_order = replace(self.order, context_policy="optional") self.fixture = fixtures.ContextDeliveryTests(methodName="runTest") self.fixture.setUp() self.addCleanup(self.fixture.doCleanups) self.backend = self.fixture.backend self.backend.result["result"]["results"][0]["text"] = CONTEXT_CANARY self.policy = self.fixture.spec["policy"] + self.optional_policy = {**self.policy, "required": False} self.spec = {**self.fixture.spec, "work_item": str(self.order.issue)} self.result = fetch(self.fixture.store, "example", self.spec, backend=self.backend, refresh=True) self.context = self.bind() + self.optional = self.bind(order=self.optional_order, policy=self.optional_policy) def bind(self, order=None, policy=None, handle=None): return packet_context(self.fixture.store, "example", handle or self.result["packet_handle"], policy or self.policy, order=order or self.order, backend=self.backend) + def run_optional(self, command, **kwargs): + return self.service.run(command, self.optional_order, apply=True, **kwargs) + def private_files(self): return [path for path in self.root.rglob("*") if path.is_file()] @@ -327,22 +340,24 @@ def assert_not_persisted(self, *outputs): for path in self.private_files(): self.assertNotIn(CONTEXT_CANARY.encode(), path.read_bytes()) - def test_packet_context_binds_the_work_order_identity_and_policy(self): - self.assertEqual((self.context.repository, self.context.issue, self.context.required), - (self.order.repository, self.order.issue, True)) - self.assertFalse(self.bind(policy={**self.policy, "required": False}).required) + def test_packet_context_carries_no_identity_and_policy_must_agree_with_the_order(self): + self.assertEqual(tuple(self.context.__dataclass_fields__), ("render",)) self.assertNotIn(CONTEXT_CANARY, repr(self.context)) - for policy in (None, {}, {**self.policy, "required": "yes"}): - with self.subTest(policy=policy), self.assertRaisesRegex(RemoteError, "invalid_request"): + with self.assertRaises(RemoteError): + replace(self.order, context_policy="always") + for order, policy in ((self.order, self.optional_policy), (self.optional_order, self.policy), + (replace(self.order, context_policy="none"), self.policy), + (self.order, None), (self.order, {}), (self.order, {**self.policy, "required": "yes"})): + with self.subTest(policy=order.context_policy), self.assertRaisesRegex(RemoteError, "invalid_request"): packet_context(self.fixture.store, "example", self.result["packet_handle"], policy, - order=self.order, backend=self.backend) + order=order, backend=self.backend) def test_preview_never_retrieves_context(self): calls = [] def spy(): calls.append(1) return self.context.render() - context = replace(self.context, render=spy) + context = PacketContext(spy) for command in ("dispatch", "clarify", "fix"): output = self.service.run(command, self.order, context=context, request="r", prose="p") self.assertTrue(output["apply_required"]) @@ -354,7 +369,7 @@ def test_create_and_message_receive_the_common_evidence_once_each(self): with patch.object(self.provider, "create", wraps=self.provider.create) as create, \ patch.object(self.provider, "message", wraps=self.provider.message) as message: outputs.append(self.run_order("dispatch", context=self.context)) - self.assertEqual(outputs[-1]["context"], "delivered") + self.assertEqual(outputs[-1]["context"], {"policy": "required", "dispatch": "delivered", "message": None}) prompt = create.call_args.args[0] self.assertIn(CANARY, prompt) self.assertIn(CONTEXT_CANARY, prompt) @@ -364,51 +379,71 @@ def test_create_and_message_receive_the_common_evidence_once_each(self): "Packet identity: " + self.result["packet_handle"]), 1) outputs.append(self.run_order("dispatch", context=self.context)) self.assertEqual(create.call_count, 1) - with self.assertRaisesRegex(RemoteError, "request_conflict"): - self.run_order("dispatch") # Changed create input, even by omission, never redispatches. + with self.assertRaisesRegex(RemoteError, "context_unavailable"): + self.run_order("dispatch") # Required context cannot be dropped on replay. self.assertEqual(create.call_count, 1) outputs.append(self.run_order("clarify", request="c-1", prose=CANARY, context=self.context)) - self.assertEqual(outputs[-1]["context"], "delivered") + self.assertEqual(outputs[-1]["context"]["message"], "delivered") self.assertIn(CONTEXT_CANARY, message.call_args.args[1]) self.assertIn(CANARY, message.call_args.args[1]) - outputs.append(self.run_order("clarify", request="c-2", prose=CANARY)) - self.assertEqual(outputs[-1]["context"], "omitted") - self.assertNotIn(CONTEXT_CANARY, message.call_args.args[1]) - self.complete(round=2) + self.complete(round=1) outputs.append(self.run_order("collect")) - self.assertIsNone(outputs[-1]["context"]) + self.assertEqual(outputs[-1]["context"], {"policy": "required", "dispatch": "delivered", "message": "delivered"}) outputs.append(self.run_order("fix", request="fix-1", prose=CANARY, reviewed_head=HEAD, context=self.context)) self.assertIn(CONTEXT_CANARY, message.call_args.args[1]) - self.assertEqual(message.call_count, 3) + self.assertEqual(message.call_count, 2) self.assert_not_persisted(*outputs) with self.remote.store.locked(_key(self.key)) as locked: self.assertNotIn(CONTEXT_CANARY, json.dumps(locked.read())) + def test_rejected_dispatch_context_writes_no_reservation(self): + oversized = PacketContext(lambda: (self.context.render()[0], "Packet identity: x\n" + "y" * MAX_INPUT_BYTES)) + for context, error in ((PacketContext(_fail), "context_unavailable"), (None, "context_unavailable"), + (oversized, "context_budget_exceeded"), + (self.bind(order=replace(self.order, issue=908)), "context_unavailable"), + (self.context.render, "context_binding_mismatch")): + with self.subTest(error=error), self.assertRaisesRegex(RemoteError, error): + self.run_order("dispatch", context=context) + with self.assertRaisesRegex(RemoteError, "work_order_not_found"): + self.run_order("status") + # The undispatched order binds neither its body nor its branch. + corrected = replace(self.order, body=CANARY + " corrected") + self.service.run("dispatch", corrected, apply=True, context=self.bind(order=corrected)) + other = replace(self.order, issue=908, branch="devin/908", context_policy="none") + self.service.run("dispatch", other, apply=True) + def test_status_collect_and_cancel_reject_context(self): - self.run_order("dispatch") + self.run_order("dispatch", context=self.context) for command in ("status", "collect", "cancel"): with self.subTest(command=command), self.assertRaisesRegex(RemoteError, "invalid_request"): self.run_order(command, request="cancel-1", context=self.context) def test_context_must_be_bound_to_this_work_order(self): - other_issue = replace(self.order, issue=908) - other_repo = replace(self.order, repository="other/repo") + ticket_b = replace(self.order, issue=908, branch="devin/908") + spec_b = {**self.spec, "work_item": "908"} + handle_b = fetch(self.fixture.store, "example", spec_b, backend=self.backend, refresh=True)["packet_handle"] + valid_b = self.bind(order=ticket_b, handle=handle_b) + self.service.run("dispatch", ticket_b, apply=True, context=valid_b) # Ticket B's own packet is fine. + # A wrapper pointed at another order authorizes against that order and fails closed. + with self.assertRaisesRegex(RemoteError, "context_unavailable"): + self.run_order("dispatch", context=self.bind(order=replace(self.order, repository="other/repo"))) + policy_none = replace(self.order, issue=909, branch="devin/909", context_policy="none") cases = { - "other_issue": self.bind(order=other_issue), - "other_repository": self.bind(order=other_repo), - "unbound_callable": self.context.render, - "forged_binding": replace(self.bind(order=other_issue), repository="other/repo"), + "packet_for_ticket_b": (self.order, valid_b), + "wrapper_relabelled_for_ticket_a": (self.order, PacketContext(valid_b.render)), + "unbound_callable": (self.order, self.context.render), + "policy_none_order_with_context": (policy_none, PacketContext(self.context.render)), } - for name, context in cases.items(): + for name, (order, context) in cases.items(): with self.subTest(case=name), patch.object(self.provider, "create", wraps=self.provider.create) as create: with self.assertRaisesRegex(RemoteError, "context_binding_mismatch"): - self.run_order("dispatch", context=context) + self.service.run("dispatch", order, apply=True, context=context) self.assertEqual(create.call_count, 0) self.run_order("dispatch", context=self.context) with patch.object(self.provider, "message", wraps=self.provider.message) as message: with self.assertRaisesRegex(RemoteError, "context_binding_mismatch"): - self.run_order("clarify", request="c-1", prose=CANARY, context=self.bind(order=other_issue)) + self.run_order("clarify", request="c-1", prose=CANARY, context=PacketContext(valid_b.render)) self.assertEqual(message.call_count, 0) self.assertEqual(self.run_order("status")["round"], 0) @@ -421,7 +456,7 @@ def test_authorization_is_rechecked_and_fails_closed_without_a_provider_write(se backend=self.backend, refresh=True)["packet_handle"])), "changed_evidence": lambda: fetch(self.fixture.store, "example", self.spec, backend=self.backend, refresh=True), - "garbage": lambda: setattr(self, "context", replace(self.context, render=lambda: CANARY)), + "garbage": lambda: setattr(self, "context", PacketContext(_fail)), } for name, arrange in cases.items(): with self.subTest(case=name): @@ -435,9 +470,9 @@ def test_authorization_is_rechecked_and_fails_closed_without_a_provider_write(se self.setUp() self.run_order("dispatch", context=self.context) with patch.object(self.provider, "message", wraps=self.provider.message) as message: - with self.assertRaisesRegex(RemoteError, "context_unavailable"): - self.run_order("clarify", request="c-1", prose=CANARY, - context=replace(self.context, render=lambda: CANARY)) + for context in (PacketContext(_fail), None): + with self.assertRaisesRegex(RemoteError, "context_unavailable"): + self.run_order("clarify", request="c-1", prose=CANARY, context=context) self.assertEqual(message.call_count, 0) self.assertEqual(self.run_order("status")["round"], 0) # No local round was consumed. self.assertEqual(self.run_order("clarify", request="c-1", prose=CANARY, @@ -449,35 +484,41 @@ def test_authorization_is_rechecked_and_fails_closed_without_a_provider_write(se self.assertEqual(message.call_count, 0) self.assertEqual(self.run_order("status")["round"], 1) - def test_optional_context_degrades_explicitly_and_required_context_fails_closed(self): - optional = self.bind(policy={**self.policy, "required": False}) + def test_optional_context_degrades_explicitly_and_states_persist(self): + unavailable = PacketContext(_fail) with patch.object(self.provider, "create", wraps=self.provider.create) as create: - output = self.run_order("dispatch", context=None) - self.assertEqual(output["context"], "omitted") + output = self.run_optional("dispatch", context=unavailable) + self.assertEqual(output["context"], {"policy": "optional", "dispatch": "degraded", "message": None}) self.assertNotIn("Packet identity", create.call_args.args[0]) - def fail(): - raise ContextError("context search unavailable") - unavailable = {"render": fail} - with patch.object(self.provider, "message", wraps=self.provider.message) as message: - with self.assertRaisesRegex(RemoteError, "context_unavailable"): - self.run_order("clarify", request="c-1", prose=CANARY, context=replace(self.context, **unavailable)) - self.assertEqual(message.call_count, 0) - output = self.run_order("clarify", request="c-1", prose=CANARY, context=replace(optional, **unavailable)) - self.assertEqual((output["context"], output["round"]), ("degraded", 1)) - self.assertNotIn(CONTEXT_CANARY, message.call_args.args[1]) - self.assertIn(CANARY, message.call_args.args[1]) - with patch.object(self.provider, "message", wraps=self.provider.message) as message: + self.assertIn(CANARY, create.call_args.args[0]) with self.assertRaisesRegex(RemoteError, "request_conflict"): - self.run_order("clarify", request="c-1", prose=CANARY, context=optional) # Input changed. - output = self.run_order("clarify", request="c-1", prose=CANARY, context=replace(optional, **unavailable)) - self.assertEqual((output["context"], output["round"]), ("degraded", 1)) - output = self.run_order("clarify", request="c-2", prose=CANARY, context=optional) - self.assertEqual((output["context"], output["round"]), ("delivered", 2)) + self.run_optional("dispatch", context=self.optional) # Input changed. + self.assertEqual(self.run_optional("dispatch", context=unavailable)["context"]["dispatch"], "degraded") + with self.assertRaisesRegex(RemoteError, "context_binding_mismatch"): # Optional never relaxes binding. + handle_b = fetch(self.fixture.store, "example", {**self.spec, "work_item": "908"}, + backend=self.backend, refresh=True)["packet_handle"] + other = self.bind(order=replace(self.optional_order, issue=908), policy=self.optional_policy, handle=handle_b) + self.run_optional("clarify", request="c-0", prose=CANARY, context=PacketContext(other.render)) + with patch.object(self.provider, "message", wraps=self.provider.message) as message: + output = self.run_optional("clarify", request="c-1", prose=CANARY, context=None) + self.assertEqual((output["context"]["message"], output["round"]), ("omitted", 1)) + output = self.run_optional("clarify", request="c-2", prose=CANARY, context=self.optional) + self.assertEqual((output["context"]["message"], output["round"]), ("delivered", 2)) self.assertIn(CONTEXT_CANARY, message.call_args.args[1]) - self.assert_not_persisted(output) + # Acknowledging a delivered message preserves its saved state instead of "omitted". + output = self.run_optional("clarify", request="c-2", prose=CANARY, acknowledge_delivered=True) + self.assertEqual(output["context"], {"policy": "optional", "dispatch": "degraded", "message": "delivered"}) + self.assertEqual(message.call_count, 2) + status = self.run_optional("status") + self.assertEqual(status["context"], {"policy": "optional", "dispatch": "degraded", "message": "delivered"}) + self.assertEqual(status["round"], 2) + with self.service.store.locked(self.service._key(self.optional_order)) as locked: + record = locked.read() + self.assertEqual((record["context"], record["message"]["context"]), ("degraded", "delivered")) + self.assert_not_persisted(output, status) def test_combined_input_is_bounded(self): - oversized = replace(self.context, render=lambda: "Packet identity: x\n" + "y" * MAX_INPUT_BYTES) + oversized = PacketContext(lambda: (self.context.render()[0], "Packet identity: x\n" + "y" * MAX_INPUT_BYTES)) with patch.object(self.provider, "create", wraps=self.provider.create) as create: with self.assertRaisesRegex(RemoteError, "context_budget_exceeded"): self.run_order("dispatch", context=oversized) From 7cfaece38fc389360b19193193dfafa262891d29 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Sat, 12 Sep 2026 12:00:19 +0000 Subject: [PATCH 4/7] Devin work orders: render evidence from the bound packet, bind retry input, keep legacy records readable - PacketContext now only loads a ValidatedPacket; evidence is rendered inside _evidence() from that exact packet after checking repository, work item and the devin:builder recipient, so no separately supplied evidence is accepted. - Required unavailable context returns the closed UNKNOWN/paused outcome at the run() boundary with no reservation or consumed round. - A digest of the complete intended input (prose plus evidence/state) is stored before the local dispatch/message intent is durable and validated on retry. - Context-free orders keep the pre-context binding and prompt serialization; legacy records without context/input fields read as context-free. Co-Authored-By: bot_apk --- docs/context-delivery.md | 41 ++++-- src/code_mower/devin_work_orders.py | 112 +++++++++++----- tests/test_devin_work_orders.py | 190 +++++++++++++++++++++------- 3 files changed, 254 insertions(+), 89 deletions(-) diff --git a/docs/context-delivery.md b/docs/context-delivery.md index ac55ff71..ef817585 100644 --- a/docs/context-delivery.md +++ b/docs/context-delivery.md @@ -84,13 +84,18 @@ dispatcher policy, and is included in the durable binding. The embedding binds one packet with `devin_work_orders.packet_context(store, name, handle, policy, order=order)`, which derives the packet request from the order's repository and issue number, requires the trusted policy's `required` flag to agree with the -order, and returns a `PacketContext` holding only a render closure. That value +order, and returns a `PacketContext` holding only a packet-loading closure. That +value is passed as `context=` to `dispatch`, `clarify`, or `fix`. Immediately before each paid create or message write, and never in preview, the context performs a new online authorization for `devin:builder`, and the loaded -packet's own repository and work-item binding is compared with the order; a -packet for another ticket, a relabelled wrapper, a bare callable, or any context +packet's own binding is compared with the order: its repository and work item +must match and `devin:builder` must be among its bound recipients. The evidence +is then rendered inside the work-order boundary from that exact validated +packet, with an identity derived from the packet digest; no separately supplied +evidence text is ever accepted. A packet for another ticket, a packet without +the recipient, a relabelled wrapper, a bare callable or string, or any context on a `none` order fails as `context_binding_mismatch`. Rendering and the 64 KiB combined-size check (`context_budget_exceeded`) happen before the work-order record, branch reservation, or a new round is written, so a rejected input @@ -98,16 +103,28 @@ leaves no undispatched reservation and consumes no round. Evidence text is appended only to the provider input; records keep only digests plus the safe state enum, and status, collect, and cancel reject a context argument. -A `required` order fails closed as `context_unavailable` when no packet is -supplied or it cannot be reauthorized (wrong account, revoked or expired -authorization, refreshed or invalidated packet, payload without the packet -identity); the work order pauses with its prior state intact. Only an `optional` -order degrades to a code-only input (`degraded`) or runs without a packet -(`omitted`), and the remote session's input fingerprint prevents a later replay -with different input. The state chosen for the create input and for each -message intent is persisted and reported by every command as +A `required` order fails closed when no packet is supplied or it cannot be +reauthorized (wrong account, revoked or expired authorization, refreshed or +invalidated packet). `run` then returns, rather than raises, the issue's closed +outcome: `{"outcome": "UNKNOWN", "state": "paused", "reason": +"context_unavailable", "context": {"policy": "required", "dispatch" | "message": +"unavailable"}, "merge_authority": false}`; nothing was reserved, persisted, or +sent, and an already dispatched order keeps its prior round and state. Only an +`optional` order degrades to a code-only input (`degraded`) or runs without a +packet (`omitted`). + +The complete intended input, including the evidence and its safe state, is +digested before the local dispatch or message intent becomes durable, and the +remote session's input fingerprint covers the same text. A retry after a stop +between the two writes must regenerate the identical input; changed, refreshed, +or dropped evidence fails as `request_conflict` and the saved state keeps +reporting what was actually intended. The state chosen for the create input and +for each message intent is persisted and reported by every command as `context: {policy, dispatch, message}`; replay, acknowledgement, status, and -collect return the saved state rather than recomputing it. +collect return the saved state rather than recomputing it. Durable records +written before the context field existed keep their original binding and +dispatch input: a `none` order serializes without `context_policy`, and records +without context or input digests are read as context-free. ## Attach evidence to independent review diff --git a/src/code_mower/devin_work_orders.py b/src/code_mower/devin_work_orders.py index b30694e1..4630938f 100644 --- a/src/code_mower/devin_work_orders.py +++ b/src/code_mower/devin_work_orders.py @@ -9,6 +9,7 @@ import json import math import re +import uuid from dataclasses import asdict, dataclass, field from pathlib import Path from typing import Callable, Protocol @@ -40,6 +41,7 @@ CONTEXT_RECIPIENT = "devin:builder" CONTEXT_POLICIES = ("none", "optional", "required") CONTEXT_STATES = ("delivered", "degraded", "omitted") +CONTEXT_UNAVAILABLE = "unavailable" # Reported, never persisted: the closed UNKNOWN/paused outcome. SHA = re.compile(r"[0-9a-f]{40}\Z") LOGIN = re.compile(r"[A-Za-z0-9][A-Za-z0-9-]{0,38}(?:\[bot\])?\Z") @@ -142,13 +144,15 @@ def read(self, repository: str, number: int) -> PullRequest: ... @dataclass(frozen=True, repr=False) class PacketContext: - """Renders one authorized packet; carries no caller-editable identity or policy. + """Loads one authorized packet; carries no caller-editable identity, policy, or text. - The trusted context decision lives on ``WorkOrder.context_policy``; the loaded - packet's own repository/work-item binding is checked against the order at - every render, so this wrapper cannot redirect another ticket's packet. + The trusted context decision lives on ``WorkOrder.context_policy``. The loaded + packet's own repository, work-item, and recipient binding is checked against + the order and the evidence is rendered from that exact packet inside the + work-order boundary, so this wrapper can supply neither another ticket's + packet nor separately authored evidence. """ - render: Callable[[], tuple[ValidatedPacket, str]] = field(repr=False) + load: Callable[[], ValidatedPacket] = field(repr=False) def packet_context(store: ContextStore, name: str, handle: str, policy, *, order: WorkOrder, @@ -170,11 +174,10 @@ def packet_context(store: ContextStore, name: str, handle: str, policy, *, order raise RemoteError("invalid_request") request = ContextRequest(order.repository, str(order.issue), CONTEXT_RECIPIENT) - def render() -> tuple[ValidatedPacket, str]: - packet = load_authorized(store, name, handle, policy, request, backend=backend) - return packet, render_evidence(packet, handle) + def load() -> ValidatedPacket: + return load_authorized(store, name, handle, policy, request, backend=backend) - return PacketContext(render) + return PacketContext(load) def _github_call(method, *args, **kwargs): @@ -201,12 +204,20 @@ def hosted(cls, root: Path, client: DevinClient, github: GitHub) -> DevinWorkOrd def _key(order): return "b" + _hash([order.repository.lower(), order.issue])[:62] + @staticmethod + def _fields(order): + """Serialized order identity; the pre-context shape is kept for context-free orders.""" + fields = asdict(order) + if fields["context_policy"] == "none": + del fields["context_policy"] + return fields + def _binding(self, order): - return _hash([asdict(order), self.remote.provider.name, self.remote.provider.account]) + return _hash([self._fields(order), self.remote.provider.name, self.remote.provider.account]) - @staticmethod - def _prompt(order, round_number=0): - policy = {k: v for k, v in asdict(order).items() if k != "body"} + @classmethod + def _prompt(cls, order, round_number=0): + policy = {k: v for k, v in cls._fields(order).items() if k != "body"} return ( "Execute exactly one trusted Code Mower work order. Single writer: you alone may " "write the specified branch in the specified repository. Never write another branch, " @@ -224,13 +235,16 @@ def _prompt(order, round_number=0): def _evidence(order, context): """Render freshly reauthorized evidence for exactly one create/message input. - ``context`` renders the authorized packet for ``devin:builder`` through - the ordinary online authorization path. It runs immediately before the - paid provider write, never in preview, and its text is never persisted. - The order's trusted ``context_policy`` decides the outcome: required - context fails closed, optional context degrades to code-only, and a - missing packet is ``omitted`` only when the policy allows it. Returns - ``(evidence, state)`` with state in ``CONTEXT_STATES``. + ``context`` loads the authorized packet for ``devin:builder`` through the + ordinary online authorization path, immediately before the paid provider + write and never in preview. The packet's embedded binding must name this + order's repository and work item and the ``devin:builder`` recipient; the + evidence is then rendered here from that exact packet, with an identity + derived from the packet digest, and is never persisted. The order's + trusted ``context_policy`` decides the outcome: required context fails + closed (``context_unavailable``), optional context degrades to code-only, + and a missing packet is ``omitted`` only when the policy allows it. + Returns ``(evidence, state)`` with state in ``CONTEXT_STATES``. """ required = order.context_policy == "required" if context is None: @@ -240,21 +254,22 @@ def _evidence(order, context): if order.context_policy == "none" or not isinstance(context, PacketContext): raise RemoteError("context_binding_mismatch") try: - packet, evidence = context.render() + packet = context.load() + if not isinstance(packet, ValidatedPacket): + raise RemoteError("context_binding_mismatch") + binding = packet.private_payload().get("binding") + if not isinstance(binding, dict): + raise RemoteError("context_binding_mismatch") + recipients = binding.get("recipients") + if (binding.get("repository") != order.repository + or binding.get("work_item") != str(order.issue) + or not isinstance(recipients, list) or CONTEXT_RECIPIENT not in recipients): + raise RemoteError("context_binding_mismatch") + evidence = render_evidence(packet, uuid.UUID(hex=packet.sha256[:32]).hex) except ContextError: if required: raise RemoteError("context_unavailable") from None return None, "degraded" - if not isinstance(packet, ValidatedPacket) or not isinstance(evidence, str): - raise RemoteError("context_binding_mismatch") - binding = packet.private_payload().get("binding") - if (not isinstance(binding, dict) or binding.get("repository") != order.repository - or binding.get("work_item") != str(order.issue)): - raise RemoteError("context_binding_mismatch") - if "Packet identity: " not in evidence: - if required: - raise RemoteError("context_unavailable") - return None, "degraded" return evidence, "delivered" @classmethod @@ -311,6 +326,28 @@ def run(self, command: str, order: WorkOrder, *, apply: bool = False, request: str = "", prose: str = "", reviewed_head: str = "", acknowledge_delivered: bool = False, context: PacketContext | None = None) -> dict: + """Run one intent; a required packet that cannot be authorized pauses the order. + + That closed outcome is returned, not raised: ``outcome`` is ``UNKNOWN``, + ``state`` is ``paused``, nothing was reserved, persisted, or sent, and + ``context`` reports ``unavailable`` for the intent that was refused. + """ + try: + return self._run(command, order, apply=apply, request=request, prose=prose, + reviewed_head=reviewed_head, acknowledge_delivered=acknowledge_delivered, + context=context) + except RemoteError as exc: + if exc.args != ("context_unavailable",): + raise + slot = "dispatch" if command == "dispatch" else "message" + return {"schema": EVIDENCE_SCHEMA, "builder": self.transport.product, + "transport": "fake" if self.remote.provider.name == "fake" else self.transport.transport, + "outcome": "UNKNOWN", "state": "paused", "reason": "context_unavailable", + "context": {"policy": order.context_policy, slot: CONTEXT_UNAVAILABLE}, + "merge_authority": False} + + def _run(self, command, order, *, apply, request, prose, reviewed_head, + acknowledge_delivered, context): if command not in {"dispatch", "status", "collect", "clarify", "fix", "cancel"}: raise RemoteError("invalid_request") # Library equivalent of --apply: no reads, writes or provider calls in preview. @@ -327,15 +364,18 @@ def run(self, command: str, order: WorkOrder, *, apply: bool = False, # so a rejected context never binds an undispatched order or its branch. evidence, context_state = self._evidence(order, context) dispatch_prose = self._with_evidence(self._prompt(order), evidence) + input_digest = _hash([dispatch_prose]) if record is None: if command != "dispatch": raise RemoteError("work_order_not_found") record = {"binding": self._binding(order), "round": 0, "claim": None, "evidence": None, "message": None, "observed_acu": None, "pr_number": None, - "requests": [], "context": context_state} + "requests": [], "context": context_state, "input": input_digest} locked.write(record) # Stable identity survives remote create uncertainty. if record["binding"] != self._binding(order): raise RemoteError("work_order_binding_mismatch") + if command == "dispatch" and record.get("input", input_digest) != input_digest: + raise RemoteError("request_conflict") # Replay may not change the create input. # Persistent branch reservation prevents a second issue becoming its writer. branch_key = "w" + _hash([order.repository.lower(), order.branch])[:62] with self.store.locked(branch_key) as branch_lock: @@ -376,14 +416,18 @@ def run(self, command: str, order: WorkOrder, *, apply: bool = False, record["round"] += 1 record.update(claim=None, evidence=None, message={ "request": request, "fingerprint": fingerprint, "pending": True, - "context": context_state}) + "context": context_state, "input": _hash([message])}) record["requests"].append(request) locked.write(record) # Invalidate evidence before any remote mutation. elif previous["fingerprint"] != fingerprint: raise RemoteError("request_conflict") + elif acknowledge_delivered: + message = self._message(record["round"], reviewed_head, prose, None) else: - evidence = None if acknowledge_delivered else self._evidence(order, context)[0] + evidence = self._evidence(order, context)[0] message = self._message(record["round"], reviewed_head, prose, evidence) + if previous.get("input", _hash([message])) != _hash([message]): + raise RemoteError("request_conflict") # Retry may not change the evidence. remote_command = "message" kwargs.update(prose=message) result = self.remote.run(remote_command, key, apply=apply, request=request, diff --git a/tests/test_devin_work_orders.py b/tests/test_devin_work_orders.py index 810e9273..94815b42 100644 --- a/tests/test_devin_work_orders.py +++ b/tests/test_devin_work_orders.py @@ -1,21 +1,22 @@ """Offline trusted-work-order delivery and hostile builder evidence fixtures.""" +import hashlib import json import os import sys import tempfile import unittest -from dataclasses import replace +import uuid +from dataclasses import asdict, replace from pathlib import Path from unittest.mock import patch sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "src")) -from code_mower.context_contract import ContextError +from code_mower.context_contract import ContextError, ValidatedPacket from code_mower.context_packets import fetch from code_mower.devin_sessions import DevinClient from code_mower.devin_work_orders import ( - COMPLETION_SCHEMA, MAX_INPUT_BYTES, Candidates, DevinWorkOrders, PacketContext, PullRequest, WorkOrder, - packet_context, + COMPLETION_SCHEMA, Candidates, DevinWorkOrders, PacketContext, PullRequest, WorkOrder, _hash, packet_context, ) from code_mower.remote_session import FakeProvider, RemoteError, RemoteSessions, _key from code_mower.work_orders import WORK_ORDER_SCHEMA @@ -298,10 +299,19 @@ def runner(method, url, body, headers): CONTEXT_CANARY = "PRIVATE_CONTEXT_PACKET_CANARY_TEXT" +class Crash(Exception): + """Process stop between the local intent write and the remote intent write.""" +PAUSED = {"outcome": "UNKNOWN", "state": "paused", "reason": "context_unavailable"} + + def _fail(): raise ContextError("context search unavailable") +def _identity(packet): + return uuid.UUID(hex=packet.sha256[:32]).hex + + @unittest.skipUnless(os.name == "posix", "private packets need POSIX protections") class ContextInjectionTests(WorkOrderCase): """The hosted builder receives the common packet only inside create/message input.""" @@ -330,18 +340,32 @@ def bind(self, order=None, policy=None, handle=None): def run_optional(self, command, **kwargs): return self.service.run(command, self.optional_order, apply=True, **kwargs) + def large(self): + """Refresh the packet with a document large enough to break the 64 KiB input budget.""" + self.backend.result["result"]["results"][0]["text"] = CONTEXT_CANARY + "y" * 19_000 + self.result = fetch(self.fixture.store, "example", self.spec, backend=self.backend, refresh=True) + self.context = self.bind() + return self.context + def private_files(self): return [path for path in self.root.rglob("*") if path.is_file()] def assert_not_persisted(self, *outputs): serialized = json.dumps(outputs) - for private in (CONTEXT_CANARY, "Packet identity", "one@example.invalid", self.result["packet_handle"]): + for private in (CONTEXT_CANARY, "Packet identity", "one@example.invalid", self.result["packet_handle"], + _identity(self.context.load())): self.assertNotIn(private, serialized) for path in self.private_files(): self.assertNotIn(CONTEXT_CANARY.encode(), path.read_bytes()) + def assert_paused(self, output, slot, policy="required"): + self.assertEqual({k: output[k] for k in PAUSED}, PAUSED) + self.assertEqual(output["context"], {"policy": policy, slot: "unavailable"}) + self.assertFalse(output["merge_authority"]) + self.assertNotIn("session", output) + def test_packet_context_carries_no_identity_and_policy_must_agree_with_the_order(self): - self.assertEqual(tuple(self.context.__dataclass_fields__), ("render",)) + self.assertEqual(tuple(self.context.__dataclass_fields__), ("load",)) self.assertNotIn(CONTEXT_CANARY, repr(self.context)) with self.assertRaises(RemoteError): replace(self.order, context_policy="always") @@ -356,7 +380,7 @@ def test_preview_never_retrieves_context(self): calls = [] def spy(): calls.append(1) - return self.context.render() + return self.context.load() context = PacketContext(spy) for command in ("dispatch", "clarify", "fix"): output = self.service.run(command, self.order, context=context, request="r", prose="p") @@ -366,6 +390,7 @@ def spy(): def test_create_and_message_receive_the_common_evidence_once_each(self): outputs = [] + identity = "Packet identity: " + _identity(self.context.load()) with patch.object(self.provider, "create", wraps=self.provider.create) as create, \ patch.object(self.provider, "message", wraps=self.provider.message) as message: outputs.append(self.run_order("dispatch", context=self.context)) @@ -373,14 +398,13 @@ def test_create_and_message_receive_the_common_evidence_once_each(self): prompt = create.call_args.args[0] self.assertIn(CANARY, prompt) self.assertIn(CONTEXT_CANARY, prompt) - self.assertIn("Packet identity: " + self.result["packet_handle"], prompt) + self.assertIn(identity, prompt) + self.assertNotIn(self.result["packet_handle"], prompt) self.assertNotIn("one@example.invalid", prompt) - self.assertEqual(prompt.split("\n").count("Private evidence for this work item. " - "Packet identity: " + self.result["packet_handle"]), 1) + self.assertEqual(prompt.split("\n").count("Private evidence for this work item. " + identity), 1) outputs.append(self.run_order("dispatch", context=self.context)) self.assertEqual(create.call_count, 1) - with self.assertRaisesRegex(RemoteError, "context_unavailable"): - self.run_order("dispatch") # Required context cannot be dropped on replay. + self.assert_paused(self.run_order("dispatch"), "dispatch") # Required context cannot be dropped on replay. self.assertEqual(create.call_count, 1) outputs.append(self.run_order("clarify", request="c-1", prose=CANARY, context=self.context)) self.assertEqual(outputs[-1]["context"]["message"], "delivered") @@ -398,13 +422,15 @@ def test_create_and_message_receive_the_common_evidence_once_each(self): self.assertNotIn(CONTEXT_CANARY, json.dumps(locked.read())) def test_rejected_dispatch_context_writes_no_reservation(self): - oversized = PacketContext(lambda: (self.context.render()[0], "Packet identity: x\n" + "y" * MAX_INPUT_BYTES)) - for context, error in ((PacketContext(_fail), "context_unavailable"), (None, "context_unavailable"), - (oversized, "context_budget_exceeded"), - (self.bind(order=replace(self.order, issue=908)), "context_unavailable"), - (self.context.render, "context_binding_mismatch")): - with self.subTest(error=error), self.assertRaisesRegex(RemoteError, error): - self.run_order("dispatch", context=context) + with patch.object(self.provider, "create", wraps=self.provider.create) as create: + for context in (PacketContext(_fail), None, self.bind(order=replace(self.order, issue=908))): + self.assert_paused(self.run_order("dispatch", context=context), "dispatch") + with self.assertRaisesRegex(RemoteError, "context_binding_mismatch"): + self.run_order("dispatch", context=self.context.load) + with self.assertRaisesRegex(RemoteError, "context_budget_exceeded"): + self.service.run("dispatch", replace(self.order, body=CANARY + "b" * 47_000), apply=True, + context=self.large()) + self.assertEqual(create.call_count, 0) with self.assertRaisesRegex(RemoteError, "work_order_not_found"): self.run_order("status") # The undispatched order binds neither its body nor its branch. @@ -419,31 +445,46 @@ def test_status_collect_and_cancel_reject_context(self): with self.subTest(command=command), self.assertRaisesRegex(RemoteError, "invalid_request"): self.run_order(command, request="cancel-1", context=self.context) - def test_context_must_be_bound_to_this_work_order(self): + def test_context_must_be_bound_to_this_work_order_and_recipient(self): ticket_b = replace(self.order, issue=908, branch="devin/908") spec_b = {**self.spec, "work_item": "908"} handle_b = fetch(self.fixture.store, "example", spec_b, backend=self.backend, refresh=True)["packet_handle"] valid_b = self.bind(order=ticket_b, handle=handle_b) + packet_b = valid_b.load() self.service.run("dispatch", ticket_b, apply=True, context=valid_b) # Ticket B's own packet is fine. # A wrapper pointed at another order authorizes against that order and fails closed. - with self.assertRaisesRegex(RemoteError, "context_unavailable"): - self.run_order("dispatch", context=self.bind(order=replace(self.order, repository="other/repo"))) + self.assert_paused(self.run_order("dispatch", context=self.bind(order=replace(self.order, repository="other/repo"))), + "dispatch") + # A packet bound to ticket A whose authorized recipients never included devin:builder. + payload = self.context.load().private_payload() + payload["binding"]["recipients"] = ["codex:builder", "claude:reviewer"] + encoded = json.dumps(payload, sort_keys=True).encode() + narrow_packet = ValidatedPacket(encoded, hashlib.sha256(encoded).hexdigest(), "current") + self.assertEqual(narrow_packet.private_payload()["binding"]["work_item"], "907") policy_none = replace(self.order, issue=909, branch="devin/909", context_policy="none") cases = { "packet_for_ticket_b": (self.order, valid_b), - "wrapper_relabelled_for_ticket_a": (self.order, PacketContext(valid_b.render)), - "unbound_callable": (self.order, self.context.render), - "policy_none_order_with_context": (policy_none, PacketContext(self.context.render)), + "wrapper_relabelled_for_ticket_a": (self.order, PacketContext(valid_b.load)), + "ticket_b_packet_returned_directly": (self.order, PacketContext(lambda: packet_b)), + "wrong_recipient": (self.order, PacketContext(lambda: narrow_packet)), + "unbound_callable": (self.order, self.context.load), + "not_a_packet": (self.order, PacketContext(lambda: "Packet identity: forged\n" + CONTEXT_CANARY)), + "policy_none_order_with_context": (policy_none, PacketContext(self.context.load)), } for name, (order, context) in cases.items(): with self.subTest(case=name), patch.object(self.provider, "create", wraps=self.provider.create) as create: with self.assertRaisesRegex(RemoteError, "context_binding_mismatch"): self.service.run("dispatch", order, apply=True, context=context) self.assertEqual(create.call_count, 0) - self.run_order("dispatch", context=self.context) + # Evidence is rendered from the bound packet itself, never supplied alongside it. + with patch.object(self.provider, "create", wraps=self.provider.create) as create: + self.run_order("dispatch", context=self.context) + self.assertIn("Packet identity: " + _identity(self.context.load()), create.call_args.args[0]) + self.assertNotIn(_identity(packet_b), create.call_args.args[0]) with patch.object(self.provider, "message", wraps=self.provider.message) as message: - with self.assertRaisesRegex(RemoteError, "context_binding_mismatch"): - self.run_order("clarify", request="c-1", prose=CANARY, context=PacketContext(valid_b.render)) + for context in (PacketContext(valid_b.load), PacketContext(lambda: narrow_packet)): + with self.assertRaisesRegex(RemoteError, "context_binding_mismatch"): + self.run_order("clarify", request="c-1", prose=CANARY, context=context) self.assertEqual(message.call_count, 0) self.assertEqual(self.run_order("status")["round"], 0) @@ -463,26 +504,26 @@ def test_authorization_is_rechecked_and_fails_closed_without_a_provider_write(se self.setUp() arrange() with patch.object(self.provider, "create", wraps=self.provider.create) as create: - with self.assertRaisesRegex(RemoteError, "context_unavailable"): - self.run_order("dispatch", context=self.context) + self.assert_paused(self.run_order("dispatch", context=self.context), "dispatch") self.assertEqual(create.call_count, 0) + with self.assertRaisesRegex(RemoteError, "work_order_not_found"): + self.run_order("status") self.assertNotIn(CONTEXT_CANARY.encode(), b"".join(p.read_bytes() for p in self.private_files())) self.setUp() self.run_order("dispatch", context=self.context) with patch.object(self.provider, "message", wraps=self.provider.message) as message: for context in (PacketContext(_fail), None): - with self.assertRaisesRegex(RemoteError, "context_unavailable"): - self.run_order("clarify", request="c-1", prose=CANARY, context=context) + self.assert_paused(self.run_order("clarify", request="c-1", prose=CANARY, context=context), "message") self.assertEqual(message.call_count, 0) self.assertEqual(self.run_order("status")["round"], 0) # No local round was consumed. self.assertEqual(self.run_order("clarify", request="c-1", prose=CANARY, context=self.context)["round"], 1) self.backend.revoked = True with patch.object(self.provider, "message", wraps=self.provider.message) as message: - with self.assertRaisesRegex(RemoteError, "context_unavailable"): - self.run_order("clarify", request="c-2", prose=CANARY, context=self.context) + self.assert_paused(self.run_order("clarify", request="c-2", prose=CANARY, context=self.context), "message") self.assertEqual(message.call_count, 0) - self.assertEqual(self.run_order("status")["round"], 1) + status = self.run_order("status") + self.assertEqual((status["round"], status["context"]["message"]), (1, "delivered")) def test_optional_context_degrades_explicitly_and_states_persist(self): unavailable = PacketContext(_fail) @@ -498,7 +539,7 @@ def test_optional_context_degrades_explicitly_and_states_persist(self): handle_b = fetch(self.fixture.store, "example", {**self.spec, "work_item": "908"}, backend=self.backend, refresh=True)["packet_handle"] other = self.bind(order=replace(self.optional_order, issue=908), policy=self.optional_policy, handle=handle_b) - self.run_optional("clarify", request="c-0", prose=CANARY, context=PacketContext(other.render)) + self.run_optional("clarify", request="c-0", prose=CANARY, context=PacketContext(other.load)) with patch.object(self.provider, "message", wraps=self.provider.message) as message: output = self.run_optional("clarify", request="c-1", prose=CANARY, context=None) self.assertEqual((output["context"]["message"], output["round"]), ("omitted", 1)) @@ -517,18 +558,81 @@ def test_optional_context_degrades_explicitly_and_states_persist(self): self.assertEqual((record["context"], record["message"]["context"]), ("degraded", "delivered")) self.assert_not_persisted(output, status) - def test_combined_input_is_bounded(self): - oversized = PacketContext(lambda: (self.context.render()[0], "Packet identity: x\n" + "y" * MAX_INPUT_BYTES)) + def test_recovery_cannot_change_evidence_after_the_local_intent_is_durable(self): + self.run_optional("dispatch", context=self.optional) + with patch.object(self.remote, "run", side_effect=Crash()), self.assertRaises(Crash): + self.run_optional("clarify", request="c-1", prose=CANARY, context=self.optional) + with self.service.store.locked(self.service._key(self.optional_order)) as locked: + record = locked.read() + self.assertEqual((record["round"], record["message"]["pending"], record["message"]["context"]), + (1, True, "delivered")) + with patch.object(self.provider, "message", wraps=self.provider.message) as message: + for context in (None, PacketContext(_fail)): + with self.assertRaisesRegex(RemoteError, "request_conflict"): + self.run_optional("clarify", request="c-1", prose=CANARY, context=context) + fetch(self.fixture.store, "example", self.spec, backend=self.backend, refresh=True) + with self.assertRaisesRegex(RemoteError, "request_conflict"): # Refreshed packet: input differs. + self.run_optional("clarify", request="c-1", prose=CANARY, context=self.optional) + self.assertEqual(message.call_count, 0) + self.result = fetch(self.fixture.store, "example", self.spec, backend=self.backend, refresh=True) + self.backend.result["result"]["results"][0]["text"] = CONTEXT_CANARY + self.assertEqual(self.run_optional("status")["context"]["message"], "delivered") + # Same crash before the remote create intent: the dispatch input is bound too. + self.setUp() + with patch.object(self.remote, "run", side_effect=Crash()), self.assertRaises(Crash): + self.run_optional("dispatch", context=self.optional) + with self.assertRaisesRegex(RemoteError, "request_conflict"): + self.run_optional("dispatch") + output = self.run_optional("dispatch", context=self.optional) + self.assertEqual(output["context"]["dispatch"], "delivered") + + def test_context_free_orders_keep_their_pre_context_binding_and_input(self): + legacy = replace(self.order, context_policy="none") + legacy_fields = {k: v for k, v in asdict(legacy).items() if k != "context_policy"} + self.assertEqual(self.service._binding(legacy), + _hash([legacy_fields, self.provider.name, self.provider.account])) + self.assertIn(json.dumps({k: v for k, v in legacy_fields.items() if k != "body"}, sort_keys=True)[1:-1], + self.service._prompt(legacy)) + self.assertNotIn("context_policy", self.service._prompt(legacy)) + self.assertIn('"context_policy": "required"', self.service._prompt(self.order)) + self.assertNotEqual(self.service._binding(self.order), self.service._binding(legacy)) + self.assertNotEqual(self.service._binding(self.optional_order), self.service._binding(legacy)) with patch.object(self.provider, "create", wraps=self.provider.create) as create: - with self.assertRaisesRegex(RemoteError, "context_budget_exceeded"): - self.run_order("dispatch", context=oversized) - self.assertEqual(create.call_count, 0) + self.service.run("dispatch", legacy, apply=True) + prompt = create.call_args.args[0] + # Simulate a record and remote intent written before the context field existed. + with self.service.store.locked(self.key) as locked: + record = locked.read() + del record["context"], record["input"] + locked.write(record) + for command in ("dispatch", "status"): + output = self.service.run(command, legacy, apply=True) + self.assertEqual(output["context"], {"policy": "none", "dispatch": None, "message": None}) + self.assertEqual(create.call_count, 1) + output = self.service.run("clarify", legacy, apply=True, request="c-1", prose=CANARY) + self.assertEqual((output["round"], output["context"]["message"]), (1, "omitted")) + with self.service.store.locked(self.key) as locked: + record = locked.read() + del record["message"]["input"], record["message"]["context"] + locked.write(record) + output = self.service.run("clarify", legacy, apply=True, request="c-1", prose=CANARY) + self.assertEqual((output["round"], output["context"]["message"]), (1, None)) + self.complete(round=1) + self.assertEqual(self.service.run("collect", legacy, apply=True)["verified_pr"]["pr_number"], 42) + self.service.run("cancel", legacy, apply=True, request="cancel-1") + for order in (self.order, self.optional_order): + with self.assertRaisesRegex(RemoteError, "work_order_binding_mismatch"): + self.service.run("status", order, apply=True) + self.assertNotIn("context_policy", prompt) + + def test_combined_input_is_bounded(self): self.run_order("dispatch", context=self.context) self.complete() self.run_order("collect") + big = self.large() with patch.object(self.provider, "message", wraps=self.provider.message) as message: with self.assertRaisesRegex(RemoteError, "context_budget_exceeded"): - self.run_order("fix", request="fix-1", prose=CANARY, reviewed_head=HEAD, context=oversized) + self.run_order("fix", request="fix-1", prose=CANARY + "p" * 47_000, reviewed_head=HEAD, context=big) self.assertEqual(message.call_count, 0) self.assertEqual(self.run_order("status")["round"], 0) # Oversized input leaves the record unchanged. with self.service.store.locked(self.key) as locked: @@ -537,7 +641,7 @@ def test_combined_input_is_bounded(self): self.assertIsNotNone(record["claim"]) self.assertEqual(record["requests"], []) self.assertEqual(self.run_order("fix", request="fix-1", prose=CANARY, reviewed_head=HEAD, - context=self.context)["round"], 1) + context=big)["round"], 1) if __name__ == "__main__": From 489a460252536f0e49af1690465238b0fcb914d1 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Sat, 12 Sep 2026 17:09:39 +0000 Subject: [PATCH 5/7] Accept and migrate the exact pre-context hosted Devin capability declaration Co-Authored-By: bot_apk --- docs/sessions.md | 5 ++- src/code_mower/provider_capabilities.py | 10 ++++- tests/test_devin_capabilities.py | 51 ++++++++++++++++++++++++- 3 files changed, 63 insertions(+), 3 deletions(-) diff --git a/docs/sessions.md b/docs/sessions.md index 4aaca3ba..14c456d1 100644 --- a/docs/sessions.md +++ b/docs/sessions.md @@ -298,7 +298,10 @@ product/transport declaration fails with instructions to set calibrated repository promotion requires explicit product and transport fields; selection never performs that promotion. Contradictory driver/transport pairs or capability overrides fail validation; remove `capabilities` to use maintained -defaults. Keep `provider: devin_cli` for local execution and `provider: devin` +defaults. The one exception is the exact earlier maintained hosted declaration +(`devin_api_v3` with `context: unavailable`), which earlier templates wrote: +it is read as the current declaration in memory, again without file writes, and +any other deviation still fails. Keep `provider: devin_cli` for local execution and `provider: devin` for hosted compatibility, with `product: devin` in both cases. Devin Cloud needs its own execution setup. Cursor's agent diff --git a/src/code_mower/provider_capabilities.py b/src/code_mower/provider_capabilities.py index c756d358..9c0516c2 100644 --- a/src/code_mower/provider_capabilities.py +++ b/src/code_mower/provider_capabilities.py @@ -73,6 +73,13 @@ def brief(self) -> dict[str, Any]: ), }) +# Earlier maintained declarations, still accepted and migrated in memory to the current ones. +LEGACY_CAPABILITIES = MappingProxyType({ + "devin_api_v3": ( + {**asdict(TRANSPORTS["devin_api_v3"].capabilities), "context": "unavailable"}, + ), +}) + TRANSPORT_ALIASES = MappingProxyType({ "devin": "devin_api_v3", # legacy hosted lane/provider, not the product default "devin_cloud": "devin_api_v3", @@ -116,7 +123,8 @@ def lane_transport(lane_id: str, lane: Mapping[str, Any]) -> ProviderTransport | raise ConfigError("Keep existing Devin lane identities: devin_cli is local; devin is hosted. Select a transport without repurposing its lane.") if lane.get("driver") != transport.driver: raise ConfigError("Devin transport and driver disagree; devin_cli requires local_cli; devin_api_v3 requires hosted_bridge") - if "capabilities" in lane and lane["capabilities"] != asdict(transport.capabilities): + if "capabilities" in lane and lane["capabilities"] != asdict(transport.capabilities) \ + and lane["capabilities"] not in LEGACY_CAPABILITIES.get(transport.transport, ()): raise ConfigError("Devin capabilities must match the declared transport; remove capabilities to use maintained defaults") provider_config = lane.get("provider_config", {}) if isinstance(provider_config, Mapping) and "campaign_transport" in provider_config: diff --git a/tests/test_devin_capabilities.py b/tests/test_devin_capabilities.py index 6f5f3d30..34568b45 100644 --- a/tests/test_devin_capabilities.py +++ b/tests/test_devin_capabilities.py @@ -13,7 +13,7 @@ from code_mower import config, init, participants, session from code_mower.doctor_checks.providers import check_lane_runtime from code_mower.provider_capabilities import ( - TRANSPORTS, lane_transport, normalize_config, normalize_lane, resolve_transport, + LEGACY_CAPABILITIES, TRANSPORTS, lane_transport, normalize_config, normalize_lane, resolve_transport, ) from code_mower.provider_registry import REFERENCE_PROVIDERS from code_mower.package_rendering import _render_yaml @@ -72,6 +72,55 @@ def test_unambiguous_legacy_config_migrates_without_writing(self): self.assertIn("informational", rendered["flags"]) self.assertNotIn("merge-authority", rendered["flags"]) + def test_pre_context_hosted_capabilities_migrate_and_only_that_declaration(self): + current = participants.reference_review_config("devin") + self.assertEqual(current["capabilities"]["context"], "agent_handoff") + legacy = copy.deepcopy(current) + legacy["capabilities"]["context"] = "unavailable" # Exact pre-D5 template declaration. + self.assertEqual(LEGACY_CAPABILITIES, {"devin_api_v3": (legacy["capabilities"],)}) + lane = normalize_lane("devin", legacy) + self.assertEqual(lane["capabilities"], current["capabilities"]) + self.assertEqual((lane["transport"], lane["driver"]), ("devin_api_v3", "hosted_bridge")) + self.assertFalse(lane["merge_authority"]) + source = config.load_config(STARTER) + source["lanes"]["devin"] = legacy + with tempfile.TemporaryDirectory() as tmp: + path = Path(tmp) / "code-mower.yml" + path.write_text("\n".join(_render_yaml(source)) + "\n") + before = path.read_bytes() + loaded = config.load_config(path) + self.assertEqual(path.read_bytes(), before) + self.assertEqual(config.validate_config(loaded), []) + self.assertEqual(loaded["lanes"]["devin"]["capabilities"]["context"], "unavailable") + self.assertEqual(normalize_config(loaded)["lanes"]["devin"]["capabilities"], current["capabilities"]) + rendered = config.render_dry_run(loaded).data["lanes"]["devin"] + self.assertEqual(rendered["transport"], "devin_api_v3") + brief = session.build_session(repo="owner/repo", host="codex", selected=("devin_api_v3",), config=loaded) + self.assertEqual(brief["participants"][0]["execution"], TRANSPORTS["devin_api_v3"].brief()) + self.assertIn("context=agent_handoff", session.render_session(brief)) + self.assertEqual(participants.configured_transports(loaded), participants.configured_transports( + {**loaded, "lanes": {**loaded["lanes"], "devin": current}})) + for change in ( + {"context": "local_runner"}, {"context": "unavailable", "review": "agent_handoff"}, + {"context": "unavailable", "message": "agent_handoff"}, {"context": "unavailable", "extra": "x"}, + {"context": "unavailable", "coordinate": "agent_handoff"}, + ): + drifted = copy.deepcopy(current) + drifted["capabilities"] = {**current["capabilities"], **change} + with self.subTest(change=change), self.assertRaisesRegex(config.ConfigError, "capabilities must match"): + normalize_lane("devin", drifted) + missing = copy.deepcopy(legacy) + del missing["capabilities"]["structured_results"] + with self.assertRaisesRegex(config.ConfigError, "capabilities must match"): + normalize_lane("devin", missing) + # The local transport never had another maintained declaration. + local = participants.reference_review_config("devin_cli") + local["capabilities"]["context"] = "agent_handoff" + with self.assertRaisesRegex(config.ConfigError, "capabilities must match"): + normalize_lane("devin_cli", local) + with self.assertRaisesRegex(config.ConfigError, "lane identities"): + normalize_lane("devin_cli", legacy) + def test_shipped_root_config_loads_without_semantic_normalization(self): loaded = config.load_config(ROOT / "code-mower.yml") self.assertIn(loaded["version"], (1, "1")) From bd3829b4702b251784c82f2f78dee2e679272202 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Sat, 12 Sep 2026 17:30:49 +0000 Subject: [PATCH 6/7] Load hosted Devin context inside the work-order boundary; bind a tracker-neutral context work item PacketContext now carries only the protected store, connection name, packet handle, normalized policy, and backend; _evidence() calls load_authorized() with a request derived from the order and renders under the exact authorized handle, the same identity peer Claude/Codex delivery renders. WorkOrder gains context_work_item for context-bearing orders (a Jira key may differ from the integer GitHub issue used for PR verification); context-free serialization and legacy recovery are unchanged. Co-Authored-By: bot_apk --- docs/context-delivery.md | 46 +++-- src/code_mower/devin_work_orders.py | 98 +++++++---- tests/test_devin_work_orders.py | 262 +++++++++++++++++++++------- 3 files changed, 293 insertions(+), 113 deletions(-) diff --git a/docs/context-delivery.md b/docs/context-delivery.md index ef817585..666ddb5b 100644 --- a/docs/context-delivery.md +++ b/docs/context-delivery.md @@ -80,22 +80,35 @@ logs, tracked files, PR descriptions, and shared artifacts. A trusted hosted work order can carry the packet to `devin:builder` without a local prompt file. The trusted context decision is part of the work order: `WorkOrder.context_policy` is `none`, `optional`, or `required`, comes from -dispatcher policy, and is included in the durable binding. The embedding binds -one packet with `devin_work_orders.packet_context(store, name, handle, policy, -order=order)`, which derives the packet request from the order's repository and -issue number, requires the trusted policy's `required` flag to agree with the -order, and returns a `PacketContext` holding only a packet-loading closure. That -value -is passed as `context=` to `dispatch`, `clarify`, or `fix`. +dispatcher policy, and is included in the durable binding. A context-bearing +order also carries the trusted `WorkOrder.context_work_item`: the tracker-neutral +identity from the session or manifest context binding (a Jira key, for example), +which may differ from the integer GitHub delivery issue `WorkOrder.issue`. Only +the integer issue closes and verifies the pull request; only the work item binds +the packet. When a key is supplied the prepared source may omit `issue_number`, +though a present one must still match. Context-free orders carry no key, and +their serialized binding is unchanged. + +The embedding binds one packet with `devin_work_orders.packet_context(store, +name, handle, policy, order=order)`, which validates the handle and the trusted +policy (its `required` flag must agree with the order) and returns a +`PacketContext` holding only the protected store, connection name, packet +handle, normalized policy, and backend. It carries no packet, evidence, callable, +or identity of its own: the request is derived from the order it is used with. +That value is passed as `context=` to `dispatch`, `clarify`, or `fix`. Immediately before each paid create or message write, and never in preview, the -context performs a new online authorization for `devin:builder`, and the loaded -packet's own binding is compared with the order: its repository and work item -must match and `devin:builder` must be among its bound recipients. The evidence -is then rendered inside the work-order boundary from that exact validated -packet, with an identity derived from the packet digest; no separately supplied -evidence text is ever accepted. A packet for another ticket, a packet without -the recipient, a relabelled wrapper, a bare callable or string, or any context +work-order boundary itself calls `load_authorized()` with the order's repository, +work item, and `devin:builder`: a new online authorization under the store lock +for the selected account, current authorization, revocation and expiry, then the +packet's own binding, freshness, and recipients. The evidence is rendered inside +that boundary from the exact packet the authorized load returned, under the +exact authorized packet handle as its `Packet identity`, the same identity the +Claude and Codex peer paths render; no separately supplied packet or evidence is +ever accepted, and a synthetic local packet with matching binding fields never +reaches the store lookup. A handle for another ticket, a packet without the +recipient, or a store without an authorizable connection is `unavailable`; a +bare handle, packet, string, look-alike object, store subclass, or any context on a `none` order fails as `context_binding_mismatch`. Rendering and the 64 KiB combined-size check (`context_budget_exceeded`) happen before the work-order record, branch reservation, or a new round is written, so a rejected input @@ -123,8 +136,9 @@ for each message intent is persisted and reported by every command as `context: {policy, dispatch, message}`; replay, acknowledgement, status, and collect return the saved state rather than recomputing it. Durable records written before the context field existed keep their original binding and -dispatch input: a `none` order serializes without `context_policy`, and records -without context or input digests are read as context-free. +dispatch input: a `none` order serializes without `context_policy` or +`context_work_item`, and records without context or input digests are read as +context-free. ## Attach evidence to independent review diff --git a/src/code_mower/devin_work_orders.py b/src/code_mower/devin_work_orders.py index 4630938f..d248bc11 100644 --- a/src/code_mower/devin_work_orders.py +++ b/src/code_mower/devin_work_orders.py @@ -9,14 +9,13 @@ import json import math import re -import uuid from dataclasses import asdict, dataclass, field from pathlib import Path -from typing import Callable, Protocol +from typing import Protocol -from .context_contract import ContextError, ContextRequest, ValidatedPacket, normalize_policy +from .context_contract import ContextError, ContextRequest, ValidatedPacket, _text, normalize_policy from .context_delivery import render_evidence -from .context_packets import load_authorized +from .context_packets import _handle, load_authorized from .context_store import ContextStore from .devin_sessions import REPO, DevinClient from .provider_capabilities import resolve_transport @@ -58,6 +57,13 @@ def _branch(value) -> bool: for p in value.split("/")) and not value.endswith("/")) +def _work_item(value) -> bool: + try: + return _text(value, maximum=128) == value.strip() + except ContextError: + return False + + def _positive(value) -> bool: return type(value) is int and 0 < value <= 2**53 - 1 @@ -79,19 +85,24 @@ class WorkOrder: acu_limit: int body: str = field(repr=False) context_policy: str = "none" + context_work_item: str = "" # Tracker-neutral packet work item; defaults to the GitHub issue. @classmethod def from_manifest(cls, manifest: dict, body: str, *, repository: str, issue: int, branch: str, base: str, author_id: int, author_login: str, - acu_limit: int = 10, context_policy: str = "none") -> WorkOrder: + acu_limit: int = 10, context_policy: str = "none", + context_work_item: str = "") -> WorkOrder: source = manifest.get("source", {}) + # A tracker-keyed (context-bearing) source may omit the GitHub delivery issue, + # which then comes from dispatcher policy alone; a present one must still match. + issue_number = source.get("issue_number") if isinstance(source, dict) else None if (manifest.get("schema") != WORK_ORDER_SCHEMA or manifest.get("repo") != repository or not isinstance(source, dict) or source.get("repo") != repository - or str(source.get("issue_number")) != str(issue)): + or (str(issue_number) != str(issue) and not (context_work_item and not issue_number))): raise RemoteError("work_order_binding_mismatch") return cls(repository, issue, branch, base, author_id, author_login, acu_limit, body, - context_policy) + context_policy, context_work_item) def __post_init__(self): if (not isinstance(self.repository, str) or len(self.repository) > 256 @@ -101,9 +112,16 @@ def __post_init__(self): or not LOGIN.fullmatch(self.author_login) or type(self.acu_limit) is not int or not 1 <= self.acu_limit <= 100 or not isinstance(self.body, str) or not self.body.strip() - or len(self.body.encode()) > 48000 or self.context_policy not in CONTEXT_POLICIES): + or len(self.body.encode()) > 48000 or self.context_policy not in CONTEXT_POLICIES + or not isinstance(self.context_work_item, str) + or (self.context_work_item and (self.context_policy == "none" or not _work_item(self.context_work_item)))): raise RemoteError("invalid_work_order") + @property + def work_item(self) -> str: + """The packet work-item identity: the tracker key when bound, else the GitHub issue.""" + return self.context_work_item or str(self.issue) + @dataclass(frozen=True, repr=False) class PullRequest: @@ -144,40 +162,42 @@ def read(self, repository: str, number: int) -> PullRequest: ... @dataclass(frozen=True, repr=False) class PacketContext: - """Loads one authorized packet; carries no caller-editable identity, policy, or text. - - The trusted context decision lives on ``WorkOrder.context_policy``. The loaded - packet's own repository, work-item, and recipient binding is checked against - the order and the evidence is rendered from that exact packet inside the - work-order boundary, so this wrapper can supply neither another ticket's - packet nor separately authored evidence. + """Names one packet in a protected store; carries no packet, evidence, or identity. + + The trusted context decision lives on ``WorkOrder.context_policy``. The + work-order boundary itself performs ``load_authorized`` for these inputs + with a request derived from the order, checks the loaded packet's own + repository, work-item, and recipient binding, and renders the evidence + from that exact packet under the authorized handle, so a caller can supply + neither a synthetic packet, another ticket's packet, nor authored evidence. """ - load: Callable[[], ValidatedPacket] = field(repr=False) + store: ContextStore = field(repr=False) + name: str = field(repr=False) + handle: str = field(repr=False) + policy: dict = field(repr=False) + backend: object = field(default=None, repr=False) def packet_context(store: ContextStore, name: str, handle: str, policy, *, order: WorkOrder, backend=None) -> PacketContext: """Bind one authorized packet to the hosted builder before its PR exists. - The packet request is derived from the work order (repository and issue - number as the work item), never from caller-supplied identity, and the + The packet request is derived from the work order (repository and its + tracker-neutral ``work_item``), never from caller-supplied identity, and the trusted policy's ``required`` flag must agree with the order's declared context policy. Each render performs a new online authorization for ``devin:builder``; nothing is cached or written. """ try: normalized = normalize_policy(policy) + _handle(handle) except ContextError: raise RemoteError("invalid_request") from None - if normalized is None or order.context_policy == "none" \ - or normalized["required"] != (order.context_policy == "required"): + if (normalized is None or order.context_policy == "none" + or normalized["required"] != (order.context_policy == "required") + or type(store) is not ContextStore or not isinstance(name, str)): raise RemoteError("invalid_request") - request = ContextRequest(order.repository, str(order.issue), CONTEXT_RECIPIENT) - - def load() -> ValidatedPacket: - return load_authorized(store, name, handle, policy, request, backend=backend) - - return PacketContext(load) + return PacketContext(store, name, handle, normalized, backend) def _github_call(method, *args, **kwargs): @@ -210,6 +230,8 @@ def _fields(order): fields = asdict(order) if fields["context_policy"] == "none": del fields["context_policy"] + if not fields["context_work_item"]: + del fields["context_work_item"] return fields def _binding(self, order): @@ -235,12 +257,13 @@ def _prompt(cls, order, round_number=0): def _evidence(order, context): """Render freshly reauthorized evidence for exactly one create/message input. - ``context`` loads the authorized packet for ``devin:builder`` through the - ordinary online authorization path, immediately before the paid provider - write and never in preview. The packet's embedded binding must name this - order's repository and work item and the ``devin:builder`` recipient; the - evidence is then rendered here from that exact packet, with an identity - derived from the packet digest, and is never persisted. The order's + The packet named by ``context`` is loaded here through ``load_authorized`` + for ``devin:builder`` with a request derived from the order, immediately + before the paid provider write and never in preview. The packet's + embedded binding must name this order's repository and work item and the + ``devin:builder`` recipient; the evidence is then rendered here from that + exact packet under the authorized handle, the same packet identity peer + Claude/Codex delivery renders, and is never persisted. The order's trusted ``context_policy`` decides the outcome: required context fails closed (``context_unavailable``), optional context degrades to code-only, and a missing packet is ``omitted`` only when the policy allows it. @@ -251,10 +274,13 @@ def _evidence(order, context): if required: raise RemoteError("context_unavailable") return None, "omitted" - if order.context_policy == "none" or not isinstance(context, PacketContext): + if (order.context_policy == "none" or type(context) is not PacketContext + or type(context.store) is not ContextStore): raise RemoteError("context_binding_mismatch") + request = ContextRequest(order.repository, order.work_item, CONTEXT_RECIPIENT) try: - packet = context.load() + packet = load_authorized(context.store, context.name, context.handle, context.policy, + request, backend=context.backend) if not isinstance(packet, ValidatedPacket): raise RemoteError("context_binding_mismatch") binding = packet.private_payload().get("binding") @@ -262,10 +288,10 @@ def _evidence(order, context): raise RemoteError("context_binding_mismatch") recipients = binding.get("recipients") if (binding.get("repository") != order.repository - or binding.get("work_item") != str(order.issue) + or binding.get("work_item") != order.work_item or not isinstance(recipients, list) or CONTEXT_RECIPIENT not in recipients): raise RemoteError("context_binding_mismatch") - evidence = render_evidence(packet, uuid.UUID(hex=packet.sha256[:32]).hex) + evidence = render_evidence(packet, context.handle) except ContextError: if required: raise RemoteError("context_unavailable") from None diff --git a/tests/test_devin_work_orders.py b/tests/test_devin_work_orders.py index 94815b42..24c66239 100644 --- a/tests/test_devin_work_orders.py +++ b/tests/test_devin_work_orders.py @@ -4,6 +4,7 @@ import os import sys import tempfile +import time import unittest import uuid from dataclasses import asdict, replace @@ -12,11 +13,13 @@ sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "src")) -from code_mower.context_contract import ContextError, ValidatedPacket -from code_mower.context_packets import fetch +from code_mower.context_contract import ContextRequest +from code_mower.context_connections import connect, disconnect +from code_mower.context_packets import fetch, load_authorized +from code_mower.context_store import ContextStore from code_mower.devin_sessions import DevinClient from code_mower.devin_work_orders import ( - COMPLETION_SCHEMA, Candidates, DevinWorkOrders, PacketContext, PullRequest, WorkOrder, _hash, packet_context, + COMPLETION_SCHEMA, Candidates, DevinWorkOrders, PullRequest, WorkOrder, _hash, packet_context, ) from code_mower.remote_session import FakeProvider, RemoteError, RemoteSessions, _key from code_mower.work_orders import WORK_ORDER_SCHEMA @@ -304,12 +307,20 @@ class Crash(Exception): PAUSED = {"outcome": "UNKNOWN", "state": "paused", "reason": "context_unavailable"} -def _fail(): - raise ContextError("context search unavailable") +def _expire(proof): + proof.expires_at = int(time.time()) - 1 + return proof -def _identity(packet): - return uuid.UUID(hex=packet.sha256[:32]).hex +def _packet_of(context, order): + """Test-only view of the authorized packet a bound context names for this order.""" + return load_authorized(context.store, context.name, context.handle, context.policy, + ContextRequest(order.repository, order.work_item, "devin:builder"), + backend=context.backend) + + +class Unprotected(ContextStore): + """A store subclass a caller might substitute; never accepted at the work-order boundary.""" @unittest.skipUnless(os.name == "posix", "private packets need POSIX protections") @@ -321,6 +332,9 @@ def setUp(self): self.order = replace(self.order, context_policy="required") self.key = self.service._key(self.order) self.optional_order = replace(self.order, context_policy="optional") + outside = tempfile.TemporaryDirectory() + self.addCleanup(outside.cleanup) + self.outside = Path(outside.name).resolve() self.fixture = fixtures.ContextDeliveryTests(methodName="runTest") self.fixture.setUp() self.addCleanup(self.fixture.doCleanups) @@ -337,6 +351,41 @@ def bind(self, order=None, policy=None, handle=None): return packet_context(self.fixture.store, "example", handle or self.result["packet_handle"], policy or self.policy, order=order or self.order, backend=self.backend) + def synthetic(self, *, text=None, recipients=None, order=None, handle=None): + """A local packet outside the protected store with this order's own binding fields.""" + order = order or self.order + context = self.context if handle is None else replace(self.context, handle=handle) + payload = _packet_of(context, order).private_payload() + if recipients is not None: + payload["binding"]["recipients"] = recipients + if text is not None: + payload["documents"][0]["text"] = text + root = Path(tempfile.mkdtemp(dir=self.outside)) + store = ContextStore(root) + with self.fixture.store.locked("example") as locked: + index = locked.artifact("i-" + hashlib.sha256(b"example").hexdigest()[:48]).read() + with store.locked("example") as locked: + for entry in index["entries"]: + entry["reference"] = None + raw = json.dumps(payload, sort_keys=True).encode() + entry = next(e for e in index["entries"] if e["handle"] == context.handle) + entry["reference"] = {"path": ".p-" + entry["handle"] + ".json", "sha256": hashlib.sha256(raw).hexdigest()} + (root / entry["reference"]["path"]).write_bytes(raw) + locked.artifact("i-" + hashlib.sha256(b"example").hexdigest()[:48]).write(index) + self.assertEqual(payload["binding"]["repository"], order.repository) + self.assertEqual(payload["binding"]["work_item"], order.work_item) + return replace(context, store=store) + + def reconnect(self, recipients): + disconnect(self.fixture.store, "example", backend=self.backend) + connect(self.fixture.store, "example", {"principal": "one@example.invalid", "workspace": "example", + "repositories": ["owner/repo"], "recipients": recipients}, backend=self.backend) + + def unavailable(self): + """A bound context whose store no longer holds an authorizable connection.""" + empty = ContextStore(Path(tempfile.mkdtemp(dir=self.outside))) + return replace(self.context, store=empty) + def run_optional(self, command, **kwargs): return self.service.run(command, self.optional_order, apply=True, **kwargs) @@ -352,8 +401,7 @@ def private_files(self): def assert_not_persisted(self, *outputs): serialized = json.dumps(outputs) - for private in (CONTEXT_CANARY, "Packet identity", "one@example.invalid", self.result["packet_handle"], - _identity(self.context.load())): + for private in (CONTEXT_CANARY, "Packet identity", "one@example.invalid", self.result["packet_handle"]): self.assertNotIn(private, serialized) for path in self.private_files(): self.assertNotIn(CONTEXT_CANARY.encode(), path.read_bytes()) @@ -365,8 +413,17 @@ def assert_paused(self, output, slot, policy="required"): self.assertNotIn("session", output) def test_packet_context_carries_no_identity_and_policy_must_agree_with_the_order(self): - self.assertEqual(tuple(self.context.__dataclass_fields__), ("load",)) + self.assertEqual(tuple(self.context.__dataclass_fields__), ("store", "name", "handle", "policy", "backend")) self.assertNotIn(CONTEXT_CANARY, repr(self.context)) + self.assertNotIn(self.result["packet_handle"], repr(self.context)) + for handle in ("", "not-a-handle", self.result["packet_handle"].upper(), None): + with self.subTest(handle=handle), self.assertRaisesRegex(RemoteError, "invalid_request"): + packet_context(self.fixture.store, "example", handle, self.policy, order=self.order, + backend=self.backend) + for store in (self.fixture.store.root, Unprotected(self.fixture.store.root)): + with self.subTest(store=type(store).__name__), self.assertRaisesRegex(RemoteError, "invalid_request"): + packet_context(store, "example", self.result["packet_handle"], self.policy, + order=self.order, backend=self.backend) with self.assertRaises(RemoteError): replace(self.order, context_policy="always") for order, policy in ((self.order, self.optional_policy), (self.optional_order, self.policy), @@ -377,20 +434,16 @@ def test_packet_context_carries_no_identity_and_policy_must_agree_with_the_order order=order, backend=self.backend) def test_preview_never_retrieves_context(self): - calls = [] - def spy(): - calls.append(1) - return self.context.load() - context = PacketContext(spy) - for command in ("dispatch", "clarify", "fix"): - output = self.service.run(command, self.order, context=context, request="r", prose="p") - self.assertTrue(output["apply_required"]) - self.assertEqual(calls, []) + with patch("code_mower.devin_work_orders.load_authorized", wraps=load_authorized) as load: + for command in ("dispatch", "clarify", "fix"): + output = self.service.run(command, self.order, context=self.context, request="r", prose="p") + self.assertTrue(output["apply_required"]) + self.assertEqual(load.call_count, 0) self.assertFalse((self.root / "builder").exists()) def test_create_and_message_receive_the_common_evidence_once_each(self): outputs = [] - identity = "Packet identity: " + _identity(self.context.load()) + identity = "Packet identity: " + self.result["packet_handle"] with patch.object(self.provider, "create", wraps=self.provider.create) as create, \ patch.object(self.provider, "message", wraps=self.provider.message) as message: outputs.append(self.run_order("dispatch", context=self.context)) @@ -399,7 +452,6 @@ def test_create_and_message_receive_the_common_evidence_once_each(self): self.assertIn(CANARY, prompt) self.assertIn(CONTEXT_CANARY, prompt) self.assertIn(identity, prompt) - self.assertNotIn(self.result["packet_handle"], prompt) self.assertNotIn("one@example.invalid", prompt) self.assertEqual(prompt.split("\n").count("Private evidence for this work item. " + identity), 1) outputs.append(self.run_order("dispatch", context=self.context)) @@ -423,10 +475,10 @@ def test_create_and_message_receive_the_common_evidence_once_each(self): def test_rejected_dispatch_context_writes_no_reservation(self): with patch.object(self.provider, "create", wraps=self.provider.create) as create: - for context in (PacketContext(_fail), None, self.bind(order=replace(self.order, issue=908))): + for context in (self.unavailable(), None, self.bind(handle=uuid.uuid4().hex)): self.assert_paused(self.run_order("dispatch", context=context), "dispatch") with self.assertRaisesRegex(RemoteError, "context_binding_mismatch"): - self.run_order("dispatch", context=self.context.load) + self.run_order("dispatch", context=self.result["packet_handle"]) with self.assertRaisesRegex(RemoteError, "context_budget_exceeded"): self.service.run("dispatch", replace(self.order, body=CANARY + "b" * 47_000), apply=True, context=self.large()) @@ -450,41 +502,44 @@ def test_context_must_be_bound_to_this_work_order_and_recipient(self): spec_b = {**self.spec, "work_item": "908"} handle_b = fetch(self.fixture.store, "example", spec_b, backend=self.backend, refresh=True)["packet_handle"] valid_b = self.bind(order=ticket_b, handle=handle_b) - packet_b = valid_b.load() self.service.run("dispatch", ticket_b, apply=True, context=valid_b) # Ticket B's own packet is fine. - # A wrapper pointed at another order authorizes against that order and fails closed. - self.assert_paused(self.run_order("dispatch", context=self.bind(order=replace(self.order, repository="other/repo"))), - "dispatch") - # A packet bound to ticket A whose authorized recipients never included devin:builder. - payload = self.context.load().private_payload() - payload["binding"]["recipients"] = ["codex:builder", "claude:reviewer"] - encoded = json.dumps(payload, sort_keys=True).encode() - narrow_packet = ValidatedPacket(encoded, hashlib.sha256(encoded).hexdigest(), "current") - self.assertEqual(narrow_packet.private_payload()["binding"]["work_item"], "907") + # The context names a packet only; the order it is used with supplies the binding. + self.assertEqual(self.bind(order=replace(self.order, repository="other/repo")), self.context) policy_none = replace(self.order, issue=909, branch="devin/909", context_policy="none") - cases = { - "packet_for_ticket_b": (self.order, valid_b), - "wrapper_relabelled_for_ticket_a": (self.order, PacketContext(valid_b.load)), - "ticket_b_packet_returned_directly": (self.order, PacketContext(lambda: packet_b)), - "wrong_recipient": (self.order, PacketContext(lambda: narrow_packet)), - "unbound_callable": (self.order, self.context.load), - "not_a_packet": (self.order, PacketContext(lambda: "Packet identity: forged\n" + CONTEXT_CANARY)), - "policy_none_order_with_context": (policy_none, PacketContext(self.context.load)), + paused = { + "packet_for_ticket_b": (self.order, valid_b), # Ticket B's handle does not authorize for A. + "wrong_recipient": (self.order, self.synthetic(recipients=["codex:builder", "claude:reviewer"])), + "synthetic_same_binding_changed_text": (self.order, self.synthetic(text="forged " + CONTEXT_CANARY)), + "synthetic_verbatim_copy": (self.order, self.synthetic()), } - for name, (order, context) in cases.items(): + for name, (order, context) in paused.items(): + with self.subTest(case=name), patch.object(self.provider, "create", wraps=self.provider.create) as create: + self.assert_paused(self.service.run("dispatch", order, apply=True, context=context), "dispatch") + self.assertEqual(create.call_count, 0) + mismatched = { + "bare_handle": (self.order, self.result["packet_handle"]), + "bare_packet": (self.order, _packet_of(self.context, self.order)), + "evidence_text": (self.order, "Packet identity: forged\n" + CONTEXT_CANARY), + "look_alike": (self.order, type("PacketContext", (), dict(asdict(self.context)))()), + "store_subclass": (self.order, replace(self.context, store=Unprotected(self.fixture.store.root))), + "policy_none_order_with_context": (policy_none, self.context), + } + for name, (order, context) in mismatched.items(): with self.subTest(case=name), patch.object(self.provider, "create", wraps=self.provider.create) as create: with self.assertRaisesRegex(RemoteError, "context_binding_mismatch"): self.service.run("dispatch", order, apply=True, context=context) self.assertEqual(create.call_count, 0) - # Evidence is rendered from the bound packet itself, never supplied alongside it. + with self.assertRaisesRegex(RemoteError, "work_order_not_found"): + self.run_order("status") + # Evidence is rendered from the authorized packet itself under its own handle. with patch.object(self.provider, "create", wraps=self.provider.create) as create: self.run_order("dispatch", context=self.context) - self.assertIn("Packet identity: " + _identity(self.context.load()), create.call_args.args[0]) - self.assertNotIn(_identity(packet_b), create.call_args.args[0]) + self.assertIn("Packet identity: " + self.result["packet_handle"], create.call_args.args[0]) + self.assertNotIn(handle_b, create.call_args.args[0]) + self.assertNotIn("forged", create.call_args.args[0]) with patch.object(self.provider, "message", wraps=self.provider.message) as message: - for context in (PacketContext(valid_b.load), PacketContext(lambda: narrow_packet)): - with self.assertRaisesRegex(RemoteError, "context_binding_mismatch"): - self.run_order("clarify", request="c-1", prose=CANARY, context=context) + for context in (valid_b, self.synthetic(recipients=["codex:builder"])): + self.assert_paused(self.run_order("clarify", request="c-1", prose=CANARY, context=context), "message") self.assertEqual(message.call_count, 0) self.assertEqual(self.run_order("status")["round"], 0) @@ -497,8 +552,12 @@ def test_authorization_is_rechecked_and_fails_closed_without_a_provider_write(se backend=self.backend, refresh=True)["packet_handle"])), "changed_evidence": lambda: fetch(self.fixture.store, "example", self.spec, backend=self.backend, refresh=True), - "garbage": lambda: setattr(self, "context", PacketContext(_fail)), + "expired": lambda: setattr(self.backend, "proof", lambda expected, sequence: _expire(proof(expected, sequence))), + "changed_recipients": lambda: self.reconnect(["codex:builder", "claude:reviewer"]), + "synthetic_packet": lambda: setattr(self, "context", self.synthetic(text="forged " + CONTEXT_CANARY)), + "store_without_connection": lambda: setattr(self, "context", self.unavailable()), } + proof = self.backend.proof for name, arrange in cases.items(): with self.subTest(case=name): self.setUp() @@ -512,7 +571,7 @@ def test_authorization_is_rechecked_and_fails_closed_without_a_provider_write(se self.setUp() self.run_order("dispatch", context=self.context) with patch.object(self.provider, "message", wraps=self.provider.message) as message: - for context in (PacketContext(_fail), None): + for context in (self.unavailable(), None): self.assert_paused(self.run_order("clarify", request="c-1", prose=CANARY, context=context), "message") self.assertEqual(message.call_count, 0) self.assertEqual(self.run_order("status")["round"], 0) # No local round was consumed. @@ -526,7 +585,7 @@ def test_authorization_is_rechecked_and_fails_closed_without_a_provider_write(se self.assertEqual((status["round"], status["context"]["message"]), (1, "delivered")) def test_optional_context_degrades_explicitly_and_states_persist(self): - unavailable = PacketContext(_fail) + unavailable = self.unavailable() with patch.object(self.provider, "create", wraps=self.provider.create) as create: output = self.run_optional("dispatch", context=unavailable) self.assertEqual(output["context"], {"policy": "optional", "dispatch": "degraded", "message": None}) @@ -535,16 +594,20 @@ def test_optional_context_degrades_explicitly_and_states_persist(self): with self.assertRaisesRegex(RemoteError, "request_conflict"): self.run_optional("dispatch", context=self.optional) # Input changed. self.assertEqual(self.run_optional("dispatch", context=unavailable)["context"]["dispatch"], "degraded") - with self.assertRaisesRegex(RemoteError, "context_binding_mismatch"): # Optional never relaxes binding. - handle_b = fetch(self.fixture.store, "example", {**self.spec, "work_item": "908"}, - backend=self.backend, refresh=True)["packet_handle"] - other = self.bind(order=replace(self.optional_order, issue=908), policy=self.optional_policy, handle=handle_b) - self.run_optional("clarify", request="c-0", prose=CANARY, context=PacketContext(other.load)) + handle_b = fetch(self.fixture.store, "example", {**self.spec, "work_item": "908"}, + backend=self.backend, refresh=True)["packet_handle"] + with patch.object(self.provider, "message", wraps=self.provider.message) as message: + # Optional never relaxes binding: another ticket's packet degrades to code-only, not delivery. + output = self.run_optional("clarify", request="c-0", prose=CANARY, + context=replace(self.optional, handle=handle_b)) + self.assertEqual(output["context"]["message"], "degraded") + self.assertNotIn(CONTEXT_CANARY, message.call_args.args[1]) + self.assertNotIn("Packet identity", message.call_args.args[1]) with patch.object(self.provider, "message", wraps=self.provider.message) as message: output = self.run_optional("clarify", request="c-1", prose=CANARY, context=None) - self.assertEqual((output["context"]["message"], output["round"]), ("omitted", 1)) + self.assertEqual((output["context"]["message"], output["round"]), ("omitted", 2)) output = self.run_optional("clarify", request="c-2", prose=CANARY, context=self.optional) - self.assertEqual((output["context"]["message"], output["round"]), ("delivered", 2)) + self.assertEqual((output["context"]["message"], output["round"]), ("delivered", 3)) self.assertIn(CONTEXT_CANARY, message.call_args.args[1]) # Acknowledging a delivered message preserves its saved state instead of "omitted". output = self.run_optional("clarify", request="c-2", prose=CANARY, acknowledge_delivered=True) @@ -552,7 +615,7 @@ def test_optional_context_degrades_explicitly_and_states_persist(self): self.assertEqual(message.call_count, 2) status = self.run_optional("status") self.assertEqual(status["context"], {"policy": "optional", "dispatch": "degraded", "message": "delivered"}) - self.assertEqual(status["round"], 2) + self.assertEqual(status["round"], 3) with self.service.store.locked(self.service._key(self.optional_order)) as locked: record = locked.read() self.assertEqual((record["context"], record["message"]["context"]), ("degraded", "delivered")) @@ -567,7 +630,7 @@ def test_recovery_cannot_change_evidence_after_the_local_intent_is_durable(self) self.assertEqual((record["round"], record["message"]["pending"], record["message"]["context"]), (1, True, "delivered")) with patch.object(self.provider, "message", wraps=self.provider.message) as message: - for context in (None, PacketContext(_fail)): + for context in (None, self.unavailable()): with self.assertRaisesRegex(RemoteError, "request_conflict"): self.run_optional("clarify", request="c-1", prose=CANARY, context=context) fetch(self.fixture.store, "example", self.spec, backend=self.backend, refresh=True) @@ -586,9 +649,86 @@ def test_recovery_cannot_change_evidence_after_the_local_intent_is_durable(self) output = self.run_optional("dispatch", context=self.optional) self.assertEqual(output["context"]["dispatch"], "delivered") + def test_tracker_key_binds_context_while_the_github_issue_binds_the_pull_request(self): + """A guided Jira session keeps its own work item; the delivery issue stays the integer one.""" + manifest = {"schema": WORK_ORDER_SCHEMA, "repo": "owner/repo", "source": {"repo": "owner/repo"}, + "output_path": CANARY, "context_manifest": CANARY} + common = dict(repository="owner/repo", issue=907, branch="devin/907", base="main", + author_id=123, author_login="builder[bot]", acu_limit=5, context_policy="required") + with self.assertRaisesRegex(RemoteError, "work_order_binding_mismatch"): # No key: the issue is required. + WorkOrder.from_manifest(manifest, CANARY, **common) + with self.assertRaisesRegex(RemoteError, "work_order_binding_mismatch"): # A present issue must still match. + WorkOrder.from_manifest({**manifest, "source": {"repo": "owner/repo", "issue_number": "908"}}, + CANARY, **common, context_work_item="EXAMPLE-1") + for bad in ("", " EXAMPLE-1", "EXAMPLE\n1", "x" * 129, 907): + with self.subTest(work_item=bad), self.assertRaisesRegex(RemoteError, "invalid_work_order|binding_mismatch"): + WorkOrder.from_manifest(manifest, CANARY, **common, context_work_item=bad) + with self.assertRaisesRegex(RemoteError, "invalid_work_order"): # Context-free orders carry no key. + replace(self.order, context_policy="none", context_work_item="EXAMPLE-1") + self.order = WorkOrder.from_manifest(manifest, CANARY, **common, context_work_item="EXAMPLE-1") + self.key = self.service._key(self.order) + self.assertEqual((self.order.issue, self.order.work_item), (907, "EXAMPLE-1")) + self.assertEqual(self.service._fields(self.order)["context_work_item"], "EXAMPLE-1") + self.assertNotEqual(self.service._binding(self.order), + self.service._binding(replace(self.order, context_work_item="EXAMPLE-2"))) + # The GitHub issue's packet is not this order's context; the Jira packet is. + github_packet = self.bind() + jira_handle = self.fixture.result["packet_handle"] + self.assertNotEqual(jira_handle, self.result["packet_handle"]) + jira = self.bind(handle=jira_handle) + with patch.object(self.provider, "create", wraps=self.provider.create) as create: + self.assert_paused(self.run_order("dispatch", context=github_packet), "dispatch") + self.assert_paused(self.run_order("dispatch", context=self.synthetic(order=self.order, handle=jira_handle, + text="forged")), "dispatch") + self.assertEqual(create.call_count, 0) + with self.assertRaisesRegex(RemoteError, "work_order_not_found"): + self.run_order("status") + output = self.run_order("dispatch", context=jira) + self.assertEqual(output["context"], {"policy": "required", "dispatch": "delivered", "message": None}) + prompt = create.call_args.args[0] + self.assertIn("Packet identity: " + jira_handle, prompt) + self.assertNotIn(self.result["packet_handle"], prompt) + self.assertNotIn("forged", prompt) + # Cross-participant identity: the Claude/Codex peer paths render the very same handle. + current = self.fixture.attach("codex") + for recipient in ("claude:reviewer", "codex:builder", "devin:builder"): + peer = self.fixture.delivery(current, recipient) + self.assertEqual(peer.binding["handle"], jira_handle) + self.assertIn("Packet identity: " + jira_handle, peer.text) + # The retry digest binds the Jira packet; a refreshed or absent packet conflicts. + with patch.object(self.remote, "run", side_effect=Crash()), self.assertRaises(Crash): + self.run_order("clarify", request="c-1", prose=CANARY, context=jira) + with patch.object(self.provider, "message", wraps=self.provider.message) as message: + for context in (None, self.unavailable(), github_packet): + self.assert_paused(self.run_order("clarify", request="c-1", prose=CANARY, context=context), "message") + refreshed = fetch(self.fixture.store, "example", self.fixture.spec, backend=self.backend, refresh=True) + self.assertNotEqual(refreshed["packet_handle"], jira_handle) + with self.assertRaisesRegex(RemoteError, "request_conflict"): # Refreshed packet: input differs. + self.run_order("clarify", request="c-1", prose=CANARY, context=self.bind(handle=refreshed["packet_handle"])) + self.assert_paused(self.run_order("clarify", request="c-1", prose=CANARY, context=jira), "message") + self.assertEqual(message.call_count, 0) + self.assertEqual(self.run_order("status")["context"]["message"], "delivered") + # A fresh Jira session: PR verification still closes and checks the integer GitHub issue only. + self.setUp() + self.order = WorkOrder.from_manifest(manifest, CANARY, **common, context_work_item="EXAMPLE-1") + self.key = self.service._key(self.order) + jira_handle = self.fixture.result["packet_handle"] + jira = self.bind(handle=jira_handle) + self.run_order("dispatch", context=jira) + with patch.object(self.provider, "message", wraps=self.provider.message) as message: + self.assertEqual(self.run_order("clarify", request="c-1", prose=CANARY, context=jira)["round"], 1) + self.assertIn("Packet identity: " + jira_handle, message.call_args.args[1]) + self.complete(round=1) + verified = self.run_order("collect")["verified_pr"] + self.assertEqual((verified["issue"], verified["pr_number"]), (907, 42)) + self.assertNotIn("EXAMPLE-1", json.dumps(verified)) + self.github.pr = replace(self.github.pr, linked_issues=(("owner/repo", 908),)) + with self.assertRaisesRegex(RemoteError, "pull_request_binding_mismatch"): + self.run_order("collect") + def test_context_free_orders_keep_their_pre_context_binding_and_input(self): legacy = replace(self.order, context_policy="none") - legacy_fields = {k: v for k, v in asdict(legacy).items() if k != "context_policy"} + legacy_fields = {k: v for k, v in asdict(legacy).items() if k not in ("context_policy", "context_work_item")} self.assertEqual(self.service._binding(legacy), _hash([legacy_fields, self.provider.name, self.provider.account])) self.assertIn(json.dumps({k: v for k, v in legacy_fields.items() if k != "body"}, sort_keys=True)[1:-1], From 619638849b32308046342e0d839c58f80ca99edd Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Sat, 12 Sep 2026 17:35:27 +0000 Subject: [PATCH 7/7] Require any present manifest issue_number to match; only an absent key may defer to the tracker work item Co-Authored-By: bot_apk --- src/code_mower/devin_work_orders.py | 10 ++++++---- tests/test_devin_work_orders.py | 8 ++++++-- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/code_mower/devin_work_orders.py b/src/code_mower/devin_work_orders.py index d248bc11..9c5322b9 100644 --- a/src/code_mower/devin_work_orders.py +++ b/src/code_mower/devin_work_orders.py @@ -93,13 +93,15 @@ def from_manifest(cls, manifest: dict, body: str, *, repository: str, issue: int acu_limit: int = 10, context_policy: str = "none", context_work_item: str = "") -> WorkOrder: source = manifest.get("source", {}) - # A tracker-keyed (context-bearing) source may omit the GitHub delivery issue, - # which then comes from dispatcher policy alone; a present one must still match. - issue_number = source.get("issue_number") if isinstance(source, dict) else None + # A tracker-keyed (context-bearing) source may omit the GitHub delivery issue key, + # which then comes from dispatcher policy alone; any present value must match exactly. + omitted = isinstance(source, dict) and "issue_number" not in source and bool(context_work_item) if (manifest.get("schema") != WORK_ORDER_SCHEMA or manifest.get("repo") != repository or not isinstance(source, dict) or source.get("repo") != repository - or (str(issue_number) != str(issue) and not (context_work_item and not issue_number))): + or not (omitted or (type(source.get("issue_number")) in (str, int) + and type(source["issue_number"]) is not bool + and str(source["issue_number"]) == str(issue)))): raise RemoteError("work_order_binding_mismatch") return cls(repository, issue, branch, base, author_id, author_login, acu_limit, body, context_policy, context_work_item) diff --git a/tests/test_devin_work_orders.py b/tests/test_devin_work_orders.py index 24c66239..aac08acd 100644 --- a/tests/test_devin_work_orders.py +++ b/tests/test_devin_work_orders.py @@ -657,8 +657,12 @@ def test_tracker_key_binds_context_while_the_github_issue_binds_the_pull_request author_id=123, author_login="builder[bot]", acu_limit=5, context_policy="required") with self.assertRaisesRegex(RemoteError, "work_order_binding_mismatch"): # No key: the issue is required. WorkOrder.from_manifest(manifest, CANARY, **common) - with self.assertRaisesRegex(RemoteError, "work_order_binding_mismatch"): # A present issue must still match. - WorkOrder.from_manifest({**manifest, "source": {"repo": "owner/repo", "issue_number": "908"}}, + for present in ("908", 908, 0, False, "", None, True, 907.0, [907]): # A present issue must still match. + with self.subTest(issue_number=present), self.assertRaisesRegex(RemoteError, "work_order_binding_mismatch"): + WorkOrder.from_manifest({**manifest, "source": {"repo": "owner/repo", "issue_number": present}}, + CANARY, **common, context_work_item="EXAMPLE-1") + for present in ("907", 907): + WorkOrder.from_manifest({**manifest, "source": {"repo": "owner/repo", "issue_number": present}}, CANARY, **common, context_work_item="EXAMPLE-1") for bad in ("", " EXAMPLE-1", "EXAMPLE\n1", "x" * 129, 907): with self.subTest(work_item=bad), self.assertRaisesRegex(RemoteError, "invalid_work_order|binding_mismatch"):