feat: introduce dev2merge workflow and modular refactors#293
feat: introduce dev2merge workflow and modular refactors#293RyderFreeman4Logos merged 14 commits intomainfrom
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3202470520
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Generate a deterministic Conventional Commits message from staged files. | ||
|
|
||
| ```bash | ||
| scripts/gen_commit_msg.sh "${SCOPE:-}" |
There was a problem hiding this comment.
Step 13 commits with ${COMMIT_MSG}, but Step 12 only runs scripts/gen_commit_msg.sh and never writes that value back to a workflow variable. In the current runner, declared variables are populated only from defaults/--var (parse_variables in crates/cli-sub-agent/src/plan_cmd.rs) and step stdout is stored as STEP_<id>_OUTPUT (execute_plan), so COMMIT_MSG remains unset unless the caller provides it manually. That makes this path fail at commit time because git commit -m "" requires explicit --allow-empty-message (see git commit -h).
Useful? React with 👍 / 👎.
patterns/dev2merge/workflow.toml
Outdated
| BOT_COMMENTS=$(printf '%s' "${PAYLOAD}" | jq -r --arg ts "${TRIGGER_TS}" '[.comments[]? | select(.createdAt >= $ts and (.author.login | ascii_downcase | test("codex|bot|connector")) and (((.body // "") | ascii_downcase | contains("@codex review")) | not))] | length') | ||
| BOT_REVIEWS=$(printf '%s' "${PAYLOAD}" | jq -r --arg ts "${TRIGGER_TS}" '[.reviews[]? | select(.submittedAt >= $ts and (.author.login | ascii_downcase | test("codex|bot|connector")))] | length') | ||
| if [ "${BOT_COMMENTS}" -gt 0 ] || [ "${BOT_REVIEWS}" -gt 0 ]; then | ||
| echo "Bot response received." |
There was a problem hiding this comment.
This step computes BOT_COMMENTS/BOT_REVIEWS but never exports BOT_HAS_ISSUES (or any updated workflow var) for later conditions, so downstream checks like condition = "${BOT_HAS_ISSUES}" remain unresolved and evaluate false by default (evaluate_condition in crates/cli-sub-agent/src/plan_condition.rs). As a result, the bot-remediation path (steps 20–24) is skipped even when the poll found comments, and the workflow can proceed through the clean-merge branch incorrectly.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7987e9c679
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
patterns/dev2merge/workflow.toml
Outdated
| If on main or dev, abort immediately. | ||
|
|
||
| ```bash | ||
| BRANCH="${BRANCH}" |
There was a problem hiding this comment.
The branch safety gate relies on ${BRANCH}, but this workflow never assigns a value to BRANCH (it is only declared as a variable with no default), and the runner only injects declared defaults/CLI --var inputs. In default usage, BRANCH is empty, so the main/dev comparison is bypassed and protected-branch execution can proceed unintentionally; initialize BRANCH from git branch --show-current inside the step (or define a default-population step) before the check.
Useful? React with 👍 / 👎.
patterns/dev2merge/workflow.toml
Outdated
| - just test | ||
| - csa review --range main...HEAD | ||
| }" | ||
| gh pr create --base main --repo "${REPO}" --title "${COMMIT_MSG_LOCAL}" --body "${PR_BODY_LOCAL}" |
There was a problem hiding this comment.
This PR flow passes --repo "${REPO}" to gh, but REPO is never set anywhere in the workflow, so normal runs hit empty/unset repo values during PR creation/review steps (and later steps run with set -u, which hard-fails on unset variables). This breaks the PR/bot-review path unless callers manually provide REPO; add an explicit step to derive REPO (for example from gh repo view or origin URL) before Step 17.
Useful? React with 👍 / 👎.
patterns/dev2merge/workflow.toml
Outdated
| condition = "${STEP_19_OUTPUT}" | ||
|
|
||
| [workflow.steps.loop_var] | ||
| variable = "comment" |
There was a problem hiding this comment.
Step 19 now emits a truthy output ("1") when bot feedback exists, and this condition directly enables Step 21, which is a loop_var step. The current runner marks any loop_var step as unsupported and returns a non-zero skipped result (plan_cmd.rs), which is later treated as workflow failure; in practice, successful bot detection now drives the workflow into a guaranteed failure path instead of continuing review handling.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 743a58ea71
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
patterns/dev2merge/workflow.toml
Outdated
| echo "ERROR: Cannot resolve repository owner/name." >&2 | ||
| exit 1 | ||
| fi | ||
| gh pr merge "${PR_NUM}" --repo "${REPO_LOCAL}" --squash --delete-branch |
There was a problem hiding this comment.
This step references ${PR_NUM}, but the workflow never assigns that variable from prior steps: step 18 only prints PR_NUM=..., and the runner stores prior results under STEP_<id>_OUTPUT (it does not auto-promote printed KEY=VALUE lines into standalone variables). In a normal run without an explicit --var PR_NUM=..., merge execution will use an empty PR identifier instead of the PR created earlier.
Useful? React with 👍 / 👎.
patterns/dev2merge/workflow.toml
Outdated
| id = 26 | ||
| title = "Merge PR" | ||
| tool = "bash" |
There was a problem hiding this comment.
After STEP_19_OUTPUT indicates findings, this workflow runs steps 20–24 and re-triggers @codex review, but Merge PR is unconditional and executes immediately afterward. Because there is no second poll/evaluation gate after step 24, the PR can be merged before the new bot review round completes, defeating the intended review loop and allowing unresolved issues through.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9860edc41
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
patterns/dev2merge/workflow.toml
Outdated
| git checkout main && git pull origin main | ||
| ```""" | ||
| on_fail = "abort" | ||
| condition = "(!(${STEP_19_OUTPUT}))" |
There was a problem hiding this comment.
The merge gate treats STEP_19_OUTPUT being empty as "initial review clean", but Step 19 also returns empty after timing out with no bot response (echo ""), so this condition allows merging when the bot never replied at all. In runs where GitHub/Codex review is delayed or unavailable, the workflow can bypass the intended cloud-review requirement and merge unreviewed changes.
Useful? React with 👍 / 👎.
patterns/dev2merge/workflow.toml
Outdated
| BOT_INLINE_COMMENTS=$(gh api "repos/${REPO_LOCAL}/pulls/${PR_NUM_FROM_STEP}/comments?per_page=100" | jq -r --arg ts "${TRIGGER_TS}" '[.[]? | select(.created_at >= $ts and (.user.login | ascii_downcase | test("codex|bot|connector")))] | length') | ||
| BOT_PR_COMMENTS=$(gh api "repos/${REPO_LOCAL}/issues/${PR_NUM_FROM_STEP}/comments?per_page=100" | jq -r --arg ts "${TRIGGER_TS}" '[.[]? | select((.created_at // "") >= $ts and (.user.login | ascii_downcase | test("codex|bot|connector")) and (((.body // "") | ascii_downcase | contains("@codex review")) | not))] | length') | ||
| BOT_REVIEWS=$(gh api "repos/${REPO_LOCAL}/pulls/${PR_NUM_FROM_STEP}/reviews?per_page=100" | jq -r --arg ts "${TRIGGER_TS}" '[.[]? | select((.submitted_at // "") >= $ts and (.user.login | ascii_downcase | test("codex|bot|connector")))] | length') | ||
| if [ "${BOT_INLINE_COMMENTS}" -gt 0 ] || [ "${BOT_PR_COMMENTS}" -gt 0 ] || [ "${BOT_REVIEWS}" -gt 0 ]; then |
There was a problem hiding this comment.
This branch marks STEP_19_OUTPUT=1 whenever any bot inline comment, PR comment, or review exists, but that does not mean findings exist (e.g., approval/"no issues" responses also satisfy this check). In that case the workflow enters the fix/retrigger path and can hit the "remaining findings" abort despite a clean bot response, blocking valid merges.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 16476e8da1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
patterns/dev2merge/workflow.toml
Outdated
| BOT_INLINE_COMMENTS=$(gh api "repos/${REPO_LOCAL}/pulls/${PR_NUM_FROM_STEP}/comments?per_page=100" | jq -r --arg ts "${TRIGGER_TS}" '[.[]? | select(.created_at >= $ts and (.user.login | ascii_downcase | test("codex|bot|connector")))] | length') | ||
| BOT_PR_COMMENTS=$(gh api "repos/${REPO_LOCAL}/issues/${PR_NUM_FROM_STEP}/comments?per_page=100" | jq -r --arg ts "${TRIGGER_TS}" '[.[]? | select((.created_at // "") >= $ts and (.user.login | ascii_downcase | test("codex|bot|connector")) and (((.body // "") | ascii_downcase | contains("@codex review")) | not))] | length') | ||
| BOT_REVIEWS=$(gh api "repos/${REPO_LOCAL}/pulls/${PR_NUM_FROM_STEP}/reviews?per_page=100" | jq -r --arg ts "${TRIGGER_TS}" '[.[]? | select((.submitted_at // "") >= $ts and (.user.login | ascii_downcase | test("codex|bot|connector")))] | length') | ||
| if [ "${BOT_INLINE_COMMENTS}" -gt 0 ] || [ "${BOT_PR_COMMENTS}" -gt 0 ]; then |
There was a problem hiding this comment.
This condition marks the review as failing whenever any bot-authored PR comment exists, but that includes clean summary comments (for example “No issues found”), so clean reviews get routed into the fix loop and can ultimately be blocked by Step 26. The repo already documents this pitfall in patterns/pr-codex-bot/references/rebase-clean-history.md:107-110 (clean summaries can be misclassified), so this gate should parse actionable severity signals instead of using raw comment count; the same counting logic is duplicated in Step 25.
Useful? React with 👍 / 👎.
patterns/dev2merge/workflow.toml
Outdated
| MKTD_OUTPUT="$(csa run --skill mktd "${MKTD_PROMPT}" 2>&1)" | ||
| MKTD_STATUS=$? |
There was a problem hiding this comment.
With set -euo pipefail enabled, a non-zero exit from csa run --skill mktd terminates the shell at the command substitution, so MKTD_STATUS, the printf of captured output, and the custom error branch never run. That makes the intended failure handling unreachable and drops the helpful mktd output when this gate fails; wrap this call in if ! ...; then ... (or temporarily disable -e) so diagnostics are emitted before aborting.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c182a2c96
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
patterns/dev2merge/workflow.toml
Outdated
| if [ "${BOT_PR_COMMENTS}" -gt 0 ] || [ "${BOT_REVIEWS}" -gt 0 ]; then | ||
| echo "" | ||
| exit 0 |
There was a problem hiding this comment.
This branch marks the run as "clean" whenever the bot replies via PR comments or reviews (BOT_PR_COMMENTS/BOT_REVIEWS) by emitting an empty output, so the workflow can skip Steps 20–27 and proceed to merge even if that comment/review contains requested fixes. In other words, only inline comments are currently treated as findings, which bypasses the remediation loop when the bot reports issues in top-level review text.
Useful? React with 👍 / 👎.
patterns/dev2merge/workflow.toml
Outdated
| if [ "${BOT_PR_COMMENTS}" -gt 0 ] || [ "${BOT_REVIEWS}" -gt 0 ]; then | ||
| echo "" | ||
| exit 0 |
There was a problem hiding this comment.
The re-trigger polling logic repeats the same clean/dirty misclassification: any bot PR comment or review response is converted to empty output, which makes Step 25 appear clean and allows the post-fix merge path to continue even when the second review still contains findings in non-inline form. This undermines the intended "re-review must be clean" gate.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d40f4d207
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
patterns/dev2merge/workflow.toml
Outdated
| while [ "$ELAPSED" -lt "$TIMEOUT" ]; do | ||
| BOT_INLINE_COMMENTS=$(gh api "repos/${REPO_LOCAL}/pulls/${PR_NUM_FROM_STEP}/comments?per_page=100" | jq -r --arg ts "${TRIGGER_TS}" '[.[]? | select(.created_at >= $ts and (.user.login | ascii_downcase | test("codex|bot|connector")))] | length') | ||
| BOT_PR_COMMENTS=$(gh api "repos/${REPO_LOCAL}/issues/${PR_NUM_FROM_STEP}/comments?per_page=100" | jq -r --arg ts "${TRIGGER_TS}" '[.[]? | select((.created_at // "") >= $ts and (.user.login | ascii_downcase | test("codex|bot|connector")) and (((.body // "") | ascii_downcase | contains("@codex review")) | not))] | length') | ||
| BOT_PR_FINDINGS=$(gh api "repos/${REPO_LOCAL}/issues/${PR_NUM_FROM_STEP}/comments?per_page=100" | jq -r --arg ts "${TRIGGER_TS}" '[.[]? | select((.created_at // "") >= $ts and (.user.login | ascii_downcase | test("codex|bot|connector")) and (((.body // "") | ascii_downcase | contains("@codex review")) | not) and ((.body // "") | ascii_downcase | test("p[0-3][^a-z0-9]*badge|changes requested|must fix|blocking|severity|critical")))] | length') |
There was a problem hiding this comment.
In Poll for Bot Response, BOT_PR_FINDINGS only flags PR comments when the body matches p[0-3][^a-z0-9]*badge|..., which does not match the severity format used by Codex findings (**P0**/**P1**/**P2**, documented in patterns/pr-codex-bot/references/rebase-clean-history.md). A bot comment containing **P1** therefore gets treated as “clean”, causing this step to emit empty output, skip the fix/arbitration path, and allow merge with unresolved findings; the same predicate is repeated in step 25.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ed55992367
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
patterns/dev2merge/workflow.toml
Outdated
| echo "ERROR: TODO file is empty: ${TODO_PATH}" >&2 | ||
| exit 1 | ||
| fi | ||
| grep -qE '^- \[ \] .+' "${TODO_PATH}" || { echo "ERROR: TODO missing checkbox tasks: ${TODO_PATH}" >&2; exit 1; } |
There was a problem hiding this comment.
Use escaped backslashes inside this multiline TOML string (\\[/\\]), because basic TOML strings treat \ as an escape prefix and \[ is invalid. As written, the new dev2merge workflow file fails TOML parsing before execution (e.g., when loading it for csa plan run/weave validation), so the newly introduced primary workflow cannot run.
Useful? React with 👍 / 👎.
|
@codex review |
1 similar comment
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b0a70227f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| **YOU ARE THE EXECUTOR.** Follow these rules: | ||
| 1. **SKIP the "Execution Protocol" section below** -- it is for the orchestrator, not you. | ||
| 2. **Read the pattern** at `patterns/dev2merge/PATTERN.md` and follow it step by step. | ||
| 3. **ABSOLUTE PROHIBITION**: Do NOT run `csa run`, `csa review`, `csa debate`, or ANY `csa` command. You must perform the work DIRECTLY. Running any `csa` command causes infinite recursion. |
There was a problem hiding this comment.
The executor-mode rule forbids running any csa command, but the new dev2merge workflow it tells executors to follow requires multiple csa invocations (for example csa run --skill mktd in step 101 and later csa review steps in patterns/dev2merge/workflow.toml), so executor runs cannot complete without violating the skill contract. This makes the primary advertised workflow path internally inconsistent for sub-agent execution.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6cacea7b5f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
patterns/dev2merge/workflow.toml
Outdated
| git add -A | ||
| if ! printf '%s' "${SCOPE:-}" | grep -Eqi 'release|version|lock|deps|dependency'; then | ||
| STAGED_FILES="$(git diff --cached --name-only)" | ||
| if printf '%s\n' "${STAGED_FILES}" | grep -Eq '(^|/)Cargo\.toml$|(^|/)package\.json$|(^|/)pnpm-workspace\.yaml$|(^|/)go\.mod$'; then |
There was a problem hiding this comment.
This workflow file is not valid TOML as written: inside a TOML basic multiline string, \ starts an escape, so sequences like Cargo\.toml written as Cargo\.toml with a single backslash before . are parsed as invalid escapes (\.). weave::compiler::plan_from_toml deserializes workflow files with toml::from_str, so this causes patterns/dev2merge/workflow.toml to fail loading entirely (blocking plan-based execution/visualization for the new workflow).
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e0c7035cfb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| PR_BODY_LOCAL="${PR_BODY:-## Summary | ||
| - Scope: ${SCOPE:-unspecified}''' |
There was a problem hiding this comment.
This bash step is syntactically broken: PR_BODY_LOCAL starts a quoted parameter expansion but never closes it, so Step 19 exits with a shell parse error before gh pr create can run, and on_fail = "abort" stops the workflow at PR creation. The same truncated block appears in patterns/dev-to-merge/workflow.toml, so both workflow entrypoints are affected.
Useful? React with 👍 / 👎.
| Create the commit using the generated message from Step 12. | ||
|
|
||
| ```bash | ||
| COMMIT_MSG_LOCAL="${STEP_12_OUTPUT:-${COMMIT_MSG:-}}" |
There was a problem hiding this comment.
The commit step pulls COMMIT_MSG_LOCAL from ${STEP_12_OUTPUT}, but the commit message is generated in Step 14; Step 12 is a conditional quality-gate rerun and is often skipped. In the common no-review-issues path this leaves COMMIT_MSG_LOCAL empty and the commit aborts, and in the reviewed path it can use just pre-commit output as the commit title instead of the generated message.
Useful? React with 👍 / 👎.
| PR_NUM_FROM_STEP="$(printf '%s\n' "${STEP_18_OUTPUT:-}" | sed -n 's/^PR_NUM=//p' | tail -n1)" | ||
| TRIGGER_TS="$(printf '%s\n' "${STEP_18_OUTPUT:-}" | sed -n 's/^TRIGGER_TS=//p' | tail -n1)" | ||
| TRIGGER_COMMENT_ID="$(printf '%s\n' "${STEP_18_OUTPUT:-}" | sed -n 's/^TRIGGER_COMMENT_ID=//p' | tail -n1)" |
There was a problem hiding this comment.
Polling reads PR_NUM, TRIGGER_TS, and TRIGGER_COMMENT_ID from ${STEP_18_OUTPUT}, but those values are emitted by the trigger step (id 21), not the push step (id 18). As written, trigger metadata is dropped and TRIGGER_TS falls back to 1970-01-01T00:00:00Z, so polling can include stale historical bot activity and drive incorrect fix/merge branching.
Useful? React with 👍 / 👎.
1 participant
Summary
Validation
running 1 test
test hub_forwards_requests_and_proxy_latency_budget_is_within_5ms ... ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out; finished in 0.48s
The uncommitted changes on branch
feat/dev2merge-workflowinclude a version bump (0.1.57 -> 0.1.58), refactoring of prompt guard logic incrates/cli-sub-agent, and the introduction of a new comprehensive development patterndev2merge. The refactoring is clean and follows project conventions. However, thedev2mergepattern has several synchronization and logic issues, particularly regarding variable capture and Rule 027 compliance.Code Review:
feat/dev2merge-workflow1. Rust Refactoring (
crates/cli-sub-agent)should_emit_prompt_guard_to_callerandemit_prompt_guard_to_callerfrompipeline.rstopipeline_prompt_guard.rs.pub(super)correctly for internal module access.pipeline_tests_prompt_guard.rsand integrated them using#[path], maintaining consistency with existing test organization.PROMPT_GUARD_ENV_LOCKin tests correctly addresses potential race conditions when modifying process-wide environment variables.2. Pattern Development (
patterns/dev2merge)The
dev2mergepattern is a highly sophisticated 24+ step workflow. While ambitious, it contains several issues that need addressing for reliable execution.MKTD_TODO_PATH,MKTD_TODO_TIMESTAMP, andCUMULATIVE_REVIEW_COMPLETEDare defined inworkflow.tomlbut never referenced with${}inPATTERN.md.REPOis referenced inworkflow.toml(Steps 17, 18, 19, 24, 26) but is never defined, set, or referenced inPATTERN.md. This will likely cause runtime failures unless provided as an external input.Generate Commit Message): Callsscripts/gen_commit_msg.shbut does not capture its stdout into theCOMMIT_MSGvariable. Consequently, Step 13 (Commit) will attempt to rungit commit -m "", which will fail.csaCalls:csa run,csa review, andcsa debate. Whilecsasupports recursion up toCSA_DEPTH, theSKILL.mdexplicitly prohibits executors from callingcsacommands to avoid infinite recursion. This creates a conflict where the pattern cannot be safely executed by a sub-agent despite being designed for it.PATTERN.mdusesStep 1.5forPlan with mktd, whileworkflow.tomlusesid = 101.workflow.tomlStep 7, thesedcommand uses\\1, whilePATTERN.mduses\1. Ensure the escaping is correct for the target shell environment.3. Security Analysis
Security Scan) and Step 7 (Security Audit) in the workflow demonstrates strong "Security First" principles.emit_prompt_guard_to_callerfunction correctly implements a mechanism for sub-agents to pass instructions back to parents, with an opt-out environment variable. No sensitive data leaks were identified in the implementation.4. Project Integrity
0.1.58acrossCargo.toml,Cargo.lock, andweave.lock.skill.mdandskills/AGENTS.mdwere correctly updated to reflect the new pattern.Recommendations
dev2merge/workflow.tomlStep 12.REPOis properly initialized (e.g., viagit remote get-url originin Step 1).PATTERN.mdandworkflow.tomlvariables per Rule 027.dev2mergepattern to ensure safe execution by sub-agents.The PR successfully introduces the
dev2mergeend-to-end workflow pattern, refactors prompt guard and output helper logic into specialized modules, and increments the workspace version to0.1.58. The implementation demonstrates high adherence to Rust best practices, security guidelines (including injection prevention in shell blocks), and the project's recursive agent architecture.Architectural & Quality Review
1. New
dev2mergeWorkflow Patternmktdwith mandatory debate) to merging, incorporating quality gates (lint/test), security audits, and heterogeneous reviews.PATTERN.mdandworkflow.tomlare perfectly synchronized with 11 extracted variables correctly mapped.csa run --skillfor sub-task delegation, maintaining session isolation as mandated byCLAUDE.md.2. Code Refactoring & Cleanliness
pipeline_prompt_guard.rs,executor_arg_helpers.rs, andlib_output_helpers.rssignificantly reduces the cognitive load and complexity of the core crates.pipeline_tests_prompt_guard.rs,lib_tests_heartbeat.rs) use process-wide mutexes to safely guard environment variable mutations, ensuring reliable parallel test execution.String::draininaccumulate_and_flush_linesprovides efficient line-based output processing with minimal allocations.Security Review
PATTERN.mdshell blocks uses robust quoting (e.g.,"${BRANCH}","${COMMIT_MSG}"), effectively mitigating shell injection risks from malicious branch names or commit messages.Minor Observations & Findings
gemini_prompt_directoriesuses a whitespace-based heuristic to find absolute paths in prompts. While it handles greedy matching for valid paths, it may miss paths containing spaces. This is acceptable given the current usage context.patterns/dev2merge/FINDINGS.mdprovides excellent meta-analysis of theweavecompiler's current limitations (e.g., markdown headers in code blocks) and serves as a valuable reference for future pattern development.Verdict
PASS. The changes are ready for merge once the automated CI pipeline confirms the test suite passes.