feat(multi-lens-review): standalone durable fan-out code-review workflow (replaces parallel-brief-runner) - #1709
Merged
Merged
Conversation
…dition verdict New apps/multi-lens-review on @themoltnet/tasks-orchestrator: parallelTasks fan-out of per-lens freeform reviews + a server-gated (joinCondition) synthesis verdict. 6 inline tests, typecheck/lint/build green. MoltNet-Diary: 806e0e04-da11-4361-b257-5bb133d9961f
…hema The freeform input schema is additionalProperties:false (brief, expectedOutput, constraints, successCriteria, context, execution, continueFrom). Dropped the extra 'lens' and 'reviewTaskIds' fields — the lens is baked into the review prompt and the review task ids into the synthesis brief text — so task creation passes validation. Verified end-to-end against a live agent-daemon on Ollama Cloud: security caught SQL+command injection, correctness caught the off-by-one, synthesis fetched both reviews and returned request-changes.
Contributor
✅ CLI go.mod matches internal Go module releases
|
Contributor
🚨 Dependency Audit — Vulnerabilities foundFull report |
Contributor
|
legreffier
Bot
changed the title
![legreffier[bot]](https://avatars.githubusercontent.com/in/2878569?s=60&v=4){kind=small}
Contributor
Author
There was a problem hiding this comment.
Design review: pivot recommended
What this PR is trying to accomplish
Add a durable standalone multi-lens review app and reuse its fan-out machinery inside issue-lifecycle.
Why the current approach is problematic
The standalone workflow is sound, but @themoltnet/review-workflow is not a meaningful shared review abstraction. It delegates every domain decision—task construction, awaiting, status effects, parsing, and synthesis—to callbacks. The only shared behavior is already provided by parallelTasks and joinCondition. Issue-lifecycle does not use synthesis, while multi-lens-review is its sole user, so the new package mostly wraps an existing primitive and imposes review-specific naming on generic orchestration.
The repository's parallel-brief-runner already demonstrates the same fan-out-plus-gated-continuation shape directly. This also matches a signature-verified project decision: consumer-specific inputs can produce a generic layer that is harder to inspect without centralizing actual domain behavior.
Alternative approaches
- Keep
multi-lens-review, implement it directly withparallelTasksandjoinCondition, and leave issue-lifecycle onparallelTasks. This has small explicit duplication but preserves clear ownership and existing patterns. - If three or more consumers establish a stable common contract, add a domain-neutral fan-out-plus-continuation primitive to
tasks-orchestratorand migrateparallel-brief-runnertoo. This offers real reuse but expands a published orchestration API and deserves separate design work.
Recommended next step
Remove libs/review-workflow, restore issue-lifecycle's direct parallelTasks use, and retain the standalone app using the existing orchestration primitives.
Pivot size: small (under 100 LOC of rework).
…-lens-review multi-lens-review is the canonical durable fan-out -> gated-join example (real use case, same shape). Referenced only in the root tsconfig; the e2e was already removed. (This branch also drops the review-workflow lib + issue-lifecycle wiring per the #1709 design review — the standalone app is the reusable workflow.) MoltNet-Diary: d6370d5c-0d6d-4058-8056-0c26e35e8c79
legreffier
Bot
force-pushed
the
multi-lens-review
branch
from
July 28, 2026 07:11
ba242f8 to
e6e7c18
Compare
legreffier
Bot
changed the title
Contributor
Author
|
Agreed with the design review — pivoted. Dropped |
Contributor
Author
There was a problem hiding this comment.
Deep review: Needs work
The architectural pivot is complete and sound: the standalone app now composes existing orchestration primitives directly, issue-lifecycle remains decoupled, and replacing the toy parallel-brief-runner example with a real use case is reasonable.
Top risks: (1) untrusted PR text enters broadly capable tool-using agent sessions, (2) synthesis can lose or fail to retrieve its required inputs, and (3) interrupted or failed runs can duplicate fan-out or leave permanent waiting tasks.
Strengths: clear app/library boundary, preflight validation of the server join ceiling, focused workflow tests, and all current CI, CodeQL, Semgrep, and e2e checks green.
Themes: trust boundaries and failure/restart behavior need the same rigor as the successful-path orchestration. Prompt and output contracts should have one runtime-owned source of truth.
Outside-diff finding: libs/tasks-orchestrator/README.md should link apps/multi-lens-review as the canonical runnable fan-out/join example now that parallel-brief-runner is removed.
Coverage: correctness, security, DRY/codebase fit, design/API/backcompat, tests, operability, and readability ran. Performance was not routed because no files or changed-line signals matched that lane. No coverage sweep was needed because correctness inspected every changed file.
Correctness: synthesis fetch via acceptedAttemptN + list_task_attempts (:135); --synthesis composes into the scaffold not replaces it (:131). Remove FREEFORM_SUBMIT_INSTRUCTIONS (runtime injects it) (:30). Cancel-by-correlation on review failure so the gated synthesis isn't orphaned (:188). --correlation-id for resumable runs (main:22). Dedup + MAX_LENSES cap (:53). Rename parser (:161), per-review bound logging (:205), untrusted-data framing (:95). README links the example. +dedup/cancel tests. MoltNet-Diary: c39068e8-f5cf-48c5-a401-7e46e9f5e359
Contributor
Author
|
Addressed the deep review in Fixed (9): synthesis fetch via Deferred (2): stateful All 11 threads resolved. |
getlarge
approved these changes
Jul 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A standalone durable code-review workflow on
@themoltnet/tasks-orchestrator:fan out N specialist reviews (security / correctness / performance /
test-coverage) with
parallelTasks, join them into a server-gated verdictwith
joinCondition, resumable viactx.step.Scope (revised after design review)
An earlier revision of this PR also extracted a shared
@themoltnet/review-workflowlib and rewired issue-lifecycle onto it. The design review (thanks!) correctly
found that lib to be a non-meaningful abstraction — it delegated every domain
decision to callbacks, issue-lifecycle didn't use its synthesis, and the only
shared behavior (the up-front
joinCondition-gated synthesis) is a handful oflines the example apps already inline. So that split is dropped:
libs/review-workflow;parallelTasks(unchanged frommain);demand (see RFC RFC: workflows as a first-class MoltNet resource (registry + dispatch + run-tracking) #1718). A domain-neutral fan-out+continuation primitive is
deferred until 3+ consumers share a stable contract.
This PR also removes
apps/parallel-brief-runner— the standalonemulti-lens-review is the same fan-out → gated-join shape but a real use case, so
it supersedes the toy example.
Proven live
Run end-to-end against a local stack + a real
agent-daemonon Ollama Cloud(
gemma4:31b-cloud) over a planted-bug diff: security caught SQL + commandinjection, correctness caught an off-by-one, and (unplanted) performance
flagged an unbounded query + test-coverage flagged missing injection-payload
tests — consolidated verdict
request-changes. The 4-lens run surfaced ~10issues vs ~3 for 2 lenses.
Findings surfaced while testing
inputis a strict schema (additionalProperties: false) — structureddata lives in the
brieftext.submit_freeform_outputwith averificationrecord (the injected
submit-outputgate) — baked into the prompts.500is classified non-retryable — filed as agent-daemon: transient LLM provider 500 (Internal Server Error) classified non-retryable, fails the attempt #1706.Verified
typecheck+lint+test(6 inlineFakeTaskstests) +buildgreen.References