feat(ai): outputFormat strategy + riteway ai init [8/8]#423
Open
ianwhitedeveloper wants to merge 14 commits intoai-testing-framework-implementation-consolidationfrom
Open
feat(ai): outputFormat strategy + riteway ai init [8/8]#423ianwhitedeveloper wants to merge 14 commits intoai-testing-framework-implementation-consolidationfrom
ianwhitedeveloper wants to merge 14 commits intoai-testing-framework-implementation-consolidationfrom
Conversation
ianwhitedeveloper
force-pushed
the
pr/agent-output-format
branch
from
e314dd4 to
93b412d
Compare
ianwhitedeveloper
changed the base branch from
ai-testing-framework-implementation-consolidation
to
pr/ai-e2e-fixtures
ianwhitedeveloper
changed the base branch from
pr/ai-e2e-fixtures
to
ai-testing-framework-implementation-consolidation
This was referenced Mar 4, 2026
Collaborator
Author
|
@cursoragent please /review |
This comment was marked as resolved.
This comment was marked as resolved.
- Add no-prompt-under-test.sudo, missing-user-prompt.sudo, no-assertions.sudo fixtures to trigger extraction validation errors (MISSING_PROMPT_UNDER_TEST, MISSING_USER_PROMPT, NO_ASSERTIONS_FOUND) through the full E2E pipeline - Add sudolang-prompt-test.sudo fixture to verify the framework handles SudoLang syntax in the userPrompt field - Add describe.skipIf blocks for each validation error case and for the SudoLang happy-path scenario - Replace try/catch error capture with Try helper, consistent with ai-runner.test.js and test-extractor.test.js conventions - Update fixtures/README.md with new fixture descriptions Made-with: Cursor
- Replace { name, code } partial assertion shapes with full
error?.cause comparisons for all three validation error tests
(MISSING_PROMPT_UNDER_TEST, MISSING_USER_PROMPT, NO_ASSERTIONS_FOUND)
- Expected objects now include name, message, code, and testFile —
the complete deterministic cause shape from test-extractor.js
- Follows Jan's review principle from PR #409: deterministic
functions should assert the complete expected value, not
individual properties
Made-with: Cursor
- A1: validate userPrompt/assertions before resolveImportPaths so structural errors surface before any filesystem IO - A2: guard CONTEXT section in buildJudgePrompt matching existing buildResultPrompt pattern; add test for empty case - A3: use JSON.stringify for TAP YAML in mock helper, eliminating backtick-in-template fragility Made-with: Cursor
- Rewrite buildExtractionPrompt with explicit EXTRACTION RULES block: agent must return "" / [] for missing fields rather than inferring userPrompt or assertions from import/context - Update MISSING_USER_PROMPT and NO_ASSERTIONS_FOUND messages to correctly attribute failures to the agent, not the test file - Add explicit "return []" fallback to importPaths rule for consistency with rules 1 and 3 - Remove !assertions dead code (parseExtractionResult guarantees an array; only .length === 0 check is needed) - Update e2e expected messages and test-extractor snapshot test to match revised prompt and error messages - buildJudgePrompt now conditionally omits CONTEXT section when promptUnderTest is empty, matching buildResultPrompt pattern - Reorder extractTests validation: structural checks before IO Made-with: Cursor
- Replace parseOutput fn with declarative outputFormat string on all built-in agent configs (claude, opencode, cursor) - Add outputFormat lookup map in execute-agent.js (json/ndjson/text) - Add outputFormat to agentConfigFileSchema (z.enum, default 'json') — configs loaded from file are now fully serializable - Update tests: remove parseOutput mock, add real NDJSON integration test and schema round-trip test - Add source/fixtures/ndjson-agent-config.json fixture Made-with: Cursor
- Add `riteway ai init [--force]` subcommand that writes all built-in agent configs to riteway.agent-config.json - Add loadAgentRegistry / resolveAgentConfig: three-level priority chain (--agent-config > registry > built-ins) - Relax aiArgsSchema agent field to z.string().min(1) so custom registry agents pass validation before resolution - Add registry + bad-schema-registry + invalid-registry fixtures - fix: guard passRate against division by zero (NaN -> 0%) - refactor: import defaults/runsSchema/thresholdSchema/ concurrencySchema from constants.js; remove local duplicates - refactor: move agentConfigs to module scope in agent-config.js - chore: add "node": true to .eslintrc.json; switch agent-config.test.js to fileURLToPath(new URL(...)) - docs: add riteway ai section to README with .sudo example, options table, and riteway ai init walkthrough Made-with: Cursor
ianwhitedeveloper
force-pushed
the
pr/agent-output-format
branch
from
93b412d to
e48cee3
Compare
Collaborator
Author
The branch was rebased onto the merged PR #421 today. The PR 8 code is unchanged — only the base shifted. The review above still accurately reflects the current diff. One stale finding to note:
The Everything else in the review remains open and accurate. |
…ISTRY - Use destructuring default `outputFormat = 'json'` in execute-agent.js processAgentOutput instead of `|| 'json'` (project style: avoid || for defaults; Avoid using || for defaults. Use parameter defaults instead.) - Add AGENT_NOT_IN_REGISTRY code assertion to resolveAgentConfig test, matching the pattern used for INVALID_AI_ARGS in ai-command.test.js - Expand AGENT_NOT_IN_REGISTRY error message to include "remove the file to use built-in defaults" as a third recovery path Made-with: Cursor
…ptionsSchema - Remove 'text' from outputFormatParsers and agentConfigFileSchema enum: identical to 'json' (identity fn), never used by any config or fixture, rawOutput:true already handles the plain-string use case (YAGNI) - Remove agentSchema: only used by aiTestOptionsSchema, which is itself dead - Remove aiTestOptionsSchema: never called in production; runAITests takes plain JS parameter defaults. Its agent field still used the old enum of 3, contradicting PR 8's relaxed z.string().min(1) for custom registry agents - Delete constants.test.js: tested only the dead schema, not application logic - Remove constraints.supportedAgents: last consumer (agentSchema) is gone 14 test files, 209 tests, 0 lint errors. Made-with: Cursor
Made-with: Cursor
- Add --timeout MS CLI flag: parsed and validated via timeoutSchema (1000–3600000ms), flows through parseAIArgs → runAICommand → runAITests. timeoutSchema was exported but unconnected; now enforced. - Remove export from constraints in constants.js: only used internally to build the individual schemas; no external consumers. - Remove re-export of defaults from ai-command.js: bin/riteway.js already imports defaults directly from constants.js. - Keep authGuidance export in validation.js: legitimately imported by validation.test.js to assert the full error message contract. 211 tests, 0 lint errors. Made-with: Cursor
The --trust flag causes the cursor agent CLI to emit non-JSON content to stdout, breaking output parsing when spawned as a subprocess. Made-with: Cursor
Made-with: Cursor
- Unrecognized flags (e.g. --agentConfig) were silently ignored by minimist, causing misconfigured runs with no error. Now throws ValidationError naming the unknown flag(s). - Add test: unknown camelCase flag throws ValidationError with INVALID_AI_ARGS code and names the flag in the message. - Add human and agent user test scripts to plan/. Made-with: Cursor
- Steps 11-15 cover: writing the registry, using and customizing it, --force overwrite, and the exists-without-force error. - Renumber old validation error steps 11-25 to 16-30 to maintain a clean sequential index. Made-with: Cursor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
parseOutput: fnwith declarativeoutputFormat: 'json' | 'ndjson' | 'text'string in agent configs, making configs fully serializable for custom JSON config filesriteway ai init [--force]subcommand that writes all built-in agent configs toriteway.agent-config.json, enabling teams to add custom agents or modify existing entries--agent-config> project registry > built-in defaultsriteway aisection to README--agentConfig) were silently ignored by minimist — now throws aValidationErrornaming the unrecognized flag(s)Status: Code review remediated (2026-03-04) · Rebased onto merged PR #421 (2026-03-09) · 212 tests · 0 lint errors
Reviewer Verification
A human test script is available for manual verification of the
riteway aiCLI:plan/riteway-ai-command-human-test.mdIt covers 25 scenarios across the full flag surface (
--runs,--threshold,--timeout,--agent,--agent-config,--color,--concurrency), the three-level config resolution chain, and validation error cases including the--agentConfigcamelCase regression fixed in this PR. Reviewers can use it to walk through the CLI manually and confirm behavior matches expectations. I've successfully run through it myself (as well as some cases it didn't include around theinitfunctionality).