refactor(workflow): strengthen contracts and zod boundaries - #100
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/workflow-types.ts (1)
13-44: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsolidate the duplicate provider ID aliases.
AgentProviderIdis defined here andWorkflowProviderIdis defined inworkflow-contracts.ts, both asLocalAgentProvider. Either import/export the consolidatedWorkflowProviderIdhere and reuse it atWorkflowAgentCallRecord.provider,AgentCacheKeyInput.provider, andbuildAgentCacheKeyInput(provider: ...), or exportAgentProviderIdfromworkflow-contracts.tsif callers should depend on the single contract alias.🤖 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 `@src/workflow-types.ts` around lines 13 - 44, Consolidate the provider ID aliases by using a single contract-level alias: import and re-export WorkflowProviderId from workflow-contracts.ts, then update WorkflowAgentCallRecord.provider, AgentCacheKeyInput.provider, and the buildAgentCacheKeyInput(provider: ...) parameter to use it consistently. Remove the duplicate AgentProviderId definition rather than maintaining parallel aliases.
🤖 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 `@src/json-types.ts`:
- Around line 40-42: Update parseJsonText to catch JSON.parse failures and
convert malformed input into the same clear validation-error type used by
parseJsonValue, while preserving normal parsing and validation behavior for
valid JSON.
In `@src/workflow-script.ts`:
- Around line 186-201: Update validateMeta to enable Zod's reportInput option
when calling workflowMetaSchema.safeParse, so issue.input is available for
distinguishing missing values from present values with invalid types. Preserve
the existing required-field handling for undefined input and allow present
wrong-type fields such as meta.concurrency to use the normal validation error
path.
In `@src/workflow-store.ts`:
- Around line 566-572: Update the bulk mapping paths in listRuns() and
listAgentCalls(), including rowToRun, rowToEvent, and rowToAgentCall, so invalid
persisted enum-like values are handled per row with safe parsing and corrupted
rows are skipped or flagged rather than aborting the entire list operation.
Preserve strict .parse() behavior for single-record getRun() and getAgentCall()
lookups.
---
Nitpick comments:
In `@src/workflow-types.ts`:
- Around line 13-44: Consolidate the provider ID aliases by using a single
contract-level alias: import and re-export WorkflowProviderId from
workflow-contracts.ts, then update WorkflowAgentCallRecord.provider,
AgentCacheKeyInput.provider, and the buildAgentCacheKeyInput(provider: ...)
parameter to use it consistently. Remove the duplicate AgentProviderId
definition rather than maintaining parallel aliases.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0546af15-0cb2-4c3c-ad46-41c0315e5417
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (24)
package.jsonsrc/json-types.tssrc/local-agent-adapters.test.tssrc/local-agent-adapters.tssrc/local-agent-capabilities.tssrc/local-agent-profiles.tssrc/local-agent-runtime.tssrc/workflow-api.tssrc/workflow-cli.tssrc/workflow-contracts.test.tssrc/workflow-contracts.tssrc/workflow-engine.tssrc/workflow-files.tssrc/workflow-replay.tssrc/workflow-sandbox.test.tssrc/workflow-sandbox.tssrc/workflow-schema.test.tssrc/workflow-schema.tssrc/workflow-script.test.tssrc/workflow-script.tssrc/workflow-store.test.tssrc/workflow-store.tssrc/workflow-tools.tssrc/workflow-types.ts
| export function parseJsonText(text: string): JsonValue { | ||
| return parseJsonValue(JSON.parse(text) as unknown); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
parseJsonText lets raw JSON.parse errors escape uncaught.
JSON.parse throws a native SyntaxError (not a Zod validation error) on malformed text. Since this helper is used to reload persisted state (e.g. workflow run args/JSON blobs), a corrupted persisted value would surface as an unstructured SyntaxError rather than a clear validation failure at the call site.
🛡️ Proposed fix
export function parseJsonText(text: string): JsonValue {
- return parseJsonValue(JSON.parse(text) as unknown);
+ let parsed: unknown;
+ try {
+ parsed = JSON.parse(text);
+ } catch (error) {
+ throw new Error(`Invalid JSON text: ${error instanceof Error ? error.message : String(error)}`);
+ }
+ return parseJsonValue(parsed);
}📝 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.
| export function parseJsonText(text: string): JsonValue { | |
| return parseJsonValue(JSON.parse(text) as unknown); | |
| } | |
| export function parseJsonText(text: string): JsonValue { | |
| let parsed: unknown; | |
| try { | |
| parsed = JSON.parse(text); | |
| } catch (error) { | |
| throw new Error(`Invalid JSON text: ${error instanceof Error ? error.message : String(error)}`); | |
| } | |
| return parseJsonValue(parsed); | |
| } |
🤖 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 `@src/json-types.ts` around lines 40 - 42, Update parseJsonText to catch
JSON.parse failures and convert malformed input into the same clear
validation-error type used by parseJsonValue, while preserving normal parsing
and validation behavior for valid JSON.
There was a problem hiding this comment.
Not included in the finalized hardening stack. Persisted-state call sites already wrap malformed JSON with contextual WorkflowStoredDataError or explicitly treat it as a replay miss; this comment concerns the generic helper’s standalone error wording. It remains reasonable cleanup, but is not a blocker for cross-harness workflow orchestration.
There was a problem hiding this comment.
@Waishnav, acknowledged. Given the existing call-site handling, this is non-blocking for the workflow hardening scope. I’ll treat helper-level parse-error normalization as optional follow-up cleanup rather than a required change for this PR.
Would you like me to open a follow-up issue for the standalone parseJsonText error consistency improvement?
✏️ Learnings added
Learnt from: Waishnav
Repo: Waishnav/devspace PR: 100
File: src/json-types.ts:40-42
Timestamp: 2026-07-26T21:01:08.221Z
Learning: In `src/json-types.ts`, `parseJsonText` may expose native `JSON.parse` errors, but persisted workflow-state consumers already either wrap malformed JSON with contextual `WorkflowStoredDataError` or treat it as a replay miss. Helper-level error normalization is optional cleanup, not a blocker for cross-harness workflow orchestration.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
refactor(agents): unify provider resolution and subagent guidance
feat(workflow): support agent profiles and project workflows
fix(workflow): replay deterministic call prefixes
fix(workflow): supervise cancellation and terminal lifecycle
feat(workflow): isolate workflow script execution
feat(ui): live workspace and workflow dashboards
feat(workflow): read-only project workflow TUI
feat(workflow): script iteration, replay provenance, and call inspection
refactor(workflow): model operational failures with better-result
1 participant
Workflow values crossed script, provider, database, replay, CLI, and MCP boundaries with several loosely typed JSON and event shapes. Invalid persisted data or provider capabilities could therefore travel too far before failing, and TypeScript could not preserve useful output and tuple types across the public API.
Define shared JSON-safe values and schema contracts, validate metadata, options, states, events, providers, and persisted payloads with Zod, and make event and capability handling exhaustive. Schema-derived
agent()output types, tuple-preserving parallel results, and typed pipeline overloads make the public API match the runtime contract.Stack
Based on #99 and merged into #94 before operational error normalization in #101.
Summary by CodeRabbit