OAPE-802: OpenSpec Integration commit#164
Conversation
Signed-off-by: Bharath B <bhb@redhat.com>
|
@bharath-b-rh: This pull request references OAPE-802 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set. DetailsIn response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
WalkthroughThis PR adds an extensive OpenSpec "agile-workflow" tooling suite for the external-secrets-operator repository: Cursor commands and skills (OAPE commands, opsx-* workflow commands), e2e test generator fixtures/docs, root documentation (AGENTS.md, CLAUDE.md, README, constitution.md), an evals retrospective pipeline, and the openspec-agile-workflow schema package (schema.yaml, templates, stage-gate prompts, eval specs). All changes are documentation/configuration files (Markdown/YAML); no source code or exported entities are modified. ChangesOAPE Cursor Commands and Skills
Estimated code review effort: 3 (Moderate) | ~25 minutes E2E Test Generator Fixtures
Root Repository Documentation
Evals Retrospective Pipeline
OpenSpec Agile Workflow Schema Package
Estimated code review effort: 3 (Moderate) | ~30 minutes Suggested labels: documentation, skip_review Suggested reviewers: none identified from provided context Poem:
✨ Finishing Touches🧪 Generate unit tests (beta)
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bharath-b-rh The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (28)
.cursor/commands/api-implement.md-168-189 (1)
168-189: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRelax the repository gate.
This precheck will reject valid controller-runtime repos that do not directly import
github.com/openshift/api. Validate the operator layout/framework instead of a specific dependency.🤖 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 @.cursor/commands/api-implement.md around lines 168 - 189, The repository validation in the precheck is too strict because it only accepts repos that look like OpenShift operators via a direct dependency check; update the logic in the precheck block around REPO_ROOT and GO_MODULE to validate the operator/controller-runtime layout or framework instead of requiring a specific OpenShift API import. Keep the git and go.mod checks, but replace the dependency-specific gate with a broader repo-structure or framework-based test so valid controller-runtime repositories pass..cursor/commands/api-generate.md-179-189 (1)
179-189: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRelax the repository gate.
Requiring
github.com/openshift/apihere will reject valid operator repos before discovery runs. This precheck should validate the operator layout / Go module, not a specific dependency.🤖 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 @.cursor/commands/api-generate.md around lines 179 - 189, The repository precheck is too strict because it gates on github.com/openshift/api and can reject valid operator repos before discovery. Update the check in the api-generate command flow to validate the operator repo layout or Go module identity more generically, and remove the dependency-specific grep from the precheck logic. Keep the existing discovery/confirmation behavior, but base it on repo structure or module metadata rather than the openshift/api dependency string..cursor/commands/eval-loop.md-72-74 (1)
72-74: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSync evals into the installed schema path.
These targets drop the
openspec/prefix used by the installed schema package, so the forward consumer will never see the refreshed eval files.🔧 Proposed fix
-- schemas/openspec-agile-workflow/evals/<stage>_eval.yaml ++ openspec/schemas/openspec-agile-workflow/evals/<stage>_eval.yamlAlso applies to: 91-95
🤖 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 @.cursor/commands/eval-loop.md around lines 72 - 74, The eval sync targets are written without the installed schema package prefix, so the forward consumer cannot find the refreshed files. Update the sync logic in the eval-loop flow to write merged files under the installed openspec schema path, specifically using openspec/schemas/agile-workflow/evals/<stage>_eval.yaml and the corresponding code-generation_eval.yaml for the per-task gate, while keeping the template mapping to templates/<name>.md where applicable. Ensure the existing eval-generation step names and sync paths are aligned so /opsx-continue and /opsx-apply read from the installed schemas location rather than the prefixless path..cursor/commands/analyze-rfe.md-69-71 (1)
69-71: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFix the skill reference path.
This points to
plugins/oape/skills/analyze-rfe/SKILL.md, but the rest of this PR stores workflow skills under.cursor/skills/, so the command will look in the wrong place.🔧 Proposed fix
-- `plugins/oape/skills/analyze-rfe/SKILL.md` +- `.cursor/skills/analyze-rfe/SKILL.md`🤖 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 @.cursor/commands/analyze-rfe.md around lines 69 - 71, The skill reference in the analyze-rfe command points to the wrong location, so update the referenced path in the markdown command to use the existing `.cursor/skills/` layout instead of `plugins/oape/skills/analyze-rfe/SKILL.md`. Check the “For detailed implementation” reference in `analyze-rfe.md` and correct it to the matching `analyze-rfe` skill file under `.cursor/skills/` so the command resolves the intended workflow skill..cursor/commands/review.md-36-42 (1)
36-42: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDon't hardcode
origin/masteras the fallback branch.This repo targets
main, so defaulting toorigin/masterwill diff against the wrong ref or fail outright on repositories that no longer havemaster. Derive the remote default branch dynamically or change the fallback to match the repo's actual default branch.🤖 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 @.cursor/commands/review.md around lines 36 - 42, The base ref fallback in the review command is hardcoded to origin/master, which can point at the wrong branch for this repo. Update the base-ref selection logic in the command script so it defaults to the repository’s actual default branch (for example by deriving it dynamically from the remote) or at least uses origin/main instead, and keep the existing $2 override behavior intact..cursor/commands/review.md-151-159 (1)
151-159: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep
/oape:reviewoutput pure JSON.Appending a fix summary after the JSON will break the downstream parser in
.cursor/commands/implement-review-fixes.md:25-72, which expects the review report itself as input. Emit the summary separately instead of on the same stream. Based on the downstream contract in.cursor/commands/implement-review-fixes.md:25-72.Also applies to: 209-209
🤖 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 @.cursor/commands/review.md around lines 151 - 159, The review command currently mixes a fix summary into the same output stream as the JSON report, which breaks the downstream parser used by implement-review-fixes.md. Update the review flow around the Step 5 logic in review.md so /oape:review emits only pure JSON as its primary output, and send any automatic fix summary separately rather than appending it after the report. Keep the review report contract unchanged for the downstream consumer..cursor/commands/predict-regressions.md-571-579 (1)
571-579: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winActually write the report to disk.
This step only creates the directory and echoes
REPORT_FILE; it never writes the generated markdown, so the command cannot satisfy its output contract or hand a file to downstream tooling.🔧 Suggested fix
mkdir -p "$OUTPUT_DIR" REPORT_FILE="$OUTPUT_DIR/regression-report.md" -# Write the generated report content to the file -echo "Report written to: $REPORT_FILE" +cat > "$REPORT_FILE" <<'EOF' +{generated regression report markdown} +EOF +echo "Report written to: $REPORT_FILE"🤖 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 @.cursor/commands/predict-regressions.md around lines 571 - 579, The “Write the Report” step in the regression report workflow only creates the output directory and prints REPORT_FILE, but never persists the generated markdown to disk. Update the report-writing logic in the relevant command flow so the generated report content is actually written into REPORT_FILE after mkdir -p succeeds, using the existing report generation/output variables in predict-regressions.md. Keep the final echo if needed, but ensure the file is created with the full markdown content so downstream tooling can consume it.openspec/schemas/openspec-agile-workflow/feedback_stage_artifacts/README.md-15-19 (1)
15-19: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMatch the shared rejection-round path to
schema.yaml.
schema.yamlwrites the co-generated round file underfeedback_stage_artifacts/repo-assessment/round-<N>.yaml, sorepo-assessment+constitution/round-<N>.yamlpoints at the wrong location and will confuse the workflow. Update the README to the schema-backed path.🛠 Proposed fix
- openspec/changes/<change-name>/feedback_stage_artifacts/repo-assessment+constitution/round-<N>.yaml + openspec/changes/<change-name>/feedback_stage_artifacts/repo-assessment/round-<N>.yaml🤖 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 `@openspec/schemas/openspec-agile-workflow/feedback_stage_artifacts/README.md` around lines 15 - 19, The shared rejection-round path in the README does not match what `schema.yaml` actually writes for the co-generated `repo-assessment` flow, so update the documented path under `feedback_stage_artifacts` to use the schema-backed `repo-assessment/round-<N>.yaml` location. Keep the wording aligned with the existing `schema.yaml` output for the co-generated gates and remove the incorrect `repo-assessment+constitution` path so readers follow the actual artifact layout.openspec/schemas/openspec-agile-workflow/evals/stages/constitution/eval-spec.yaml-16-25 (1)
16-25: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
pass_thresholdrequired as top-level field but example nests it underscoring.
required_fields(Lines 16-25) listspass_thresholdas a required top-level field, but theexample(Lines 33-45) only places it insidescoring.pass_threshold. This same pattern repeats in theimplementation,plan,repo-assessment, andtaskseval-spec files. Whichever consumer parses required fields for validation will either flag valid examples as non-compliant or silently accept cases missing a genuine top-levelpass_threshold.📝 Proposed fix (apply to all 5 affected files)
required_fields: - id - round - stage - source_issue_ids - input_refs - assertions - scoring - - pass_thresholdOr alternatively, update the examples to hoist
pass_thresholdto the top level if that is the intended schema.Also applies to: 33-45
🤖 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 `@openspec/schemas/openspec-agile-workflow/evals/stages/constitution/eval-spec.yaml` around lines 16 - 25, The eval spec schema is inconsistent because `required_fields` in the constitution, implementation, plan, repo-assessment, and tasks `eval-spec` files պահանջs a top-level `pass_threshold`, while the `example` only sets `scoring.pass_threshold`. Update the schema so `required_fields` and the example match the intended contract by either moving `pass_threshold` to the top level everywhere or removing it from `required_fields` if it should remain under `scoring`; use the `required_fields` and `example` sections in each `eval-spec` file to keep the structure consistent..cursor/skills/openspec-sync-specs/SKILL.md-49-80 (1)
49-80: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDerive the main spec path from the status context
openspec/specs/<capability>/spec.mdshould not be hardcoded in the sync flow. Use the path context fromopenspec status(planningHome/changeRoot/artifactPaths) for both reading and creating the main spec so sync works in nested homes and non-default stores.🤖 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 @.cursor/skills/openspec-sync-specs/SKILL.md around lines 49 - 80, The sync flow hardcodes the main spec location instead of using the status-derived path context. Update the delta-to-main spec logic in SKILL.md to derive the main spec path from the openspec status context (planningHome, changeRoot, and artifactPaths) for both reading and creating specs, and replace references to openspec/specs/<capability>/spec.md with the resolved path so nested homes and non-default stores work correctly..cursor/skills/openspec-sync-specs/SKILL.md-22-35 (1)
22-35: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winThread the selected store through the lookup steps.
openspec listandopenspec statusboth need the selected--store <id>context here; otherwise this flow reads the nearest local root instead of the chosen store.💡 Suggested fix
- openspec list --json + openspec list --store "<id>" --json @@ - openspec status --change "<name>" --json + openspec status --store "<id>" --change "<name>" --json🤖 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 @.cursor/skills/openspec-sync-specs/SKILL.md around lines 22 - 35, The selection flow in the openspec sync skill is missing the chosen store context, so both openspec list and openspec status should be run with the same --store <id> that the user selected. Update the prompt/selection logic in SKILL.md to carry the selected store through the lookup steps, and make sure the AskUserQuestion-based choice is followed by using that store ID when resolving the change context with openspec status..cursor/skills/eval-loop/SKILL.md-23-35 (1)
23-35: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUse the installed schema root here.
This repo’s schema package lives under
openspec/schemas/openspec-agile-workflow/, so theschemas/...literals point at a non-existent path. Update both the eval sync target and the templates path.🤖 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 @.cursor/skills/eval-loop/SKILL.md around lines 23 - 35, The eval-loop guidance still points to the wrong schema root, so the literal paths under schemas/... are incorrect for this repo. Update the SKILL.md instructions to use the installed schema root under openspec/schemas/openspec-agile-workflow/ for the eval sync target, and make the template path wording consistent with that root while keeping eval reads/writes on evals/refined-templates/..cursor/skills/openspec-propose/SKILL.md-23-47 (1)
23-47: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPropagate
--store <id>through the proposal flow.new changeand everystatuscall here should carry the selected store; otherwise they fall back to the nearest localopenspec/root and can target the wrong repo.🤖 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 @.cursor/skills/openspec-propose/SKILL.md around lines 23 - 47, The proposal flow in SKILL.md is missing store scoping for repository-aware commands. Update the steps around the change creation and status lookup so the selected store id from openspec store list --json is propagated as --store on openspec new change and every openspec status invocation, and ensure any later follow-up commands in this flow keep using the same store identifier. Reference the change-creation and status-step instructions so the behavior stays consistent throughout the proposal flow..cursor/commands/opsx-apply.md-68-96 (1)
68-96: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winResolve schema assets through
{schema_root}.
design-bundle-template.mdandcode-generation_eval.yamlare hardcoded to repo-relative paths here, so the workflow breaks when the schema package is installed underopenspec/schemas/...or otherwise relocated.Suggested fix
- schemas/openspec-agile-workflow/templates/design-bundle-template.md + {schema_root}/templates/design-bundle-template.md ... - evals/code-generation_eval.yaml + {schema_root}/evals/code-generation_eval.yaml🤖 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 @.cursor/commands/opsx-apply.md around lines 68 - 96, The workflow in opsx-apply.md hardcodes repo-relative schema paths, which breaks when schemas are resolved through {schema_root}. Update the instructions that reference design-bundle-template.md and code-generation_eval.yaml so they use {schema_root}-based resolution consistently, and make sure the steps that build the design bundle and load evals use the same path-resolution approach. Keep the existing workflow structure intact, but replace the fixed locations with the schema-root-aware references used elsewhere in the command flow..cursor/commands/opsx-propose.md-19-109 (1)
19-109: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThread the selected store through every OpenSpec call.
The store-selection block says
new change,status, andinstructionsmust be store-scoped, but the workflow still runs them against the nearest localopenspec/root. In a registered store this will read/write the wrong change tree.Suggested fix
- openspec new change "<name>" + openspec new change "<name>" --store "$STORE_ID"Apply the same flag to every other
openspecinvocation in this file.🤖 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 @.cursor/commands/opsx-propose.md around lines 19 - 109, The store-scoping logic in opsx-propose is only applied to some OpenSpec commands, so the workflow can still hit the wrong change tree in a registered store. Update the command construction in the `/opsx:propose` flow to thread the selected store id through every `openspec` invocation in this file, including the remaining calls in the main workflow such as `new change`, `status`, `instructions`, and any follow-up status/final status commands. Keep the store flag behavior consistent with the existing store-selection block so all reads and writes use the same store context..cursor/commands/opsx-apply.md-13-13 (1)
13-13: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPoint the reference at the real command files.
oape-*.mddoes not match the actual filenames under.cursor/commands/(api-generate.md,api-implement.md,e2e-generate.md, etc.), so this reference points readers at files that do not exist.Suggested fix
- `.cursor/commands/oape-*.md` + `.cursor/commands/*.md`🤖 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 @.cursor/commands/opsx-apply.md at line 13, The reference is pointing to a non-existent command pattern, so update the opsx-apply guidance to name the actual files under .cursor/commands instead of oape-*.md. Keep the rest of the reference intact, but replace the wildcard with the real command filenames or a pattern that matches the existing command set so readers can navigate to the correct docs. Use the existing reference block in opsx-apply.md as the place to fix this..cursor/commands/opsx-archive.md-10-79 (1)
10-79: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThread the selected store through the archive flow.
You document store support here, but every
openspeccall still targets the nearest local root. In a registered store this will archive the wrong change tree, and the directopenspec/changes/...paths will be wrong too.Suggested fix
- openspec status --change "<name>" --json + openspec status --change "<name>" --store "$STORE_ID" --jsonApply the same
--storethreading tolistandarchive, and derive the archive path from the store-resolved roots instead of hardcodingopenspec/changes/....🤖 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 @.cursor/commands/opsx-archive.md around lines 10 - 79, The archive command is not threading the selected store through its `openspec` operations, so `list`, `status`, and the archive move can target the wrong root in a registered store. Update the `opsx:archive` flow to pass the chosen store into the `openspec` calls used for selection and status checks, and derive the archive/source paths from the store-resolved roots instead of hardcoding `openspec/changes/...`. Make the fix in the archive command implementation that handles `openspec list`, `openspec status`, and the final move logic..cursor/commands/opsx-sync.md-12-143 (1)
12-143: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep the sync flow store-aware end to end.
You document store support here, but
openspec list/statusstill run without--store, and the later filesystem edits still assume a repo-localopenspec/root. In a registered store this will sync the wrong change.Suggested fix
- openspec status --change "<name>" --json + openspec status --change "<name>" --store "$STORE_ID" --jsonUse the store-aware roots from the CLI output for both delta-spec discovery and main-spec edits.
🤖 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 @.cursor/commands/opsx-sync.md around lines 12 - 143, The sync flow is not consistently store-aware, so it can read and write the wrong specs in a registered store. Update the `/opsx:sync` logic to thread the resolved store id through every OpenSpec CLI call, especially `openspec list` and `openspec status`, and use the store-aware roots returned by those commands instead of assuming a repo-local `openspec/` path. Make sure the delta-spec discovery and the main-spec read/write steps both use the same store context, and keep the behavior consistent in the sync command’s flow and helpers that handle capability/spec paths.openspec/schemas/openspec-agile-workflow/schema.yaml-1164-1176 (1)
1164-1176: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
constitution.mdisrequiredfor theimplementationartifact but listed underapply.optional_context.
artifacts[implementation].inputs.required(line 1167) listsconstitution.mdas required. But theapplyblock'soptional_context(line 1434) lists it alongside genuinely optional items (repo-assessment,AGENTS.md). Sinceapplydrives the same Implementation Stage, this is a direct contradiction that could lead an agent to skip resolving/verifying constitution.md before starting task execution.🔧 Proposed fix
apply: workflow_stage: implementation artifact: implementation requires: - tasks - specs - plan + required_context: + - constitution.md (input — resolved via constitution_md.lookup_order; see artifacts.implementation.inputs.required) optional_context: - repo-assessment - - constitution.md (input — resolved via constitution_md.lookup_order) - AGENTS.mdAlso applies to: 1425-1435
🤖 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 `@openspec/schemas/openspec-agile-workflow/schema.yaml` around lines 1164 - 1176, The implementation artifact’s required inputs and the apply stage’s optional context are inconsistent because constitution.md is required in artifacts[implementation].inputs.required but treated as optional in apply.optional_context. Update the apply block to match the implementation artifact by moving constitution.md out of optional_context and into the required inputs/validation path, keeping the artifact stage rules aligned across the schema. Use the existing artifacts[implementation] and apply sections as the references to make the fix.openspec/schemas/openspec-agile-workflow/stage-gate/USER_FEEDBACK_PROMPT.md-64-64 (1)
64-64: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winStale "constitution" joint-gate content — constitution.md is no longer a generated/approved artifact.
Line 64 references "joint gates (
repo-assessment+constitution)" and lines 167-174 define a full "Co-generated artifacts (repo-assessment + constitution)" section describing a shared feedback loop, round summary directory, and joint template updates for constitution.md. But schema.yaml is explicit thatconstitution.mdis resolved as an input (lookup_order/when_missing), not a generated artifact subject touser_approval_feedback_gate— seeconstitution_md.description("NOT generated as an artifact") and the inline comment "constitution is no longer a generated artifact — it is an INPUT." The Repo Understanding workflow stage also only outputsrepo-assessment.md, with no companion "constitution" artifact. This looks like leftover content from a prior design where constitution.md was co-generated with repo-assessment.Recommend removing the joint-gate references to constitution.md here, treating
repo-assessmentas a single (non-joint) feedback target consistent with the rest of the schema.Also applies to: 167-174
🤖 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 `@openspec/schemas/openspec-agile-workflow/stage-gate/USER_FEEDBACK_PROMPT.md` at line 64, Remove the stale constitution joint-gate workflow from USER_FEEDBACK_PROMPT and make the feedback guidance consistent with schema.yaml: `constitution.md` is an input, not a generated/approved artifact. Update the joint-gate wording around the `repo-assessment` flow so it only targets `repo-assessment` (including the template/load/revise language), and delete the separate “Co-generated artifacts (repo-assessment + constitution)” section that references shared rounds, summaries, or template updates for `constitution.md`.openspec/schemas/openspec-agile-workflow/schema.yaml-231-237 (1)
231-237: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
evaluation_report.output_pathdisagrees with two other files.This defines the path as
openspec/changes/<change>/eval-results/<artifact-id>_evaluation_report.md. Bothstage-gate/SYSTEM_PROMPT.md(line 124:openspec/changes/<change>/<artifact-id>_evaluation_report.md) andstage-gate/artifact-eval-map.yaml(output_dir: openspec/changes/<change-name>/, line 79) place the report directly under the change root, without theeval-results/segment. An agent following this schema field would write to a different location than the other two docs expect.🔧 Proposed fix (align with SYSTEM_PROMPT.md / artifact-eval-map.yaml)
evaluation_report: - output_path: openspec/changes/<change>/eval-results/<artifact-id>_evaluation_report.md + output_path: openspec/changes/<change>/<artifact-id>_evaluation_report.md🤖 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 `@openspec/schemas/openspec-agile-workflow/schema.yaml` around lines 231 - 237, The evaluation report path in evaluation_report.output_path is inconsistent with the other workflow docs, which expect the file under the change root rather than inside eval-results. Update the schema field in openspec-agile-workflow/schema.yaml so it matches the path used by SYSTEM_PROMPT.md and artifact-eval-map.yaml, and keep the generated_for description unchanged. Use evaluation_report, artifact-eval-map.yaml, and SYSTEM_PROMPT.md as the key symbols to locate and align the contract.openspec/schemas/openspec-agile-workflow/stage-gate/artifact-eval-map.yaml-10-63 (1)
10-63: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
schema_templatevalues are missing the-templatesuffix — broken references to the actual template files.Every
schema_templateentry here (validation.md,spec.md,repo-assessment.md,plan.md,tasks.md,implementation.md) omits the-templatesuffix that the actual shipped files use (validation-template.md,spec-template.md,repo-assessment-template.md,plan-template.md,tasks-template.md,implementation-template.md— confirmed against schema.yaml's owntemplate:field for each artifact, e.g.template: validation-template.md).USER_FEEDBACK_PROMPT.mdStep 2 ("Current template |{schema_root}/templates/<schema_template>.md") andSYSTEM_PROMPT.md's rubric-only check both resolve paths from this map, so any agent following this map to locate/patch a template during the feedback loop will fail to find the file.🔧 Proposed fix
validation: - schema_template: validation.md + schema_template: validation-template.md specs: - schema_template: spec.md + schema_template: spec-template.md repo-assessment: - schema_template: repo-assessment.md + schema_template: repo-assessment-template.md plan: - schema_template: plan.md + schema_template: plan-template.md tasks: - schema_template: tasks.md + schema_template: tasks-template.md implementation: - schema_template: implementation.md + schema_template: implementation-template.md🤖 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 `@openspec/schemas/openspec-agile-workflow/stage-gate/artifact-eval-map.yaml` around lines 10 - 63, The artifact-eval map is pointing `schema_template` at non-existent template names, so update the entries in the `validation`, `specs`, `repo-assessment`, `plan`, `tasks`, and `implementation` mappings to use the real `*-template.md` filenames. Keep the existing stage, gate, and eval settings unchanged, and align the values with the corresponding template names used in schema definitions so `USER_FEEDBACK_PROMPT.md` and `SYSTEM_PROMPT.md` resolve the correct files.evals/stages/implementation/eval-spec.yaml-16-43 (1)
16-43: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMake the required-field contract match the example.
The example omits
input_refsand keepspass_thresholdnested underscoring, butrequired_fieldsexpects both. A case copied from this example will fail validation.♻️ Proposed fix
source_issue_ids: [ISSUE-001] + input_refs: [INPUT-001] assertions: must_use_ssa_not_create_or_update: true must_match_task_payloads: true scoring: method: weighted_checklist - pass_threshold: 80 + pass_threshold: 80🤖 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 `@evals/stages/implementation/eval-spec.yaml` around lines 16 - 43, The required-field contract in eval-spec.yaml does not match the example structure, because `input_refs` is listed as required and `pass_threshold` is required at the top level even though the example places threshold under `scoring`. Update the `required_fields` list and any related validation/schema logic so it reflects the actual example used by implementation evals, and make sure the `example` entry stays valid with `round`, `stage`, `source_issue_ids`, `assertions`, and nested `scoring.pass_threshold`.evals/stages/implementation/eval-spec.yaml-8-14 (1)
8-14: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAlign the consolidated artifact label.
consolidated_schema.artifactsayscode + phase FILE OPS, which does not match the top-level artifact value and reads like a placeholder. If tooling consumes this block, implementation cases can be classified under the wrong artifact.🐛 Proposed fix
- artifact: code + phase FILE OPS + artifact: code changes + implementation-report.md🤖 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 `@evals/stages/implementation/eval-spec.yaml` around lines 8 - 14, The consolidated_schema.artifact value is inconsistent with the top-level artifact label and still contains a placeholder-like value. Update the consolidated_schema block to use the same artifact classification as the rest of the spec, using the consolidated_schema field name and its artifact entry so implementation cases are categorized correctly.evals/stages/plan/eval-spec.yaml-16-43 (1)
16-43: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep the plan schema and example in sync.
This example is missing
input_refs, andpass_thresholdis still nested underscoringeven though it is listed as a required top-level field. Please make the required fields and the example use the same shape.♻️ Proposed fix
source_issue_ids: [ISSUE-001] + input_refs: [INPUT-001] assertions: must_include_verification_matrix: true must_cite_repo_assessment: true scoring: method: weighted_checklist - pass_threshold: 80 + pass_threshold: 80🤖 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 `@evals/stages/plan/eval-spec.yaml` around lines 16 - 43, The plan schema example is out of sync with the required top-level shape: the `example` in eval-spec.yaml is missing `input_refs`, and `pass_threshold` is incorrectly nested under `scoring` instead of matching `required_fields`. Update the `example` block to include `input_refs` and move `pass_threshold` to the top level so it matches the schema used by `required_fields` and `assertion_types`. Keep the example fields aligned with the plan contract referenced by `example`, `required_fields`, and `scoring`.evals/epic-bug-analysis/SYSTEM_PROMPT.md-35-35 (1)
35-35: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winNormalize the recurrence labels to the schema.
new_pattern/recurring_patternat Line 35 do not match thenew|recurringenum in the JSON schema, so the generatedissue-taxonomy.jsonwill fail validation.🔧 Proposed fix
- For each new bug/pattern, classify as **`new_pattern`** or **`recurring_pattern`** (matches a prior eval pattern). + For each new bug/pattern, classify as **`new`** or **`recurring`** (matches a prior eval pattern).Also applies to: 94-99
🤖 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 `@evals/epic-bug-analysis/SYSTEM_PROMPT.md` at line 35, The recurrence label values in SYSTEM_PROMPT.md are mismatched with the JSON schema enum, so update the taxonomy wording to use the schema’s allowed values instead of the longer labels. Adjust the instruction text around the bug classification guidance so it consistently references new and recurring, and make sure any examples or related lines in the same prompt use the same enum-compatible terms. Use the unique taxonomy guidance text in this prompt as the place to fix the label normalization.evals/eval-generation/SYSTEM_PROMPT.md-17-31 (1)
17-31: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAllow the round-1 seed step explicitly.
Lines 17-23 forbid any access to
schemas/openspec-agile-workflow/templates/, but Line 30 requires copying from that same path on round 1. The pipeline seeds refined templates once, so the prompt needs a clear exception instead of a contradiction.🛠️ Proposed fix
- **Do NOT** read or write `schemas/openspec-agile-workflow/templates/` during Eval Generation. - That path is upstream defaults for the forward `/opsx-*` workflow — not eval pipeline input. + Do not read or write `schemas/openspec-agile-workflow/templates/` except during the round-1 seed step. + On round 1, copy the templates once into `evals/refined-templates/`.🤖 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 `@evals/eval-generation/SYSTEM_PROMPT.md` around lines 17 - 31, Clarify the eval template path rules in SYSTEM_PROMPT.md: the current guidance in the eval workflow forbids any access to schemas/openspec-agile-workflow/templates/ but the round-1 seed step in the same section depends on copying from that source. Update the wording around the template path policy and the seed step so it explicitly allows a one-time round-1 copy from schemas/openspec-agile-workflow/templates/ into evals/refined-templates/, while keeping all other read/write operations confined to evals/refined-templates/; use the existing “Critical” section and the Read/Write/Seed table to make the exception unambiguous.evals/stages/tasks/eval-spec.yaml-33-43 (1)
33-43: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFix the example case before it is copied.
Line 36 contains corrupted
stagetext, and the example also omits required fields declared above (notablyinput_refs). As written, this sample is not a validtasks_eval.yamlcase.🛠️ Proposed fix
- stage: tasks11BLXLMQA0z9mbtgESBEBe_4aMi02hSahBitFt4Uvd7yftgV5jaYd5JRvy0VZb6NGO3SFLIV3KJVcaHZhA + stage: tasks🤖 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 `@evals/stages/tasks/eval-spec.yaml` around lines 33 - 43, The example case in the eval spec is invalid because the stage value is corrupted and the sample is missing required fields such as input_refs. Update the example under the tasks case to use a valid stage identifier and include all required fields declared by the schema, checking the example block around the eval-r001-tasks-001 entry so it matches a real tasks_eval.yaml case.
🟡 Minor comments (7)
openspec/schemas/openspec-agile-workflow/evals/stages/tasks/eval-spec.yaml-34-40 (1)
34-40: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winFix the example
stagevalue — theexampleblock hasstage: tasks11BLXLMQA0z9mbtgESBEBe_4aMi02hSahBitFt4Uvd7yftgV5jaYd5JRvy0VZb6NGO3SFLIV3KJVcaHZhA, but it should bestage: tasksto match the rest of the spec.🤖 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 `@openspec/schemas/openspec-agile-workflow/evals/stages/tasks/eval-spec.yaml` around lines 34 - 40, The example in the eval spec uses an incorrect stage identifier, so update the `stage` value in the `eval-spec.yaml` example to `tasks` to match the rest of the schema. Make this change in the `eval-r001-tasks-001` example block alongside the existing `id`, `round`, and `source_issue_ids` fields, keeping the assertions unchanged.openspec/schemas/openspec-agile-workflow/schema.yaml-302-312 (1)
302-312: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
do_not_modifyincludestasks.md, contradicting the same gate's own approval flow.
code_generation_eval_gate.do_not_modify(line 311) forbids modifyingopenspec/changes/<change>/tasks.md. Butoape_routing.task_execution.task_approval_gate(lines 619-625, same file) explicitly requires "mark the task- [x]in tasks.md" on approval, andstage-gate/CODE_GENERATION_EVAL_PROMPT.mdStep 9 says the same. As written,do_not_modifyappears to be scoped to the refinement loop only, but it's listed at the gate level without qualification — worth clarifying so an implementer doesn't skip the task-completion mark.🔧 Proposed clarification
do_not_modify: - evals/code-generation_eval.yaml - - openspec/changes/<change>/tasks.md + - openspec/changes/<change>/tasks.md (except marking the current task `- [x]` on approval, per task_execution.task_approval_gate) - approved_upstream_artifacts🤖 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 `@openspec/schemas/openspec-agile-workflow/schema.yaml` around lines 302 - 312, Clarify the `code_generation_eval_gate.do_not_modify` entry so it does not conflict with the approval flow that requires updating `openspec/changes/<change>/tasks.md`. Update the gate definition in `schema.yaml` to state that the `tasks.md` restriction applies only during the refinement/eval loop, while `oape_routing.task_execution.task_approval_gate` still allows marking the task complete on approval. Keep the wording aligned with `stage-gate/CODE_GENERATION_EVAL_PROMPT.md` Step 9 and the surrounding `code_generation_eval_gate` / `task_approval_gate` symbols so implementers know the file is modified only at the approval step.openspec/schemas/openspec-agile-workflow/templates/implementation-task-report-template.md-32-34 (1)
32-34: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winLabel the placeholder fence.
This block is a markdown prompt example, so the unlabeled fence keeps markdownlint flagging MD040 and makes the placeholder less copyable.
Fix
-``` +```text🤖 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 `@openspec/schemas/openspec-agile-workflow/templates/implementation-task-report-template.md` around lines 32 - 34, The markdown prompt example in the implementation-task-report template uses an unlabeled code fence, which triggers markdownlint MD040 and makes the placeholder harder to copy. Update the placeholder fence in the template to use a language label, keeping the example block in the same section of implementation-task-report-template.md so the markdown example is lint-friendly and copyable.Source: Linters/SAST tools
openspec/schemas/openspec-agile-workflow/templates/tasks-modes/orchestration-template.md-11-19 (1)
11-19: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse
###subsections here.Jumping straight from
##to####skips a heading level and makes the generated section harder to scan.Fix
-#### Retry Boundaries +### Retry Boundaries -#### Merge Conflict Hotspots +### Merge Conflict Hotspots -#### Open Questions Requiring SME Before Execution +### Open Questions Requiring SME Before Execution🤖 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 `@openspec/schemas/openspec-agile-workflow/templates/tasks-modes/orchestration-template.md` around lines 11 - 19, The template section uses `####` headings directly under a `##` parent, which skips a level and hurts readability; update the orchestration template to use `###` subsections for the retry boundaries, merge conflict hotspots, and open questions blocks, keeping the existing placeholders like `RETRY_GUIDANCE`, `HOTSPOT_FILES_AND_MITIGATION`, and `OPEN_QUESTION` in place.Source: Linters/SAST tools
openspec/schemas/openspec-agile-workflow/templates/plan-template.md-217-256 (1)
217-256: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winLabel the user-message fence as YAML.
The block is YAML, so the unlabeled fence is a lint hit and makes the example harder to copy/paste.
Fix
-``` +```yaml🤖 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 `@openspec/schemas/openspec-agile-workflow/templates/plan-template.md` around lines 217 - 256, The example block in the plan template uses an unlabeled fenced code block even though it contains YAML, so update the fenced section in the template to be explicitly labeled as YAML. Locate the example under the `instructions`/`metadata` template content in `plan-template.md` and change the fence syntax consistently so the sample is valid and easier to copy, without altering the YAML content itself.Source: Linters/SAST tools
openspec/schemas/openspec-agile-workflow/templates/implementation-task-report-template.md-69-70 (1)
69-70: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFix the task-report relative links.
These links are emitted from
implementation/task-reports/<task-id>.md, so the current paths resolve into a nested non-existent directory. Use paths that step out oftask-reports/first.Fix
- - Design bundle: `implementation/design-bundle.md` (snapshot at approval time) - - Phase log entry: `implementation-phase-log.md` + - Design bundle: `../design-bundle.md` (snapshot at approval time) + - Phase log entry: `../../implementation-phase-log.md`🤖 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 `@openspec/schemas/openspec-agile-workflow/templates/implementation-task-report-template.md` around lines 69 - 70, The relative links in the task report template resolve incorrectly from implementation/task-reports/<task-id>.md because they do not step out of task-reports/ first. Update the links in the implementation-task-report template so the Design bundle and Phase log entry paths use the correct parent-relative locations, and verify the generated markdown still points to implementation/design-bundle.md and implementation-phase-log.md from the task-reports context.evals/eval-generation/stage-samples/repo-assessment/README.md-1-2 (1)
1-2: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMissing blank line between headings (MD022).
Line 2 (
# Paste example input/output...) is parsed as a second H1 heading immediately following line 1's heading, with no blank line separating them.📝 Proposed fix
# Optional I/O samples — repo-assessment stage -# Paste example input/output pairs from past features to guide eval authoring. + +<!-- Paste example input/output pairs from past features to guide eval authoring. -->🤖 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 `@evals/eval-generation/stage-samples/repo-assessment/README.md` around lines 1 - 2, The README has two consecutive H1 headings without the required blank line, triggering MD022. Update the intro in README.md so the heading “Optional I/O samples — repo-assessment stage” is followed by a blank line before “Paste example input/output pairs from past features to guide eval authoring,” keeping the existing heading text and spacing consistent.Source: Linters/SAST tools
🧹 Nitpick comments (7)
.cursor/e2e-test-generator/docs/e2e-patterns.md (3)
14-14: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHyphenate compound modifiers.
LanguageTool flags "controller-runtime based" and "library-go based" as needing a hyphen before "based" (compound adjective).
✏️ Proposed fix
-Typical file layout for controller-runtime based operators: +Typical file layout for controller-runtime-based operators:-Typical file layout for library-go based operators: +Typical file layout for library-go-based operators:Also applies to: 65-65
🤖 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 @.cursor/e2e-test-generator/docs/e2e-patterns.md at line 14, The phrasing in e2e-patterns.md uses compound modifiers without hyphens, which triggers LanguageTool. Update the affected prose to hyphenate the adjective forms around the “based” descriptions, such as in the “Typical file layout for controller-runtime based operators” text and the related “library-go based” mention, so the wording reads as compound adjectives before “based.”Source: Linters/SAST tools
81-81: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate heading
### Conventions.Same heading text is reused for the Library-Go section (already used at line 25 for Controller-Runtime), which markdownlint flags (MD024). Consider differentiating, e.g.
### Bash Conventions.🤖 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 @.cursor/e2e-test-generator/docs/e2e-patterns.md at line 81, The markdown section heading “### Conventions” is duplicated and triggers MD024. Update the heading used in the Library-Go section to a unique title, such as “### Bash Conventions,” and make sure the surrounding section text still matches the renamed heading; locate it via the markdown heading in e2e-patterns.md rather than relying on line numbers.Source: Linters/SAST tools
16-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd language identifiers to fenced code blocks.
markdownlint flags these directory-tree/file-layout blocks as missing a language spec (MD040). Use
textfor plain tree diagrams.📝 Proposed fix
-``` +```text test/e2e/ e2e_suite_test.go # Suite setup: kubeconfig, scheme registration, clientsAlso applies to: 67-73, 76-79
🤖 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 @.cursor/e2e-test-generator/docs/e2e-patterns.md around lines 16 - 23, The fenced tree-diagram blocks in the e2e-patterns docs are missing a language identifier and trigger markdownlint MD040. Update each affected fenced block to use the text language specifier for plain directory layouts, keeping the content the same; target the repeated diagram blocks in the e2e-patterns markdown section so all similar fences are fixed consistently.Source: Linters/SAST tools
openspec/schemas/openspec-agile-workflow/evals/stages/code-generation/eval-spec.yaml (1)
25-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winOnly 2 of 12 assertion types have documented descriptions.
assertion_types(Lines 43-56) lists 12 assertion kinds, butassertion_descriptions(Lines 57-66) only documentsmust_execute_verificationandmust_co_generate_tests. Authors of new eval cases have no guidance for the other 10 assertion types (must_use_pattern,must_not_use,must_pass_make_targets, etc.).Also applies to: 57-66
🤖 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 `@openspec/schemas/openspec-agile-workflow/evals/stages/code-generation/eval-spec.yaml` around lines 25 - 34, The eval spec is missing descriptions for most assertion types, leaving authors without guidance for the full `assertion_types` set. Update the `assertion_descriptions` section in the schema to document every assertion kind listed in `assertion_types`, including `must_use_pattern`, `must_not_use`, `must_pass_make_targets`, and the remaining undeclared types, so the spec stays aligned and complete.openspec/schemas/openspec-agile-workflow/templates/tasks-template.md (1)
130-196: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMulti-Pass Mode section duplicates the dedicated
tasks-modes/*.mdfiles.Pass 1/2/3 instructions here (skeleton output structure,
tasks_index.jsonschema, payloads/orchestration scope) largely re-statetemplates/tasks-modes/skeleton-template.md,payloads-template.md, andorchestration-template.md, which schema.yaml'sgeneration.passes[*].mode_templatealready designates as the per-pass sources (schema.yaml's own instruction: "Follow tasks.md (base template — shared rules) plus mode-specific instructions intemplates/tasks-modes/"). Maintaining the same content in two places risks drift as the pass templates evolve independently.Consider trimming this section to a short pointer ("see
tasks-modes/<pass>-template.mdfor pass-specific output schemas") rather than duplicating the full instructions.🤖 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 `@openspec/schemas/openspec-agile-workflow/templates/tasks-template.md` around lines 130 - 196, The Multi-Pass Mode block in tasks-template.md duplicates the pass-specific guidance already owned by the dedicated tasks-modes templates and schema.yaml. Trim the Pass 1/2/3 descriptions to a brief pointer that directs readers to skeleton-template.md, payloads-template.md, and orchestration-template.md for the exact output rules, while keeping only the high-level pass names and scope in tasks-template.md.openspec/schemas/openspec-agile-workflow/schema.yaml (1)
1436-1473: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
apply.instructionlargely re-statesartifacts[implementation].instruction.The task-loop steps here (compose bundle → resolve command → verify → code eval gate → approve → advance) closely duplicate the fuller version at lines 1233-1266. Two independent copies of the same orchestration logic are prone to drifting out of sync as the workflow evolves.
Consider having
apply.instructionsimply referenceartifacts.implementation.instructionas the canonical task-loop description, keeping only stage-entry specifics (fork/working-folder resolution) here.🤖 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 `@openspec/schemas/openspec-agile-workflow/schema.yaml` around lines 1436 - 1473, `apply.instruction` duplicates the implementation task-loop already defined in `artifacts[implementation].instruction`, which risks drift. Update the `apply.instruction` block to reference the canonical `artifacts.implementation.instruction` for the full task-loop behavior, and keep only the stage-entry specifics here such as fork/working-folder resolution and the initial `inputs/jira.yaml` handling.openspec/schemas/openspec-agile-workflow/templates/repo-assessment-template.md (1)
77-92: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the library-go branch from this repo’s guidance.
This repository is controller-runtime only, so the dual-framework wording sends the planner down a path the repo doesn’t use.
Based on learnings, this repository uses controller-runtime only; do not use library-go.
Fix
- - Dual/multiple controller framework architectures (library-go vs controller-runtime) + - Controller-runtime architecture (this repo does not use library-go)🤖 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 `@openspec/schemas/openspec-agile-workflow/templates/repo-assessment-template.md` around lines 77 - 92, The Kubernetes/OpenShift operator guidance still mentions a library-go vs controller-runtime split, but this repo is controller-runtime only. Update the repo-assessment template section for Kubernetes/OpenShift Operator Repos to remove the library-go branch and rewrite the framework guidance around controller-runtime-only reconciliation, while keeping the rest of the operator checklist intact. Use the existing operator checklist block in repo-assessment-template.md to locate and edit the framework-architecture bullet.Source: Learnings
| command: /eval-loop | ||
|
|
||
| # Eval workflow template source — NOT schemas/ (upstream defaults for /opsx-* only) | ||
| refined_templates_dir: evals/refined-templates/ | ||
| refined_agents_md: evals/refined-templates/agents.md | ||
|
|
||
| # Upstream defaults (read-only during eval pipeline; seed refined-templates on round 1 if empty) | ||
| schema_templates_dir: | ||
| installed: openspec/schemas/openspec-agile-workflow/templates | ||
| distribution: schemas/openspec-agile-workflow/templates | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
YAML syntax error: orphaned mapping keys break the entire file.
Lines 11-12 (refined_templates_dir, refined_agents_md) are indented under a comment with no parent mapping key, immediately after the scalar command: /eval-loop. This is invalid YAML — YAMLlint confirms syntax error: expected <block end>, but found '<block mapping start>' at line 11. Since this config is consumed by /eval-loop orchestration, a parse failure breaks the whole pipeline.
🐛 Proposed fix
command: /eval-loop
# Eval workflow template source — NOT schemas/ (upstream defaults for /opsx-* only)
- refined_templates_dir: evals/refined-templates/
- refined_agents_md: evals/refined-templates/agents.md
+template_source:
+ refined_templates_dir: evals/refined-templates/
+ refined_agents_md: evals/refined-templates/agents.md📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| command: /eval-loop | |
| # Eval workflow template source — NOT schemas/ (upstream defaults for /opsx-* only) | |
| refined_templates_dir: evals/refined-templates/ | |
| refined_agents_md: evals/refined-templates/agents.md | |
| # Upstream defaults (read-only during eval pipeline; seed refined-templates on round 1 if empty) | |
| schema_templates_dir: | |
| installed: openspec/schemas/openspec-agile-workflow/templates | |
| distribution: schemas/openspec-agile-workflow/templates | |
| command: /eval-loop | |
| # Eval workflow template source — NOT schemas/ (upstream defaults for /opsx-* only) | |
| template_source: | |
| refined_templates_dir: evals/refined-templates/ | |
| refined_agents_md: evals/refined-templates/agents.md | |
| # Upstream defaults (read-only during eval pipeline; seed refined-templates on round 1 if empty) | |
| schema_templates_dir: | |
| installed: openspec/schemas/openspec-agile-workflow/templates | |
| distribution: schemas/openspec-agile-workflow/templates |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 11-11: syntax error: expected , but found ''
(syntax)
🤖 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 `@evals/pipeline.yaml` around lines 8 - 18, The evals/pipeline.yaml content has
orphaned mapping keys after command: /eval-loop, so the YAML parser will fail.
Fix the structure by attaching refined_templates_dir and refined_agents_md under
an existing parent mapping (or grouping them into a proper top-level mapping) in
the eval_loop/pipeline configuration, keeping schema_templates_dir aligned as a
valid nested mapping; use the command, refined_templates_dir, refined_agents_md,
and schema_templates_dir keys as the anchors when restructuring.
Source: Linters/SAST tools
|
@bharath-b-rh: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #164 +/- ##
==========================================
+ Coverage 46.19% 46.26% +0.07%
==========================================
Files 31 31
Lines 5451 5464 +13
==========================================
+ Hits 2518 2528 +10
- Misses 2626 2628 +2
- Partials 307 308 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Summary by CodeRabbit