Repository navigation
feat(validation): internal validation log page + live per-criterion progress checklist - #307
Closed
KCSAbeywickrama wants to merge 8 commits into
Closed
KCSAbeywickrama wants to merge 8 commits into
KCSAbeywickrama wants to merge 8 commits into
Conversation
…ernal log page regardless of the validation state Co-Authored-By: Claude <noreply@anthropic.com>
…nto validation-phase
- Introduced CriterionReport type to represent acceptance criterion status. - Added RunnerCriteriaReport handler to process incoming reports. - Created RunnerCriteriaReportRequestObject and response types for handling requests and responses. - Implemented database migration for criterion_statuses table to store validation criteria statuses. - Updated migration steps to include criterion_statuses creation. - Adjusted step order tests to reflect the new migration step. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
… validation consistency Co-Authored-By: Claude <noreply@anthropic.com>
…nto validation-phase
📝 WalkthroughWalkthroughAdds durable per-criterion validation status storage and runner callback APIs. The remote worker reports Playwright criterion transitions through a Unix-socket listener, live progress events, and an optional platform callback. The API persists and serves criterion statuses. The console merges durable statuses with live events to render checklist state and progress, while completed deployment validation links now route to the internal validation log. Sequence Diagram(s)sequenceDiagram
participant Playwright
participant RemoteWorker
participant AEPAPI
participant DurableStore
participant Console
Playwright->>RemoteWorker: Emit criterion status
RemoteWorker->>AEPAPI: POST validation criterion report
AEPAPI->>DurableStore: Upsert latest criterion status
Console->>AEPAPI: GET validation criteria
AEPAPI-->>Console: Return durable status rows
Console->>Console: Overlay live events and render checklist
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
apps/console/src/features/tasks/lib/validationCriterionStatus.test.ts (1)
41-68: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the
skippedstatus.
skippedis a supported criterion state but is not asserted in the merge contract. Add a durable or live-frame case to prevent it from being filtered as unknown.🤖 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 `@apps/console/src/features/tasks/lib/validationCriterionStatus.test.ts` around lines 41 - 68, Extend the mergeCriterionStatus tests with a durable-row or live-frame case using status “skipped,” and assert that the resulting criterion map preserves that status. Keep the test focused on confirming skipped is accepted rather than filtered as unknown.
🤖 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 `@apps/console/src/features/tasks/components/ValidationPage.tsx`:
- Line 137: Update the validation-state branch in ValidationPage to check the
raw timeline’s log.lines length instead of the derived logLines length, so
criterion-only activity is recognized as validation having started. Preserve the
existing behavior for genuinely empty timelines and the surrounding status
handling.
In `@packages/contracts/api/internal/v1/openapi.yaml`:
- Around line 246-265: Close the criterion status contract by adding the enum
values validating, passed, failed, and skipped to
ValidationCriterionReport.status in
packages/contracts/api/internal/v1/openapi.yaml; validate the same set in
criterion_ingest.go and return the established typed validation error for
invalid or blank values; update internal.go to map that validation error to
errBadRequest.
In `@packages/contracts/api/v1/openapi.yaml`:
- Around line 4393-4395: Define the criterion-status enum with validating,
passed, failed, and skipped in packages/contracts/api/v1/openapi.yaml at lines
4393-4395, and add the same enum to
packages/contracts/api/internal/v1/openapi.yaml; then regenerate
services/aep-api/internal/igen/igen_gen.go at lines 88-98 so the generated
artifact enforces these values.
In `@runners/remote-worker/src/lib/progress/validation-criterion-listener.ts`:
- Around line 60-90: Update the HTTP server setup in the criterion listener to
enforce a short request lifetime: configure a request timeout and destroy any
request that remains unfinished after the timeout, while preserving normal
completion and error handling. Ensure criterionListener.close() can finish even
when a client stops after sending only request headers.
In `@services/aep-api/internal/delivery/validation_criterion_status.go`:
- Around line 35-43: Include OrgID in the persisted conflict identity: update
validation_criterion_status.go so OrgID participates in the primary/unique key,
update repository_validation_criterion_status.go so the upsert conflict columns
include org_id, and extend repository_validation_criterion_status_dbtest_test.go
with a same repo/issue/criterion key across different orgs to verify both rows
persist independently.
In `@services/aep-api/internal/delivery/validation/criterion_ingest.go`:
- Around line 60-67: Require validation-task eligibility before persisting
criterion reports: update LookupExecutionTask in
services/aep-api/internal/app/validation_adapters.go:77-85 to verify and return
whether the execution is associated with a validation task, then update the
ingest flow in
services/aep-api/internal/delivery/validation/criterion_ingest.go:60-67 to
reject lookup results that are not eligible before calling UpsertCriterion.
---
Nitpick comments:
In `@apps/console/src/features/tasks/lib/validationCriterionStatus.test.ts`:
- Around line 41-68: Extend the mergeCriterionStatus tests with a durable-row or
live-frame case using status “skipped,” and assert that the resulting criterion
map preserves that status. Keep the test focused on confirming skipped is
accepted rather than filtered as unknown.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 47277e8a-4cb0-4ae6-806f-caa59b8772da
⛔ Files ignored due to path filters (2)
services/aep-api/internal/gen/models_gen.gois excluded by!**/gen/**services/aep-api/internal/gen/server_gen.gois excluded by!**/gen/**
📒 Files selected for processing (48)
apps/console/src/features/projects/components/DeploymentsPage.test.tsxapps/console/src/features/projects/components/DeploymentsPage.tsxapps/console/src/features/tasks/api/keys.tsapps/console/src/features/tasks/api/queries.tsapps/console/src/features/tasks/components/ValidationPage.test.tsxapps/console/src/features/tasks/components/ValidationPage.tsxapps/console/src/features/tasks/lib/validationCriterionStatus.test.tsapps/console/src/features/tasks/lib/validationCriterionStatus.tsapps/console/src/mocks/fixtures/project.tsapps/console/src/mocks/handlers/project.tspackages/contracts/api/internal/v1/openapi.yamlpackages/contracts/api/v1/openapi.yamlpackages/ui/validation-view/src/ValidationView.tsxpackages/ui/validation-view/src/index.tsrunners/remote-worker/package.jsonrunners/remote-worker/plugin/skills/aep-validation/references/criterion-reporter.cjsrunners/remote-worker/plugin/skills/aep-validation/references/playwright.config.template.tsrunners/remote-worker/src/lib/progress/criterion-reporter.test.tsrunners/remote-worker/src/lib/progress/schema.tsrunners/remote-worker/src/lib/progress/validation-criterion-client.tsrunners/remote-worker/src/lib/progress/validation-criterion-listener.test.tsrunners/remote-worker/src/lib/progress/validation-criterion-listener.tsrunners/remote-worker/src/lib/runner.tsrunners/remote-worker/src/oneshot.tsservices/aep-api/internal/app/app.goservices/aep-api/internal/app/validation_adapters.goservices/aep-api/internal/delivery/README.mdservices/aep-api/internal/delivery/repository_validation_criterion_status.goservices/aep-api/internal/delivery/repository_validation_criterion_status_dbtest_test.goservices/aep-api/internal/delivery/task/handlers.goservices/aep-api/internal/delivery/task/ports.goservices/aep-api/internal/delivery/task/reads.goservices/aep-api/internal/delivery/task/reads_test.goservices/aep-api/internal/delivery/task/task_component_test.goservices/aep-api/internal/delivery/task/task_test.goservices/aep-api/internal/delivery/validation/criterion_ingest.goservices/aep-api/internal/delivery/validation/criterion_ingest_test.goservices/aep-api/internal/delivery/validation/ports.goservices/aep-api/internal/delivery/validation_criterion_status.goservices/aep-api/internal/edge/errors.goservices/aep-api/internal/edge/internal.goservices/aep-api/internal/edge/internal_surface_test.goservices/aep-api/internal/edge/internal_test.goservices/aep-api/internal/edge/method_origin_test.goservices/aep-api/internal/igen/igen_gen.goservices/aep-api/internal/migrate/run_all.goservices/aep-api/internal/migrate/step_order_test.goservices/aep-api/internal/migrate/validation_criterion_statuses.go
…ed structures - Added ValidationCriterionReportStatus enum with values: Failed, Passed, Skipped, and Validating. - Updated ValidationCriterionReport to use ValidationCriterionReportStatus for the Status field. - Enhanced Valid method to validate known members of the ValidationCriterionReportStatus enum. - Modified validation_criterion_statuses table schema to include org_id as part of the primary key, ensuring org-fenced data integrity. Co-Authored-By: Claude <noreply@anthropic.com>
kaje94
approved these changes
Jul 23, 2026
Contributor
Author
|
After an offline discussion with @xlight05, decided to design a generalized event system for all coding agents instead of a validation-specific event system. |
KCSAbeywickrama
marked this pull request as draft
July 23, 2026 06:08
KCSAbeywickrama
added a commit
to KCSAbeywickrama/labs-agentic-engineer
that referenced
this pull request
Aug 31, 2026
… does it A validation cycle is dispatched with a 7200s deadline, and until it merges its pull request and the platform reads report.json at that merge commit, nothing on the page can say anything about any individual criterion. wso2#669 put a row per criterion on screen; every one of them read "Pending" for up to two hours. Worse on a repeat attempt, which carries the previous attempt's report and so froze the failed rows on last attempt's verdict for the whole run that was re-working them. Rows now carry what the run is doing to them — planned, exploring, authoring, running, healing — settling on report.json's own pass/fail. A tenth progress kind carries it: progress_item, with an itemId to fold on and a status, which is one new field on the wire. Every other kind reports something that happened once at a moment, and none of them carries an identity a consumer can repaint a row by. Inferred from the agent's own calls, never declared by it. A PreToolUse hook reads statuses off work the run has to do anyway: the test plan it commits, the spec file it writes, the command it runs. An instruction to report progress is one an agent can skip with nobody noticing for a whole run. PR wso2#307 built this and was closed for being validation-specific, so the kind is generic — cycleKind already says whether the ids are criteria or issues, which is genericity without a field nobody uses yet. wso2#307's Unix socket is gone too, and needed no workflow change to remove: authoring.md already requires every spec to pass alone and twice consecutively, and healing.md re-runs the same shape, so the criterion id is already on commands the hook sees. Making the RUN step itself per-spec was tried and reverted. The suite runs serially against one shared environment, so a spec that passes alone can fail in sequence — healing.md's data-collision class — and isolated runs cannot find it. Replacing the integrated run with them moved that signal past the heal budget, where it lands in report.json as a genuine failure and mints a repair issue against code that is not broken. The config now picks its reporters from whether the command names a spec, so the per-spec runs in steps 6 and 8 write no results.json and cannot leave a one-spec file where the report expects the suite — which only call ordering hid before. Inferred from the command rather than a flag, because naming a spec is how you run one spec and there is nothing to remember; the cost is that a sharded authoritative run writes nothing, so the skill forbids sharding that one call. generate-report.mjs is untouched. The config is also re-copied every run now rather than skipped when present, or a repo scaffolded earlier keeps the old behaviour silently. Exploration is the longest unobservable stretch and playwright-cli calls name URLs, never criteria — so the skill writes a spec's mandatory `// spec:` header before exploring rather than after. Not a new artifact, only a different moment for a line that was already required. Newly created specs only: blanking an existing one reads as a pre-existing spec modified with no heal-log entry, which the report generator fails the run for. healing fires only for a criterion that passed and then broke, which is what healing.md scopes a heal to. authoring.md requires a spec to pass twice consecutively, so failing on the way to a first pass is the normal path and reporting it as healing would make a healthy run read as a struggling one. Live statuses outrank the report while a cycle is open, and the report wins again the moment it settles — which also fixes the unreported re-dispatch, where the page fetched no report and rows rendered no chip at all. The one run-wide line above the rows is derived FROM the rows, so it cannot contradict them, and speaks only in the two windows where they say nothing: before any criterion is picked up and after they have all settled. No durable store — the stream replays the cycle log on reconnect and the archive after the pod is reaped. Renderers format progress_item to no text, as activity already does. A criterion moving through five statuses would otherwise be five log rows narrating what the row above already shows. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
KCSAbeywickrama
added a commit
to KCSAbeywickrama/labs-agentic-engineer
that referenced
this pull request
Aug 31, 2026
… does it A validation cycle is dispatched with a 7200s deadline, and until it merges its pull request and the platform reads report.json at that merge commit, nothing on the page can say anything about any individual criterion. wso2#669 put a row per criterion on screen; every one of them read "Pending" for up to two hours. Worse on a repeat attempt, which carries the previous attempt's report and so froze the failed rows on last attempt's verdict for the whole run that was re-working them. Rows now carry what the run is doing to them — planned, exploring, authoring, running, healing — settling on report.json's own pass/fail. A tenth progress kind carries it: progress_item, with an itemId to fold on and a status, which is one new field on the wire. Every other kind reports something that happened once at a moment, and none of them carries an identity a consumer can repaint a row by. Inferred from the agent's own calls, never declared by it. A PreToolUse hook reads statuses off work the run has to do anyway: the test plan it commits, the spec file it writes, the command it runs. An instruction to report progress is one an agent can skip with nobody noticing for a whole run. PR wso2#307 built this and was closed for being validation-specific, so the kind is generic — cycleKind already says whether the ids are criteria or issues, which is genericity without a field nobody uses yet. wso2#307's Unix socket is gone too, and needed no workflow change to remove: authoring.md already requires every spec to pass alone and twice consecutively, and healing.md re-runs the same shape, so the criterion id is already on commands the hook sees. Making the RUN step itself per-spec was tried and reverted. The suite runs serially against one shared environment, so a spec that passes alone can fail in sequence — healing.md's data-collision class — and isolated runs cannot find it. Replacing the integrated run with them moved that signal past the heal budget, where it lands in report.json as a genuine failure and mints a repair issue against code that is not broken. The config now picks its reporters from whether the command names a spec, so the per-spec runs in steps 6 and 8 write no results.json and cannot leave a one-spec file where the report expects the suite — which only call ordering hid before. Inferred from the command rather than a flag, because naming a spec is how you run one spec and there is nothing to remember; the cost is that a sharded authoritative run writes nothing, so the skill forbids sharding that one call. generate-report.mjs is untouched. The config is also re-copied every run now rather than skipped when present, or a repo scaffolded earlier keeps the old behaviour silently. Exploration is the longest unobservable stretch and playwright-cli calls name URLs, never criteria — so the skill writes a spec's mandatory `// spec:` header before exploring rather than after. Not a new artifact, only a different moment for a line that was already required. Newly created specs only: blanking an existing one reads as a pre-existing spec modified with no heal-log entry, which the report generator fails the run for. healing fires only for a criterion that passed and then broke, which is what healing.md scopes a heal to. authoring.md requires a spec to pass twice consecutively, so failing on the way to a first pass is the normal path and reporting it as healing would make a healthy run read as a struggling one. Live statuses outrank the report while a cycle is open, and the report wins again the moment it settles — which also fixes the unreported re-dispatch, where the page fetched no report and rows rendered no chip at all. The one run-wide line above the rows is derived FROM the rows, so it cannot contradict them, and speaks only in the two windows where they say nothing: before any criterion is picked up and after they have all settled. No durable store — the stream replays the cycle log on reconnect and the archive after the pod is reaped. Renderers format progress_item to no text, as activity already does. A criterion moving through five statuses would otherwise be five log rows narrating what the row above already shows. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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
Two user-facing additions to the validation phase, plus a naming-consistency sweep that
qualifies the criteria/validation surface across the stack.
1. Internal validation log page
The Deployments board's validation chip now always opens an internal validation log page
(
/projects/$projectName/deployments/validation/$issueNumber) in every state — running, failed,and completed. It shows live progress while the run is in flight and the historical runner log once
it has finished. Validation dispatches through the shared coding executor as a
kind=codingexecution, so its pod log is captured durably exactly like an implementation task's — no
validation-specific backend log work was needed. The page also carries jump-to-issue and
jump-to-PR buttons. The external
validationUrlremains the fallback only when the statuspayload carries no issue number.
2. Real-time per-criterion validation checklist
The validation page renders the acceptance-criteria checklist (authored in the design phase, read
from
specs/validation/validation-criteria.json) and overlays each criterion's live status(
validating/passed/failed/skipped) as the runner works, with a current-row highlightand an "N/M done" tally.
Pipeline:
criterion-reporter.cjs) emits per-test begin/end over a Unix socket to aharness listener in the runner. (The SDK captures Playwright's own stdout as a tool_result and
drops it, so a side-channel is required for the live path.)
kind:"criterion"frames)consumed by the existing SSE task-log, and (b) a POST to the platform that persists
per-criterion status in a durable store.
page load, or a run that has already finished, still shows the per-criterion results.
Contract changes
GET /projects/{projectName}/tasks/{issueNumber}/validation-criteria→ValidationCriterionStatus[](one-shot seed for the checklist).POST /executions/{executionId}/validation-criteria(runner → platform report),sitting in the
runner-validation-*subfamily.DeployStage.validationIssue(int64) andTaskView/TaskDetail.prUrladded.Durable store
validation_criterion_statusestable, keyed by(repo, issue_number, criterion_id), last-write-winsupsert, org-fenced reads;
execution_idkept for provenance only. Registered as thevalidation_criterion_statusesmigration step (appended last — no FK, ordering not load-bearing).Naming consistency
Qualified the validation surface so bare
criteria/criterionnames read unambiguously wherevertheir location doesn't already scope them: the endpoints, the entity/table/repository, the
ingest ports + adapters,
InternalDeps.ValidationCriteria,Reads.ValidationCriteria, and therunner files (
validation-criterion-client,validation-criterion-listener). Symbols alreadyscoped by a validation package/dir, or whose peer fields are bare, were left bare to match their
neighbors.
Result
Testing (proof of execution)
All run locally on this branch:
pnpm --filter @aep/console test): 37 files, 246 tests pass — includes newValidationPage,validationCriterionStatusmerge/split, andDeploymentsPagechip-routing cases.remote-workerpnpm test): 68 tests pass — includes the criterion reporter andthe Unix-socket criterion listener.
go build ./...+go vetclean;delivery,delivery/validation,delivery/task,edge,migratepackages all pass (migrationstep_order,method_originledger, and
internal_surfacegoldens updated).Docs
services/aep-api/internal/delivery/README.mdupdated (persistence line + ports table now carry thevalidation-qualified names, plus the criteria-checklist invariant).
Notes
aep-validation-runner:devimage bakes the runner TS; it needs a rebuild(
make build-validation-runner FORCE=1) before deploy since this branch renamed baked runner files.🤖 Generated with Claude Code