Skip to content

feat(agent): implementation subagent for the development loop#90

Merged
ncolesummers merged 10 commits into
mainfrom
codex/48-implementation-agent
Jul 14, 2026
Merged

feat(agent): implementation subagent for the development loop#90
ncolesummers merged 10 commits into
mainfrom
codex/48-implementation-agent

Conversation

@ncolesummers

Copy link
Copy Markdown
Owner

Closes #48.

Summary

Adds the implementer subagent for the development stage under the ADR 0015 orchestration contract:

  • Typed result contractloopworks.implementation_result.v1: a digest-bound, production-only unified diff plus per-test green evidence and aggregate bun run validate evidence, with patch-safety refinement (path allowlist, forbidden operations, byte/digest integrity).
  • Guarded subagent tools — commit-pinned checkout with scoped network egress, one-shot exact test-patch application, one bounded production-only write, per-planned-test green runs, and aggregate validation, all emitting HMAC-signed redacted receipts.
  • Root-owned persistenceapplyDevelopmentLoopImplementationResult revalidates the approval, handoff digests, and every receipt inside one transaction, claims the step against concurrent replays, stores the result on the development patch artifact, and advances the run to validation. Idempotent replays return the persisted outcome.
  • Orchestrator routing, fixture mode, evals — development delegates to implementer; LOOPWORKS_EVE_IMPLEMENTER_FIXTURE_MODE serves a deterministic canned handoff; eval covers root→implementer routing on issue Implementation subagent for the development loop #48.

Validation

  • bun run precommit green (typecheck, 352 unit tests across 57 files, 22 e2e).
  • Eve eval discovery includes the implementation routing scenario.

Adversarial review findings (not yet fixed — kept as review input)

