Repository navigation
fix: make cy-qa-workflow extension work on compozy.tasks/v2 runs - #234
Conversation
WalkthroughTask creation now supports v2 task graph manifests while preserving legacy indexes. New manifest utilities handle rendering and graph updates. The verification pipeline builds, tests, and lints configured standalone Go extensions. ChangesTask graph creation
Standalone extension verification
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant HostCreateTask
participant TaskGraphUtilities
participant TasksManifest
participant TaskFile
HostCreateTask->>TaskGraphUtilities: Load and classify _tasks.md
HostCreateTask->>TaskGraphUtilities: Render task graph task file
TaskGraphUtilities->>TaskFile: Write task_02.md
HostCreateTask->>TaskGraphUtilities: Append task node and dependency edge
TaskGraphUtilities->>TasksManifest: Rewrite _tasks.md
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bbdc60f6b1
ℹ️ 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".
|
I found some issues when trying locally, I'll put again as ready to review it when I have it working well. |
bbdc60f to
6bd08af
Compare
…-aware The cy-qa-workflow extension aborted every `compozy tasks run` on a compozy.tasks/v2 workflow. Two independent defects were involved. Defect 1 — missing go.mod. The extension ships `[subprocess] go run .` and imports the SDK, but was committed without go.mod/go.sum, so a standalone install failed with "go.mod file not found". It only compiled in-repo because it was absorbed into the root module. Package it as the standalone module the docs and scaffolder prescribe (module example.com/cy-qa-workflow, require github.com/compozy/compozy v0.2.11). Defect 2 — host.tasks.create only understood the legacy Markdown-table index. It rejected a v2 frontmatter-graph _tasks.md with "tasks table header not found" and wrote a dependencies key the v2 validator rejects. Make the writer format-aware: for a compozy.tasks/v2 manifest, render the task file without a dependencies key and append a node (+ dependency edges) to the frontmatter graph so injected tasks are visible to the runner. Legacy Markdown-table behavior is unchanged. Also keep the now-separate extension module covered: a verify-extensions make target wired into verify, extensions/** in the CI paths filter, and go.work*/extension binaries gitignored for local workspace-based dev. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
6bd08af to
e0d78f5
Compare
|
Good catch — fixed. Raised Caveat: this PR is the host-side graph fix, so the "first release containing the new graph branch" isn't tagged yet — the current latest release (v0.2.13) predates it. Without the raise, the newly added |
|
Re-validated against a real |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Makefile (1)
162-173: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider extracting the per-module verification into a helper target.
checkmake flags this target body as exceeding the 5-line limit (8 lines). Extracting the inner loop body into a separate recipe target would improve readability and satisfy the linter.
♻️ Optional refactor to reduce target body length
EXTENSION_GO_MODULES=extensions/cy-qa-workflow + +define verify-extension-module + echo "Verifying extension module $$dir"; \ + ( cd $$dir && \ + $(GOBUILD) -o /dev/null ./... && \ + $(GOTEST) -race ./... && \ + $(LINTCMD) run ) || exit 1 +endef # ... verify-extensions: - `@for` dir in $(EXTENSION_GO_MODULES); do \ - echo "Verifying extension module $$dir"; \ - ( cd $$dir && \ - $(GOBUILD) -o /dev/null ./... && \ - $(GOTEST) -race ./... && \ - $(LINTCMD) run ) || exit 1; \ - done + `@for` dir in $(EXTENSION_GO_MODULES); do \ + $(verify-extension-module); \ + done `@echo` "$(GREEN)Extension modules verified$(NC)"🤖 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 `@Makefile` around lines 162 - 173, Extract the per-module build, test, and lint commands from verify-extensions into a dedicated helper target, passing each module directory as an argument. Keep verify-extensions focused on iterating over EXTENSION_GO_MODULES and invoking the helper, preserving the existing failure handling and verification message while reducing the target body below checkmake’s five-line limit.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.
Nitpick comments:
In `@Makefile`:
- Around line 162-173: Extract the per-module build, test, and lint commands
from verify-extensions into a dedicated helper target, passing each module
directory as an argument. Keep verify-extensions focused on iterating over
EXTENSION_GO_MODULES and invoking the helper, preserving the existing failure
handling and verification message while reducing the target body below
checkmake’s five-line limit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 873fb549-d77d-483d-9793-3276e5305866
⛔ Files ignored due to path filters (3)
.github/workflows/ci.ymlis excluded by!**/*.ymlextensions/cy-qa-workflow/extension.tomlis excluded by!**/*.tomlextensions/cy-qa-workflow/go.sumis excluded by!**/*.sum
📒 Files selected for processing (8)
.gitignoreMakefileextensions/cy-qa-workflow/go.modinternal/core/extension/host_writes.gointernal/core/extension/host_writes_test.gointernal/core/tasks/manifest_write.gointernal/core/tasks/manifest_write_test.gotest/frontend-verification-contract.test.ts
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (6)
- extensions/cy-qa-workflow/go.mod
- internal/core/tasks/manifest_write_test.go
- test/frontend-verification-contract.test.ts
- internal/core/tasks/manifest_write.go
- internal/core/extension/host_writes_test.go
- internal/core/extension/host_writes.go
pedronauck
left a comment
There was a problem hiding this comment.
The PR-specific implementation and local gates passed in my audit, but the required GitHub verify check on head e0d78f56654534e2544edea96e7dafb7d26fc7ce is red, so this cannot be merged under the repository's zero-issue policy. The failing log is internal/core/run/ui/multi_remote.go:1038:21: string merged has 4 occurrences, make it a constant (goconst).
I confirmed that file is identical between this head and current main, so this is a base/environment failure rather than a defect introduced by #234. Please update the branch after the main Go 1.26.4 lint failure is fixed (or coordinate the base fix first), trigger a fresh CI run, and return with the required verify check green. I am not requesting a workaround or unrelated suppression in this PR; the blocking condition is the failed canonical gate.
pedronauck
left a comment
There was a problem hiding this comment.
Re-review completed at head 1ffb01eeb19285ec9925ea065ff63b7ecee8d79d. The prior host-version concern is resolved, no active review threads remain, and the current merge ref 87427a84bb861660045df70014905d5be5211993 passed CI run 29539072178 end to end: zero lint issues, 4,775 tests with 4 intentional skips, extension-module verification, build, and Playwright. Approved for squash merge.
pedronauck
left a comment
There was a problem hiding this comment.
Current head and exact merge tree are clear. Canonical workflow run 29542823245 verified merge SHA 4f4e66a: formatting passed, golangci-lint reported 0 issues, 4,798 Go tests passed (4 intentional skips), build and cy-qa-workflow module verification passed, and 5 Playwright E2E tests passed.
cy-capture-decisions (compozy#237) and cy-qa-workflow (compozy#234) are independent skill-only extensions with zero coupling to task groups; they belong in their own PRs. Removing them keeps this PR scoped to Task Groups.
Reverts an over-eager removal: cy-qa-workflow already ships in main (this branch only carries its compozy#234 module-packaging/version bump), so it must stay. Only cy-capture-decisions (genuinely new on this branch) remains removed.
Reverts six PRs that rode along on this branch but are independent of the Task Groups feature (verified: no task-group code depends on them): - compozy#234 cy-qa-workflow module + host.tasks.create v2 (extension SDK) - compozy#237 cy-capture-decisions extension - compozy#226 Kiro CLI ACP model handling - compozy#248 sync-test isolation (touched cy-capture-decisions) - compozy#252 release v0.2.15 prep - compozy#253 test-daemon reaping Kept the task-group-relevant infra: compozy#230 (multi-run recovery), compozy#236 (reviews-fix progress), compozy#241 (parallel task status), compozy#250 (task artifacts).
Summary
The
cy-qa-workflowextension aborts everycompozy tasks runon acompozy.tasks/v2workflow. This PR fixes the two independent root-cause defects behind it. A follow-up PR adds a configurable QA runtime on top of this.Why
With
cy-qa-workflowenabled, launching any v2 (frontmatter-graph) task workflow fails during plan discovery, beforerun.started, and no task jobs are created. Because the extension'splan.pre_discoverhook isrequired = true, its failure aborts the whole run. Each aborted run also leaves a partial artifact that can flip the daemon todegraded. The only workaround today is to disable the extension entirely, which loses QA-task automation.There are two separate causes:
go.mod, so itsgo run .subprocess can't even start once installed standalone.host.tasks.createcall is rejected by the daemon, because that host method only understands the legacy Markdown-table_tasks.md, not the v2 frontmatter-graph format thatcy-create-tasksgenerates and the runner requires.What changed
Defect 1 — extension had no Go module
The installed extension contained only
extension.toml+main.go, butextension.tomlruns the subprocess asgo run .andmain.goimportsgithub.com/compozy/compozy/sdk/extension. With nogo.mod, the subprocess failed immediately withgo: go.mod file not found.extensions/cy-qa-workflow/go.mod(new) — makes the extension the standalone module the docs/scaffolder prescribe. Module pathexample.com/cy-qa-workflow: a deliberately non-publishable path (example.comis RFC 2606 reserved), matching whatcreate-extensionemits (defaultModuleName → example.com/<name>) anddocs/extensibility/hello-world-go.md(go mod init example.com/hello-go). The module is an app run viago run ., never imported, so the path is only an identifier.extensions/cy-qa-workflow/go.sum(new) — generated bygo mod tidy; pins the SDK + transitive deps.Defect 2 —
host.tasks.createwasn'tcompozy.tasks/v2-awarehost.tasks.createsearched for the legacy header| # | Title | Status | Complexity | Dependencies |and returned "tasks table header not found" on a v2 manifest, aborting the run atextension.ready. It now detects the manifest format and mutates the v2 graph instead.internal/core/tasks/manifest_write.go(new) — three helpers on the manifest type:IsTaskGraphManifestContent— detects a v2 manifest (schema version orgraph.nodes).AppendTaskGraphNode— adds a node tograph.nodesand dependency edges tograph.edges(dep → new), deduplicated; errors on a dangling dependency so an invalid manifest is never written; re-serializes the frontmatter.RenderTaskGraphTaskFile— renders a v2 task file with only task-owned front matter (nodependencieskey, which the v2 validator rejects on task files).internal/core/extension/host_writes.go—CreateTaskis now format-aware: it reads_tasks.mdonce (loadTaskIndexState) and branches the task-file rendering (renderHostTaskFile) and the index update (buildTaskIndexUpdate). For a v2 manifest it renders withoutdependenciesand appends node + edges; the legacy Markdown-table path is unchanged.Build coverage
Adding a
go.modmakes the extension a nested module, which Go excludes from the root./..., somake verifywould silently stop covering it.Makefile— newverify-extensionstarget (build-o /dev/null,go test -race,golangci-lint runper module listed inEXTENSION_GO_MODULES), wired intoverify..github/workflows/ci.yml—extensions/**added to the CIchangespaths filter so extension-only changes still triggerverify..gitignore— ignorego.work/go.work.sum(local dev) and the compiledcy-qa-workflowbinary fromgo build ..test/frontend-verification-contract.test.ts— theverifytarget-dependency contract now includesverify-extensions.Tests
internal/core/tasks/manifest_write_test.go(new) — table-driven coverage for the three helpers, plus an end-to-end case that writes the files and runs the realLoadValidatedTaskGraphManifestto prove the mutated manifest is valid.internal/core/extension/host_writes_test.go—TestHostTasksCreateUpdateIndexAppendsTaskGraphNodedrives the realhost.tasks.createRPC on a v2_tasks.mdand asserts the node + edge injection, the missingdependencieskey, and that the runner's own validator loads the result.Behavior notes
_tasks.mdpath is unchanged; only v2 manifests take the new branch.AppendTaskGraphNodereturns an error if a dependency doesn't resolve to an existing graph node.Testing
TestHostTasksCreateUpdateIndexAppendsTaskGraphNodefails on the pre-fix code with the exact production error (tasks table header not found) and passes with the fix.make verify-extensions— build +-race+ lint, 0 issues.compozy tasks runon a v2 workflow reachesrun.started, injects the QA tasks into the graph, and an agent creates/refactors a file to completion.CI note
If the
verifyjob fails on agoconstininternal/core/run/ui/multi_remote.go("merged"), that is a pre-existing issue onmain(main's own CI is currently red on it), not introduced by this change — a rebase clears it once main's lint is green. This branch is rebased on currentmain.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Chores
verifypipeline to build, test (with race checks), and lint extension modules automatically.