Repository navigation
fix: make loop goals converge reliably - #335
Merged
Merged
Conversation
|
Too many files changed for review (136 files, 100 file limit). Bypass the limit by tagging |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (14)
📒 Files selected for processing (11)
WalkthroughThe change adds runtime contract materialization, structured goal criteria and warnings, diagnostic persistence, workspace-aware command execution, expanded API responses, and web rendering for materialized contracts and goal diagnostics. ChangesLoop contract materialization
Persistence and API projection
Web loop views
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/loop/goal/templates.go (1)
125-181: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winTreat prior diagnostics as untrusted prompt data.
Stdout,Stderr, and warning messages can contain evaluator-controlled instructions. This code writes them directly into the next Goal agent prompt. A command criterion can emit text that changes the next agent behavior.Render diagnostic values in a bounded, explicitly untrusted representation. Add an instruction that the agent must not follow instructions contained in diagnostic output.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/loop/goal/templates.go` around lines 125 - 181, Update appendPreviousCriterionStream and appendPreviousWarnings to render stdout, stderr, and warning messages as bounded, explicitly untrusted diagnostic data rather than injecting unrestricted text into the prompt. Add clear prompt text near the existing diagnostic sections instructing the Goal agent not to follow instructions contained in diagnostic output, while preserving the existing criterion and warning context.
🧹 Nitpick comments (1)
internal/loop/gate/evaluator_test.go (1)
1492-1499: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a regression for literal template-shaped rubric text.
This test now uses an already materialized rubric. It does not protect the boundary where
RenderAgentJudgeRubricmust preserve authored{{ ... }}text and must not render it a second time. Add a focused case in this canonical suite.Suggested test shape
t.Run("Should append a materialized rubric to contract context", func(t *testing.T) { t.Parallel() // existing assertions }) + t.Run("Should preserve literal template-shaped rubric text", func(t *testing.T) { + t.Parallel() + const literal = `Check {{ .inputs.value }} as authored text` + rendered, err := RenderAgentJudgeRubric( + dsl.GateCriterion{ + ID: "judge", Type: dsl.CriterionAgentJudge, Rubric: literal, + }, + validContract(), + JudgeEvidence{}, + ) + if err != nil { + t.Fatalf("RenderAgentJudgeRubric() error = %v", err) + } + if !stringsContains(rendered, literal) { + t.Fatalf("rendered rubric does not preserve %q:\n%s", literal, rendered) + } + })As per path instructions, update the canonical Go test suite and keep the new case under
t.Run("Should …").🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/loop/gate/evaluator_test.go` around lines 1492 - 1499, Add a focused t.Run("Should …") regression in the canonical suite for RenderAgentJudgeRubric that supplies a rubric containing literal template-shaped {{ ... }} text after materialization and verifies the output preserves it unchanged without a second render. Keep the existing materialized-rubric test intact and assert the expected rendered rubric and error behavior.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/daemon/loop_runtime_adapters.go`:
- Around line 188-189: Update the command execution flow around
exec.CommandContext in the runtime adapter to prevent shell-interpolated command
checks from executing untrusted template-rendered values. Require command edit
or configuration-author authorization for command.Check and command.Contains
inputs, or replace shell execution with an explicit executable-and-argv contract
that cannot interpret injected shell syntax.
In `@internal/store/globaldb/global_db_goal_judge_integration_test.go`:
- Around line 114-118: Strengthen the assertions in the completion and reload
checks for the goal judge result: verify the criterion’s ID, Type, and Outcome,
plus the diagnostic warning’s Code and Message, rather than only collection
sizes. Apply the same field-level assertions to both completed and loaded
results, using the expected values shown in the fixture.
In `@internal/store/globaldb/global_db_goal_turn_runtime_integration_test.go`:
- Around line 619-626: Strengthen the completed-event assertions after the
criteria and warnings length checks by validating representative serialized
fields: the criterion’s id, type, passed status, and stdout, plus the warning’s
code and message. Keep the existing cardinality checks and use the decoded
entries from completedEvent.
In `@web/src/systems/loops/components/stories/loop-run-scenario-props.ts`:
- Line 43: Update the props construction in the loop scenario mapping to pass
the scenario’s resolved inputs to materializeContractFixture and assign its
result to materializedContract instead of using definition.contract directly.
Preserve the existing resolved-input source and return shape so LoopRunPageBody
receives the materialized contract.
In `@web/src/systems/loops/lib/loop-events.ts`:
- Around line 291-305: Update goalCriteria to validate record?.passed is
explicitly a boolean before accepting a criterion; reject entries with missing
or invalid passed values instead of coercing them to false, while preserving
valid true/false values. Add a malformed-event test in the loop-events test
suite covering a criterion with an approved outcome but no Boolean passed field.
---
Outside diff comments:
In `@internal/loop/goal/templates.go`:
- Around line 125-181: Update appendPreviousCriterionStream and
appendPreviousWarnings to render stdout, stderr, and warning messages as
bounded, explicitly untrusted diagnostic data rather than injecting unrestricted
text into the prompt. Add clear prompt text near the existing diagnostic
sections instructing the Goal agent not to follow instructions contained in
diagnostic output, while preserving the existing criterion and warning context.
---
Nitpick comments:
In `@internal/loop/gate/evaluator_test.go`:
- Around line 1492-1499: Add a focused t.Run("Should …") regression in the
canonical suite for RenderAgentJudgeRubric that supplies a rubric containing
literal template-shaped {{ ... }} text after materialization and verifies the
output preserves it unchanged without a second render. Keep the existing
materialized-rubric test intact and assert the expected rendered rubric and
error behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 49305a19-fe19-45fe-8632-038af06c4f07
⛔ Files ignored due to path filters (18)
docs/design/opendesign/_done/loops/LOOP-RUN-REDESIGN-SPEC.mdis excluded by!docs/design/**/*,!**/*.mddocs/qa/bugs/BUG-20260808-goal-command-judge-unavailable.mdis excluded by!**/*.mddocs/qa/charters/CH-loop-goal-command-convergence.mdis excluded by!**/*.mddocs/qa/charters/CH-loop-template-materialization.mdis excluded by!**/*.mddocs/qa/reports/2026-08-10-loop-convergence.mdis excluded by!**/*.mddocs/qa/scenarios/LP-goal-command-judge.mdis excluded by!**/*.mddocs/qa/scenarios/LP-loop-template-snapshot-round-trip.mdis excluded by!**/*.mddocs/qa/scenarios/LP-run-detail-story-redesign.mdis excluded by!**/*.mdinternal/store/globaldb/schema/migrations/atlas.sumis excluded by!**/*.sum,!**/*.suminternal/tools/builtin/testdata/native-tool-catalog.jsonis excluded by!**/*.jsonopenapi/compozy.jsonis excluded by!**/*.jsonpackages/site/content/docs/loops/authoring.mdxis excluded by!**/*.mdxpackages/site/content/docs/loops/goals.mdxis excluded by!**/*.mdxpackages/site/content/docs/loops/reference-grammar.mdxis excluded by!**/*.mdxpackages/site/content/docs/loops/running.mdxis excluded by!**/*.mdxskills/compozy/references/loops.mdis excluded by!**/*.mdweb/src/generated/compozy-openapi.d.tsis excluded by!**/generated/**,!**/generated/**,!**/*.d.tsweb/src/systems/loops/components/run-page/stories/goal-turn-timeline.stories.tsxis excluded by!**/*.stories.tsx
📒 Files selected for processing (86)
internal/api/contract/goals.gointernal/api/contract/loop_runs.gointernal/api/contract/loops.gointernal/api/core/conversions_goals.gointernal/api/core/loops_test.gointernal/cli/loop_runs.gointernal/cli/loop_runs_output.gointernal/daemon/daemon_review_and_fix_e2e_integration_test.gointernal/daemon/loop_api_goal_turns.gointernal/daemon/loop_api_runs.gointernal/daemon/loop_goal_executor.gointernal/daemon/loop_runtime_adapters.gointernal/daemon/loop_runtime_adapters_test.gointernal/daemon/loop_runtime_selection_integration_test.gointernal/daemon/native_loop_tools_test.gointernal/daemon/task_runtime_boot_roles.gointernal/loop/action_contract.gointernal/loop/action_goal_params.gointernal/loop/compiler.gointernal/loop/contract_references.gointernal/loop/control_contract_gate.gointernal/loop/control_gate.gointernal/loop/control_gate_config_test.gointernal/loop/coordinator_action.gointernal/loop/coordinator_snapshot_test.gointernal/loop/coordinator_test.gointernal/loop/gate/evaluator.gointernal/loop/gate/evaluator_command.gointernal/loop/gate/evaluator_judge_human.gointernal/loop/gate/evaluator_test.gointernal/loop/gate/judge.gointernal/loop/gate/types.gointernal/loop/goal/executor.gointernal/loop/goal/executor_test.gointernal/loop/goal/executor_test_support_test.gointernal/loop/goal/route.gointernal/loop/goal/templates.gointernal/loop/goal/types.gointernal/loop/goal/types_judge.gointernal/loop/goal/types_tool.gointernal/loop/goal/types_turn.gointernal/loop/linter_references.gointernal/loop/linter_test.gointernal/loop/service.gointernal/loop/service_test.gointernal/loop/service_types.gointernal/session/goal_contract.gointernal/store/globaldb/global_db_goal_checkpoint_integration_test.gointernal/store/globaldb/global_db_goal_judge_helpers.gointernal/store/globaldb/global_db_goal_judge_integration_test.gointernal/store/globaldb/global_db_goal_judges.gointernal/store/globaldb/global_db_goal_schema_integration_test.gointernal/store/globaldb/global_db_goal_tools.gointernal/store/globaldb/global_db_goal_turn_complete.gointernal/store/globaldb/global_db_goal_turn_complete_helpers.gointernal/store/globaldb/global_db_goal_turn_reader.gointernal/store/globaldb/global_db_goal_turn_runtime_integration_test.gointernal/store/globaldb/global_db_task_claim_test.gointernal/store/globaldb/goal_judge_sqlc_mapping.gointernal/store/globaldb/goal_runtime_mapping.gointernal/store/globaldb/goal_verdict_diagnostics.gointernal/store/globaldb/queries/goal_turn_judge.sqlinternal/store/globaldb/schema/definitions/50_loops.sqlinternal/store/globaldb/schema/migrations/00058_schema.sqlinternal/store/globaldb/sqlcgen/goal_runtime.sql.gointernal/store/globaldb/sqlcgen/goal_turn_judge.sql.gointernal/store/globaldb/sqlcgen/models.gointernal/tools/builtin/builtin_test.gointernal/tools/builtin/loops_output_schemas.goweb/src/systems/loops/components/__tests__/goal-turn-timeline.test.tsxweb/src/systems/loops/components/__tests__/loop-run-form.test.tsxweb/src/systems/loops/components/run-form/loop-run-preview.tsxweb/src/systems/loops/components/run-page/goal-turn-timeline.tsxweb/src/systems/loops/components/run-page/loop-run-page-body.tsxweb/src/systems/loops/components/stories/loop-run-scenario-props.tsweb/src/systems/loops/hooks/__tests__/use-loop-actions.test.tsxweb/src/systems/loops/hooks/__tests__/use-loop-run-form-state.test.tsxweb/src/systems/loops/hooks/__tests__/use-loops.test.tsxweb/src/systems/loops/hooks/use-goal-turns.tsweb/src/systems/loops/lib/__tests__/loop-events.test.tsweb/src/systems/loops/lib/loop-events.tsweb/src/systems/loops/mocks/fixture-run-details.tsweb/src/systems/loops/mocks/handlers.tsweb/src/systems/loops/mocks/materialize-contract-fixture.tsweb/src/systems/os/apps/loops/loop-run-detail-location.tsxweb/src/systems/os/apps/loops/use-loop-run-page.ts
Comment on lines
188
to
+189
| cmd := exec.CommandContext(ctx, "sh", "-c", command) | ||
| cmd.Dir = rootDir |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 4 --type go \
'renderGateCriterion|CriterionCommand|RunCommand\(|Inputs:|StartLoop|CreateLoopRun|ExecuteLoop' \
internalRepository: compozy/compozy
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the gate runner and relevant loop runtime definitions without loading large search output.
wc -l internal/daemon/loop_runtime_adapters.go
sed -n '1,240p' internal/daemon/loop_runtime_adapters.go
# Locate exact gate criterion command definitions and the structs handling loop commands/criteria.
sed -n '1,220p' internal/daemon/task_runtime_adapters.go 2>/dev/null || true
rg -n --type go 'type .*CommandRequest|CommandRequest|CommandResult|type .*Criterion|Check:|Start:|gate\.' internal/internal internal/daemon internal/contract internal/task internal/api/core/loops.go internal/contract internal/dsl internal/loop internal/looppkg 2>/dev/null | head -n 250Repository: compozy/compozy
Length of output: 32610
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- loop command evaluator implementation'
sed -n '1,120p' internal/loop/gate/evaluator_command.go
echo '--- loop command request type'
sed -n '248,275p' internal/loop/gate/types.go
echo '--- command criterion construction sites'
rg -n --type go 'Check:|CriterionCommand|Inputs:|=.*Check|Render.*Check|command|check' internal/loop/gate/evaluator_command.go internal/loop/dsl/gate_start.go internal/loop 2>/dev/null | rg -n 'Render|evaluator_command|gate_start|CriterionCommand|Check:' | head -n 220
echo '--- files containing Render functions for gate criteria'
rg -n --type go 'func .*Render|Render("check"|check|Check|criterion)' internal/loop internal/dsl internal/contract | head -n 220
echo '--- run start/criteria request APIs'
rg -n --type go 'RunLoopRequest|StartLoop|CreateLoopRun|Run:|LoopPlan|loopDefinition|Contract|Inputs' internal/api/core/loops.go internal/api/core/loops_test.go internal/cli/clients.go internal/contract internal/dsl | head -n 260Repository: compozy/compozy
Length of output: 15085
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- RenderGateCriterion and callers'
sed -n '1,180p' internal/loop/control_gate_config.go
rg -n --type go 'RenderGateCriterion|GateCriteria|Criteria|loopDefinition|Definition' internal/loop/control_gate_config.go internal/loop internal/daemon internal/api/core/loops.go internal/contract internal/dsl | head -n 280
echo '--- GateCriterion and CommandRequest fields in source'
sed -n '1,40p' internal/loop/dsl/gate_start.go
sed -n '267,272p' internal/loop/gate/types.go
echo '--- auth/api contracts for run command and loop update paths'
sed -n '1,240p' internal/api/core/loops.go
rg -n --type go 'HandleRunLoop|HandleUpdateLoop|HandleCreateLoop|Permission|Editor|LoopDefinition|Contract|Start|Criteria|Inputs' internal/api/core/loops.go internal/api/core/loops_test.go internal/contract internal/dsl | head -n 320Repository: compozy/compozy
Length of output: 30927
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- internal/loop/service_start resolve/run implementation'
sed -n '1,270p' internal/loop/service_start.go
echo '--- template rendering function and gateway'
sed -n '1,140p' internal/loop/dsl/refs/template.go
rg -n --type go 'RenderTemplateString|ResolveEffectiveInputs|Build.*Effective|effectiveInputs|Values|StartMetadata' internal/loop internal/daemon internal/api/core/loops.go | head -n 260
echo '--- API loop handlers relevant sections'
sed -n '1,180p' internal/api/core/loops.go
sed -n '180,420p' internal/api/core/loops.goRepository: compozy/compozy
Length of output: 36603
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- control_gate.go command check rendering'
sed -n '230,305p' internal/loop/control_gate.go
sed -n '1,220p' internal/loop/service.go
echo '--- RunLoop service implementation'
rg -n --type go 'func \(s \*service\) RunLoop|func \(s \*service\) RunLoopDry|effectiveConfig|effectiveConfig' internal/loop/service.go internal/loop internal/daemon/loop_api_service.go | head -n 240
sed -n '220,340p' internal/loop/service.go
echo '--- GateCriterion Inputs and RenderTemplateString check target sites'
rg -n --type go 'Inputs\[\].*Raw|RenderTemplateString\(["'']check|checkRaw|checkRaw' internal/loop/internal internal/loop/control_gate.go internal/loop/gate internal/loop/dsl/gate_start.go internal/loop/dsl 2>/dev/null | head -n 220
sed -n '1,80p' internal/loop/dsl/gate_start.goRepository: compozy/compozy
Length of output: 12677
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- contract RunLoopRequest and validation'
rg -n --type go 'type RunLoopRequest|struct .*ConfigOverrides|type ConfigOverride|type EffectiveConfig|type LoopConfig|ConfigOverrides|enabled_checks' internal/contract internal/api/contract internal/loop internal/api/core/loops_test.go | head -n 260
echo '--- relevant source files'
for f in internal/api/contract/loop_payloads.go internal/contract/loops.go internal/loop/types.go internal/loop/config.go internal/loop/effective.go internal/loop/gate/evaluator.go internal/loop/gate/types.go; do
if [ -f "$f" ]; then
echo "===== $f ====="
wc -l "$f"
case "$f" in
internal/api/contract/loop_payloads.go) sed -n '1,220p' "$f" ;;
internal/contract/loops.go) sed -n '1,220p' "$f" ;;
internal/loop/types.go) sed -n '1,220p' "$f" ;;
internal/loop/config.go) sed -n '1,220p' "$f" ;;
internal/loop/effective.go) sed -n '1,260p' "$f" ;;
internal/loop/gate/evaluator.go) sed -n '180,260p' "$f" ;;
internal/loop/gate/types.go) sed -n '248,275p' "$f" ;;
esac
fi
done
echo '--- validation tests that use enabled_checks'
rg -n --type go 'enabled_checks|PatchLoopRequest|LoopConfig|ConfigOverrides' internal/api/core/loops_test.go internal/daemon/loop_api_runs_test.go internal/daemon/loop_goal_command_e2e_integration_test.go | head -n 220Repository: compozy/compozy
Length of output: 2391
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- LoopConfig contract'
sed -n '180,270p' internal/api/contract/loops.go
echo '--- RunLoopRequest validation'
sed -n '205,250p' internal/api/core/loops.go
echo '--- loop config construction and enabled check parsing'
sed -n '200,455p' internal/loop/config.go
sed -n '27,45p' internal/loop/service_types.goRepository: compozy/compozy
Length of output: 11002
Do not execute shell-interpolated command checks. command.Check and command.Contains are template-rendered before exec.CommandContext(..., "sh", "-c", command), so a template from inputs.values or config_overrides.enabled_checks_json can insert shell syntax and run arbitrary commands in the resolved workspace. Require command edit/config-author permissions for these fields, or move to an explicit executable-with-argv contract.
🧰 Tools
🪛 OpenGrep (1.26.0)
[ERROR] 188-188: Dynamic command passed to exec.Command with a shell invocation. Pass arguments directly to exec.Command without a shell wrapper.
(coderabbit.command-injection.go-exec-command)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/daemon/loop_runtime_adapters.go` around lines 188 - 189, Update the
command execution flow around exec.CommandContext in the runtime adapter to
prevent shell-interpolated command checks from executing untrusted
template-rendered values. Require command edit or configuration-author
authorization for command.Check and command.Contains inputs, or replace shell
execution with an explicit executable-and-argv contract that cannot interpret
injected shell syntax.
Source: Linters/SAST tools
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
internal/daemon/loop_runtime_adapters.go (1)
188-190:⚠️ Potential issue | 🟠 MajorDo not treat the
#noseccomment as a shell-injection fix.
exec.CommandContext(ctx, "sh", "-c", command)still interpretsreq.Commandas shell code. The comment does not enforceshellQuotefor every dynamic value or restrict authored commands to trusted principals. Replace this with an executable-and-argv contract, or prove and test the shell-quoting and authorization invariants before keeping the suppression.Verification command
#!/usr/bin/env bash set -euo pipefail rg -n -C 8 --type go \ 'shellQuote|Render.*Command|CommandRequest|enabled_checks_json|exec\.CommandContext\(ctx,\s*"sh",\s*"-c"' \ internal/loop internal/daemon internal/api🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/daemon/loop_runtime_adapters.go` around lines 188 - 190, Replace the shell-based execution in the command handling path around exec.CommandContext with an executable-and-argv contract so req.Command is not interpreted as shell code. Update the command rendering/request flow to return a validated executable and argument list, and invoke them directly; if shell execution must remain, establish authorization and complete shellQuote invariants with tests before retaining the `#nosec` suppression.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/loop/dsl/refs/template_command.go`:
- Around line 49-101: Extend validateShellCommandNode and its branch traversal
to track whether each action is inside an active shell quote context, and reject
shellQuoteTemplateFunction interpolations when surrounding literal text uses
shell quotes that the rendered value can terminate. Apply this validation to
complete shell word content rather than only the action pipeline, and add a
regression proving `echo "{{ .inputs.value | shellQuote }}"` fails compilation.
In `@internal/loop/goal/executor_test.go`:
- Around line 1559-1562: Replace the invalid new(1) initializer in the
CriterionResult test fixture with a declared int variable initialized to 1, then
assign its address to ExitCode while preserving the existing rejected verdict
and hostileStderr values.
---
Duplicate comments:
In `@internal/daemon/loop_runtime_adapters.go`:
- Around line 188-190: Replace the shell-based execution in the command handling
path around exec.CommandContext with an executable-and-argv contract so
req.Command is not interpreted as shell code. Update the command
rendering/request flow to return a validated executable and argument list, and
invoke them directly; if shell execution must remain, establish authorization
and complete shellQuote invariants with tests before retaining the `#nosec`
suppression.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e7c30f56-ec29-4863-b51f-60245ee5aa84
⛔ Files ignored due to path filters (5)
docs/qa/reports/2026-08-10-loop-convergence.mdis excluded by!**/*.mddocs/qa/scenarios/LP-loop-template-snapshot-round-trip.mdis excluded by!**/*.mdpackages/site/content/docs/loops/dsl-reference.mdxis excluded by!**/*.mdxpackages/site/content/docs/loops/goal-node.mdxis excluded by!**/*.mdxskills/compozy/references/loops.mdis excluded by!**/*.md
📒 Files selected for processing (19)
internal/daemon/loop_runtime_adapters.gointernal/loop/compiler.gointernal/loop/control_gate.gointernal/loop/coordinator_test.gointernal/loop/dsl/refs/namespace.gointernal/loop/dsl/refs/refs_test.gointernal/loop/dsl/refs/template.gointernal/loop/dsl/refs/template_command.gointernal/loop/gate/evaluator_test.gointernal/loop/goal/executor_test.gointernal/loop/goal/prompt_diagnostics.gointernal/loop/goal/templates.gointernal/loop/linter_references.gointernal/loop/linter_test.gointernal/store/globaldb/global_db_goal_judge_integration_test.gointernal/store/globaldb/global_db_goal_turn_runtime_integration_test.goweb/src/systems/loops/components/stories/loop-run-scenario-props.tsweb/src/systems/loops/lib/__tests__/loop-events.test.tsweb/src/systems/loops/lib/loop-events.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- web/src/systems/loops/components/stories/loop-run-scenario-props.ts
- internal/store/globaldb/global_db_goal_turn_runtime_integration_test.go
- internal/loop/compiler.go
- internal/loop/coordinator_test.go
- internal/loop/control_gate.go
- internal/store/globaldb/global_db_goal_judge_integration_test.go
- internal/loop/gate/evaluator_test.go
- web/src/systems/loops/lib/loop-events.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
internal/loop/dsl/refs/template_command.go (2)
192-211: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider comparing branch states with the entry state for
IfNodeandWithNode.
validateShellCommandRangerequires both branches to equal the entrystate(line 184).validateShellCommandBranchesonly requires the two branches to equal each other (line 205). A template such as{{if .x}}'{{else}}'{{end}}foo{{if .x}}'{{else}}'{{end}}therefore passes with a conditional quote span. The final check at line 75 still rejects an unbalanced end state, so this is not exploitable today. The asymmetry is intentional if you want to allow authored conditional quoting; add a short comment stating that intent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/loop/dsl/refs/template_command.go` around lines 192 - 211, Update validateShellCommandBranches to require both validated branch states to match the entry state, consistent with validateShellCommandRange, preventing conditional quote-context changes across IfNode and WithNode branches. If conditional quoting is intentionally supported, retain the current comparison and add a brief comment documenting that behavior instead.
78-82: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument the global scope of the
<<rejection.
hasDoubleLessandemitsDynamicare validator-wide flags. Any authored<<anywhere in the template blocks every dynamic interpolation in that template, even in an unrelated branch. This is a safe, conservative choice. Add a short comment so a later reader does not narrow it to positional tracking without understanding the heredoc risk.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/loop/dsl/refs/template_command.go` around lines 78 - 82, Add a concise comment immediately before the unsafeCommandTemplateError check explaining that the validator-wide hasDoubleLess and emitsDynamic flags intentionally reject any dynamic interpolation when << appears anywhere in the template, including unrelated branches, as a conservative heredoc safety rule.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/loop/dsl/refs/template_command.go`:
- Around line 192-211: Update validateShellCommandBranches to require both
validated branch states to match the entry state, consistent with
validateShellCommandRange, preventing conditional quote-context changes across
IfNode and WithNode branches. If conditional quoting is intentionally supported,
retain the current comparison and add a brief comment documenting that behavior
instead.
- Around line 78-82: Add a concise comment immediately before the
unsafeCommandTemplateError check explaining that the validator-wide
hasDoubleLess and emitsDynamic flags intentionally reject any dynamic
interpolation when << appears anywhere in the template, including unrelated
branches, as a conservative heredoc safety rule.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 789f000f-ba74-4581-bdc4-f7985ed953f6
⛔ Files ignored due to path filters (5)
docs/qa/reports/2026-08-10-loop-convergence.mdis excluded by!**/*.mddocs/qa/scenarios/LP-loop-template-snapshot-round-trip.mdis excluded by!**/*.mdpackages/site/content/docs/loops/dsl-reference.mdxis excluded by!**/*.mdxpackages/site/content/docs/loops/goal-node.mdxis excluded by!**/*.mdxskills/compozy/references/loops.mdis excluded by!**/*.md
📒 Files selected for processing (5)
internal/loop/action_goal_params.gointernal/loop/dsl/refs/namespace.gointernal/loop/dsl/refs/refs_test.gointernal/loop/dsl/refs/template_command.gointernal/loop/goal/executor_test.go
🚧 Files skipped from review as they are similar to previous changes (3)
- internal/loop/dsl/refs/namespace.go
- internal/loop/dsl/refs/refs_test.go
- internal/loop/goal/executor_test.go
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/loop/dsl/refs/refs_test.go`:
- Line 217: Update the template case in the relevant Go test so the if and else
branches produce different quote contexts, allowing the rendered or compiled
result to depend on .inputs.done. Assert the resulting behavior for both
condition values; if quote transitions inside conditionals are intentionally
rejected, rename the test and assert that rejection instead.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 39ba5b62-5b72-4d3d-b2ef-3486f2737baf
⛔ Files ignored due to path filters (2)
docs/qa/reports/2026-08-10-loop-convergence.mdis excluded by!**/*.mddocs/qa/scenarios/LP-loop-template-snapshot-round-trip.mdis excluded by!**/*.md
📒 Files selected for processing (2)
internal/loop/dsl/refs/refs_test.gointernal/loop/dsl/refs/template_command.go
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/loop/dsl/refs/template_command.go
| t.Run("Should reject conditional shell quote context changes", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| raw := `{{ if .inputs.done }}'{{ else }}'{{ end }}value{{ if .inputs.done }}'{{ else }}'{{ end }}` |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the conditional branches exercise different quote contexts.
Both if and else branches emit ' in both conditionals. The template always renders 'value', regardless of .inputs.done. This does not test a branch-dependent quote-context change. Make the branches produce different quote states, or rename the test if the compiler intentionally rejects any quote transition inside a conditional.
As per path instructions, Go tests must test meaningful behavior and verify behavior outcomes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/loop/dsl/refs/refs_test.go` at line 217, Update the template case in
the relevant Go test so the if and else branches produce different quote
contexts, allowing the rendered or compiled result to depend on .inputs.done.
Assert the resulting behavior for both condition values; if quote transitions
inside conditionals are intentionally rejected, rename the test and assert that
rejection instead.
Source: Path instructions
pedronauck
dismissed stale reviews from coderabbitai[bot], coderabbitai[bot], and coderabbitai[bot]
August 10, 2026 13:43
Superseded by the verified fixes from CodeRabbit rounds 1-3 and final closeout commit 1f539a0.
Member
Author
|
Final closeout: commit 1f539a0 adds the browser runtime/E2E stabilization found by the complete lane. The official daemon-served Playwright suite passed with no failed tests. make gate-full is pass CURRENT for fingerprint db836080f6df76ebf39dbc345852d95229d4179d (21,361 Go tests, 2 intentional skips). Isolated Web/runtime QA audit: PASS, 0 blockers, 0 warnings; teardown clean with zero survivors. Marketplace update-only manual paths remain honestly blocked-verify because the official isolated catalog exposed no update candidate; the seeded E2E update-confirm contract passed. This is the post-review closeout; no fourth CodeRabbit round is requested. |
This was referenced Aug 10, 2026
Merged
Merged
Merged
This was referenced Aug 11, 2026
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
This was referenced Aug 18, 2026
Merged
Merged
Merged
Merged
This was referenced Aug 27, 2026
Merged
Merged
This was referenced Sep 7, 2026
Merged
Merged
Merged
This was referenced Sep 15, 2026
Merged
Merged
This was referenced Sep 27, 2026
Merged
Merged
This was referenced Oct 6, 2026
Merged
Merged
Merged
This branch was successfully deployed
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.
Summary
This fixes two critical Loop failures that prevented production convergence:
compozy__goal_report, and rejected turns retain durable, sanitized diagnostics.The public contract now exposes both views deliberately:
contractremains raw for inspection and round-tripping;materialized_contractis the immutable execution snapshot shown in run previews and progress surfaces.Root cause
The production incident was reproduced from
looprun-944d89b8ccf4999din.tmp/compozy-dev-home:{{ .inputs.slug }}survived compilation and reached agents and command judges literally;command_expectation_failed, so the UI could not explain the repeated rejection;Together these made completed work look incomplete and left the Loop running until its round limit.
Runtime and contract changes
00058_schema.sqland regenerate sqlc/OpenAPI artifacts.Web and operator experience
Documentation and official skill
skills/compozy/references/loops.mdwith the raw/materialized contract and Goal reporting behavior.docs/qa/.Verification
make gate-full: PASS2d227f0e372e750587de36c6646e290867080ade.cache/gate/logs/full-1786337342.logmake codegen-check: PASSIsolated real-scenario QA
looprun-63a9f3bae9fa958dws_e258a8baa7d7851csess_f445c65958bca2c8d289de70f9582edcLP-goal-command-judge: PASSLP-loop-template-snapshot-round-trip: PASSLP-run-detail-story-redesign: PASSteardown.jsonreports"clean": trueThe QA run proved a rejected command turn with durable diagnostics, a succeeding second turn, successor execution, and terminal run state. It also proved that literal template-shaped input remains literal and that raw/materialized contracts remain distinct.
Settings and compatibility
config.tomlkeys, defaults, or settings surfaces changed.Compozy Impact Audit
compozy__goal_reportavailability is consistent for Goal sessions;compozy__loop_statusandcompozy__loop_turnsexpose structured verdict diagnostics. Descriptors, generated schemas, catalog fixtures, and native-tool tests were updated.workspace_idand execute in that workspace. Goal reporting validates run/session ownership. Turn diagnostics are run-scoped in GlobalDB and projected only through the owning run/API/SSE/native-tool paths, with cross-workspace tests.skills/compozy/references/loops.mddocuments the public raw/materialized contract, Goal reporting capability, and diagnostic behavior.Commits
9eaaf307— runtime, storage, contracts, Web, migration, docs, and regression coverage05f511d7— release-gate refactors and isolated QA evidenceSummary by CodeRabbit
New Features
Bug Fixes