Multi-angle review of this diff surfaced real issues, most severe first:

  1. Blocker (correctness)apply_exact_test_patch.ts:41 verifies applied paths with git diff --name-only, which never lists untracked files, but git apply leaves new files untracked. Any test patch adding a new test file (the test-writer's standard output) always throws "Applied test patch changed undeclared paths", blocking every real non-fixture run.
  2. Blocker (correctness)write_production_files.ts:49 has the same untracked-file blindness in its cleanliness check; it throws before any production write.
  3. Securityread_implementation_context.ts:34 interpolates repositoryFullName (validated only as non-empty string) into a double-quoted shell command where $(...) still expands, during the open-egress clone window. The sibling test-writer checkout has a format guard (read_approved_plan.ts:73) that this copy dropped.
  4. CorrectnessclassifyGreenRun (tool-policy.ts:54) marks passing runs invalid if output contains bare substrings like "timeout"/"killed"; this repo has timeout-named tests.
  5. Correctness (plausible)run_aggregate_validation.ts:54 re-scans passing output for /tests? failed|build failed/i; a test title like "renders build failed banner" fails a green validate.
  6. Availabilitywrite_production_files.ts:55 writes the one-shot claim marker before the guarded writes; any transient failure afterward permanently bricks the session with no retry path.
  7. DesignisAllowedProductionArtifactPath rejects any filename containing "fixture" or any tests segment — the implementer could not have produced this very PR (fixture-mode.ts).
  8. Observability — the implementation_stage_advanced structured log is dead code (no caller passes a logger); write_production_files and the network-egress checkout have no span/log, unlike sibling tools (agent/AGENTS.md rule 5).
  9. Reuse/drift — redaction, HMAC receipts, and the fixture-mode resolver are forked from the test-writer and have already drifted (implementer redacts PEM keys; test-writer does not; resolver now exists in 3 copies).
  10. AltitudeloadImplementationHandoff is a weaker duplicate of the transition's verification (misses redEvidence plan binding, duplicate-row tolerance), so a subagent-accepted handoff can burn a full stage before the root rejects it; receipt payload construction is duplicated at four sites.

🤖 Generated with Claude Code

uidaho-nsummers and others added 7 commits July 13, 2026 21:07
Define loopworks.implementation_result.v1: a digest-bound production
patch with per-test green evidence and aggregate validation evidence,
plus path-safety refinement and the issue-48 fixture handoff used by
implementer fixture mode.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The development-stage sibling consumes the persisted plan, test patch,
red evidence, and fixtures without re-derivation. Guarded tools pin the
checkout, apply the exact test patch once, permit one bounded
production-only write, run each planned test plus aggregate validation,
and emit HMAC-signed, redacted execution receipts.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
applyDevelopmentLoopImplementationResult revalidates the approved plan,
test-plan/red-evidence binding, and signed green receipts inside one
transaction, claims the development step against concurrent replays,
stores the result on the patch artifact, and advances the run to
validation. Idempotent replays return the persisted outcome.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The orchestrator delegates development to the implementer and persists
via apply_implementation_result; fixture mode serves a canned
development-stage context for deterministic evals. The development
stage actor becomes the declared implementer subagent.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Cover root-to-implementer delegation on issue 48: typed result
emission, control-plane persistence, and denial of raw write/bash
tools at the root.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Document the implementation sibling's handoff contract, signed green
receipts, and root-owned persistence; mark the issue-48 follow-up done
and register LOOPWORKS_EVE_IMPLEMENTER_FIXTURE_MODE.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@vercel

vercel Bot commented Jul 14, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
loopworks Ready Ready Preview, Comment Jul 14, 2026 5:10am

Request Review

uidaho-nsummers and others added 3 commits July 13, 2026 22:07
Security-critical helpers now have one implementation: redactSecrets
(the test-writer gains the private-key rule it was missing), a generic
HMAC execution-receipt pair, and a parameterized stage fixture-mode
resolver. loadImplementationHandoff verifies at parity with the root
transition so the subagent rejects anything persistence would reject
instead of burning the stage first.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tion

- Verify sandbox working trees with git status --porcelain
  --untracked-files=all instead of git diff --name-only, which never
  lists untracked files and blocked every test patch that added a new
  file; unquote C-quoted porcelain paths.
- Guard repositoryFullName against shell metacharacters before it is
  interpolated into the clone command during the open-egress window,
  matching the test-writer checkout.
- Classify green runs by exit code plus positive pass evidence instead
  of bare substrings like "timeout" that collide with test titles, and
  trust the aggregate validation exit code over phrase scans.
- Release the one-shot production-write claim with a best-effort file
  revert when a write fails, so transient errors no longer brick the
  session.
- Stop rejecting production files merely named after fixtures
  (fixture-mode.ts could not have shipped itself).
- Add spans to the production write and the network-egress checkout.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… tool

The transition's implementation_stage_advanced log was dead code
because no caller passed a logger.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@ncolesummers

Copy link
Copy Markdown
Owner Author

Review findings addressed

All 10 findings from the adversarial review are now handled in three commits (27f1323, 1cbd3d3, 3031d9b):

  • 1–2 (blockers): working-tree verification now uses git status --porcelain --untracked-files=all + C-quote-aware parsing, so test patches that add new files (and new directories) verify correctly.
  • 3 (security): repositoryFullName is format-guarded before shell interpolation in the checkout, at parity with the test-writer.
  • 4–5: classifyGreenRun requires positive pass evidence and no longer misfires on test titles containing "timeout"/"killed"; aggregate validation trusts the exit code.
  • 6: a failed production write reverts written files and releases the one-shot claim — transient errors no longer brick the session.
  • 7: the filename-contains-"fixture" denylist rule is gone (directory and .test./.spec. rules remain).
  • 8: the apply tool passes a logger (implementation_stage_advanced now emits); spans added to the production write and network-egress checkout.
  • 9: redaction, HMAC receipts, and fixture-mode resolution are single shared implementations under agent/lib/; the test-writer gained the private-key redaction rule it was missing.
  • 10: loadImplementationHandoff verifies at parity with the root transition.

A follow-up adversarial pass on the fixes themselves caught and fixed: missing --untracked-files=all, C-quoted porcelain paths, and a leaked checkout span.

Deferred follow-ups (pre-existing or larger refactors, not blocking):

  • classifyTestRun in the test-writer has the same broad-substring weakness (pre-existing on main).
  • Extracting the duplicated commit-pinned checkout block and a single shared handoff validator (parity is currently maintained by hand in two places).
  • run_green_test_suite / run_aggregate_validation share a ~100-line skeleton worth collapsing.
  • Efficiency: loadImplementationHandoff re-runs per tool call; the implementation result is schema-parsed up to four times per handoff.

Validation: bun run precommit green (typecheck, unit, e2e).

@ncolesummers
ncolesummers marked this pull request as ready for review July 14, 2026 05:13
Copilot AI review requested due to automatic review settings July 14, 2026 05:13

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@ncolesummers
ncolesummers merged commit 521c7cf into main Jul 14, 2026
4 checks passed
@github-project-automation github-project-automation Bot moved this from Todo to Done in Loopworks Backlog Jul 14, 2026
@ncolesummers
ncolesummers deleted the codex/48-implementation-agent branch July 21, 2026 06:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Implementation subagent for the development loop

3 participants