Phase 1 — 1.L.0: reconcile @relavium/shared to the 2026-06-05 contracts (+ sequencing plan)#6
Merged
Conversation
The phase doc had a per-workstream logical DAG (Work breakdown) and a Milestones table, but no explicit execution-order view. Add a "Sequencing & parallelization" section between them that answers: what order to build in, what blocks what, and what runs concurrently. - Three lanes behind one gate: 1.L.0 (do first) → Lane A (@relavium/llm, the binding critical path), Lane B (@relavium/core, concurrent), Lane C (agent-first sub-spine, additive — never gates M2). - The one scheduling insight: Lane B sprints ahead to 1.N/1.R then idles at the 1.O JOIN until Lane A delivers 1.K (which needs M1), so wall-clock to M2 is Lane A's length. - A complete lanes Mermaid (incl. 1.L.0, 1.L2, and 1.V–1.AA — which the Work-breakdown DAG omits), an ordered wave table (W0–W4 + Lane C), and a 31-row dependency matrix (depends-on / enables / on-the-M2-path). - Solo vs. two-track build guidance; a reconciliation note vs. the inline *critical path* tags. Single home for the execution plan — links to, not restates, the DAG/milestones. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…(1.L.0) Phase-1 workstream 1.L.0 — make the Zod schema set match the agent-first + hardening contracts (ADR-0024/0026/0027/0028/0029) that landed after the Phase-0 freeze. Additive, plus a few v1.0 tightenings; no engine behavior. Run/session events (run-event.ts ← sse-event-schema.md): - Split the base envelope into run / session / dual: run events require `runId`, session events require `sessionId`, the four reused events (agent:token / tool_call / tool_result / cost:updated) carry either. "Exactly one" is an emit-time invariant (a discriminatedUnion member cannot carry a cross-field refinement). - Add the 5 missing run events: run:paused, run:timeout, budget:warning, budget:paused, agent:file_patch_proposed (13 -> 18; count test updated). - Add the SessionEvent union (5 session:* variants) + SessionContextSchema. - Add the closed ErrorCode enum; bind node:failed / run:failed / session:turn_completed / RunSchema.error to it; add `retryable` to run:failed + RunSchema.error. Add attemptNumber? to agent:tool_call / tool_result / node:completed. Define StopReasonSchema (canonical home; the @relavium/llm seam re-exports it). Authored YAML (workflow.ts, node.ts <- workflow-yaml-spec.md): - workflow.metadata (ADR-0026 export transcript), budget / timeout_ms / max_parallel (ADR-0028), tools.allowedCommandGlobs (ADR-0029), WorkflowInput.validation, agent.system_prompt_append, agent/transform output_schema. - Drop reserved enum members so an authored value is rejected at parse: expression_type -> js only (jmespath/jsonlogic reserved), timeout_action -> reject/approve (escalate reserved). Config (config.ts <- config-spec.md): defaults.max_tokens_estimate + the [chat] block. +26 shared tests (130 -> 156). No migration (run_events.event_type is unconstrained text); tsc, the seam fence, and the db drift gate stay green. Flagged: workflow-yaml-spec.md says output_schema is "(also on transform / condition)"; the 1.L.0 task scopes it to agent/transform, so condition is omitted — a minor spec inconsistency for the maintainer to resolve. Refs: ADR-0024, ADR-0026, ADR-0027, ADR-0028, ADR-0029 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…L.0 review Two reviews of the 1.L.0 reconciliation. The BLOCKER: the four dual-envelope events (agent:token / tool_call / tool_result / cost:updated) declared both runId and sessionId optional, so RunEventSchema accepted payloads with neither or both — violating the canonical "exactly one" rule (sse-event-schema.md §"Correlation key"). - Enforce the invariant at the UNION level: RunEventSchema is now the discriminated union wrapped in a .superRefine requiring exactly one of runId / sessionId. (A discriminatedUnion *member* can't carry a cross-field refinement, but the union can.) Run-only / session-only events satisfy it by construction; the refine only constrains the four dual events. The count test reaches the raw union via RunEventSchema.innerType().options. New tests: neither and both are rejected (asserting the error is on the correlation key), each of run/session flavor accepted. - Document the deliberate input-leniency: a stray opposite key on a run-only/session-only event is stripped by its z.object before the refine runs, so the parsed output stays compliant (the non-strict, forward-compatible posture) — recorded, not an oversight. - Soften the dualBase comment (a union-level refine IS feasible) and the SessionContext comment (ContentPart must be shared-owned + seam-re-exported, never imported by shared from llm — the dependency must not invert). Docs (keep the canonical specs consistent with the shared code): - workflow-yaml-spec.md: drop "/ condition" from the output_schema note — a condition node selects a branch, it has no shaped output; the schema scopes output_schema to agent/transform. - agent-session-spec.md: clarify that only SessionContextSchema lands in 1.L.0; SessionMessageSchema / AgentSessionSchema land with the sub-spine (1.V/1.X). - deferred-tasks.md: track config-schema strictness parity, and codifying ContentPart's canonical home in the seam doc at 1.V/1.X. Verified by a 3-skeptic adversarial workflow (xor correctness, findings completeness, regressions): the fix holds, no finding dropped, no regression. Full gate green; +xor tests (156 shared tests); seam fence + db drift gate unchanged. Refs: ADR-0011, ADR-0024, ADR-0026 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reviewer's GuideReconciles @relavium/shared contracts to the 2026-06-05 ADRs by restructuring run/session event envelopes with strict correlation-key invariants, introducing new governance and session event types, tightening error and budget taxonomies, and extending workflow/node/config schemas for agent-first and resource-governance features, plus documenting the Phase-1 execution sequencing plan. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughExpands shared Zod schemas: adds dual-correlation run/session event envelopes and closed error/stop vocabularies; tightens run error shapes; adds node output-schema support; extends workflow input validation, tool globs, and budget controls; adds config chat defaults; and updates tests and docs. ChangesDocumentation and Roadmap
Shared Package Schemas: Constants, Nodes, Events, Workflow, Config, and Tests
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request reconciles the @relavium/shared package schemas with the latest specifications, introducing support for agent sessions, closed error code taxonomies, resource-governance budgets, and input validation rules. It also updates documentation and tests to align with these schema additions. The reviewer feedback suggests enhancing schema validation strictness by ensuring logical bounds on line selections and input validation limits, requiring at least one patch in proposed file events, and validating that allowed domains are exact FQDNs.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+88
to
+97
| export const SessionContextSchema = z.object({ | ||
| workingDir: nonEmptyString, | ||
| activeFile: nonEmptyString.optional(), | ||
| selection: z | ||
| .object({ file: nonEmptyString, startLine: nonNegativeInt, endLine: nonNegativeInt }) | ||
| .optional(), | ||
| gitRef: nonEmptyString.optional(), | ||
| fsScopeTier: z.enum(FS_SCOPE_TIERS), | ||
| variables: z.record(z.string(), z.string()).optional(), | ||
| }); |
There was a problem hiding this comment.
The selection object in SessionContextSchema defines startLine and endLine as non-negative integers. However, there is no validation ensuring that startLine is less than or equal to endLine. Adding a refinement prevents logical errors where a selection's start line exceeds its end line.
export const SessionContextSchema = z.object({
workingDir: nonEmptyString,
activeFile: nonEmptyString.optional(),
selection: z
.object({ file: nonEmptyString, startLine: nonNegativeInt, endLine: nonNegativeInt })
.superRefine((val, ctx) => {
if (val.startLine > val.endLine) {
ctx.addIssue({
code: z.ZodIssueCode.custom,
message: 'startLine must be <= endLine',
path: ['startLine'],
});
}
})
.optional(),
gitRef: nonEmptyString.optional(),
fsScopeTier: z.enum(FS_SCOPE_TIERS),
variables: z.record(z.string(), z.string()).optional(),
});
Comment on lines
+145
to
152
| export const AgentFilePatchProposedEventSchema = z.object({ | ||
| type: z.literal('agent:file_patch_proposed'), | ||
| ...runBase, | ||
| nodeId: nonEmptyString, | ||
| // Gated — no write until the user accepts (e.g. the VS Code inline-diff review). | ||
| patches: z.array(z.object({ uri: nonEmptyString, unifiedDiff: z.string() })), | ||
| attemptNumber: positiveInt.optional(), | ||
| }); |
There was a problem hiding this comment.
The patches array in AgentFilePatchProposedEventSchema is currently allowed to be empty. Proposing an event with zero patches is logically invalid. Adding a .min(1) constraint ensures that at least one file patch is proposed, aligning with other non-empty array schemas like RunPausedEventSchema.gateIds.
Suggested change
| export const AgentFilePatchProposedEventSchema = z.object({ | |
| type: z.literal('agent:file_patch_proposed'), | |
| ...runBase, | |
| nodeId: nonEmptyString, | |
| // Gated — no write until the user accepts (e.g. the VS Code inline-diff review). | |
| patches: z.array(z.object({ uri: nonEmptyString, unifiedDiff: z.string() })), | |
| attemptNumber: positiveInt.optional(), | |
| }); | |
| export const AgentFilePatchProposedEventSchema = z.object({ | |
| type: z.literal('agent:file_patch_proposed'), | |
| ...runBase, | |
| nodeId: nonEmptyString, | |
| // Gated — no write until the user accepts (e.g. the VS Code inline-diff review). | |
| patches: z.array(z.object({ uri: nonEmptyString, unifiedDiff: z.string() })).min(1), | |
| attemptNumber: positiveInt.optional(), | |
| }); |
Comment on lines
+65
to
+75
| export const InputValidationSchema = z | ||
| .object({ | ||
| format: nonEmptyString.optional(), // e.g. 'email' | ||
| pattern: nonEmptyString.optional(), // a regex source | ||
| enum: z.array(z.unknown()).optional(), | ||
| min: z.number().optional(), | ||
| max: z.number().optional(), | ||
| min_length: nonNegativeInt.optional(), | ||
| max_length: nonNegativeInt.optional(), | ||
| }) | ||
| .strict(); |
There was a problem hiding this comment.
The InputValidationSchema defines optional min/max and min_length/max_length fields, but does not validate their logical relationships. Adding a refinement ensures that min is less than or equal to max, and min_length is less than or equal to max_length, preventing invalid validation rules from being declared.
export const InputValidationSchema = z
.object({
format: nonEmptyString.optional(), // e.g. 'email'
pattern: nonEmptyString.optional(), // a regex source
enum: z.array(z.unknown()).optional(),
min: z.number().optional(),
max: z.number().optional(),
min_length: nonNegativeInt.optional(),
max_length: nonNegativeInt.optional(),
})
.strict()
.superRefine((val, ctx) => {
if (val.min !== undefined && val.max !== undefined && val.min > val.max) {
ctx.addIssue({
code: z.ZodIssueCode.custom,
message: 'min must be <= max',
path: ['min'],
});
}
if (val.min_length !== undefined && val.max_length !== undefined && val.min_length > val.max_length) {
ctx.addIssue({
code: z.ZodIssueCode.custom,
message: 'min_length must be <= max_length',
path: ['min_length'],
});
}
});
Comment on lines
104
to
110
| export const ToolPolicySchema = z | ||
| .object({ | ||
| allowedCommands: z.array(nonEmptyString).optional(), | ||
| allowedCommandGlobs: z.array(nonEmptyString).optional(), | ||
| allowedDomains: z.array(nonEmptyString).optional(), | ||
| }) | ||
| .strict(); |
There was a problem hiding this comment.
The allowedDomains field in ToolPolicySchema is intended to be an exact-FQDN allowlist. However, users might mistakenly include protocols (e.g., https://) or paths (e.g., /v1). Adding a refinement to validate that the domains do not contain slashes, colons, or start with http prevents runtime matching failures.
export const ToolPolicySchema = z
.object({
allowedCommands: z.array(nonEmptyString).optional(),
allowedCommandGlobs: z.array(nonEmptyString).optional(),
allowedDomains: z
.array(
nonEmptyString.refine((dom) => !dom.includes('/') && !dom.includes(':') && !dom.startsWith('http'), {
message: 'allowedDomains must be exact FQDNs (no protocol, port, or path)',
}),
)
.optional(),
})
.strict();![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="packages/shared/src/workflow.ts" line_range="119-122" />
<code_context>
+ * (integer micro-cents) and applies `on_exceed`. The whole-run `timeout_ms` and the concurrency
+ * cap `max_parallel` live alongside `budget` on the workflow spec.
+ */
+export const BudgetSchema = z
+ .object({
+ max_cost_microcents: nonNegativeInt,
+ on_exceed: z.enum(ON_EXCEED_ACTIONS),
+ })
+ .strict();
</code_context>
<issue_to_address>
**question:** Clarify whether a zero budget is valid and, if not, tighten the type
`max_cost_microcents` currently uses `nonNegativeInt`, so `0` is allowed. For workflows, is `0` intended as “no budget” or should it be rejected as invalid? `ChatConfigSchema` documents `0/absent` as unbounded, but there’s no similar note here. If `0` isn’t a meaningful value for workflow budgets, consider using `positiveInt` instead, or document the intended semantics to avoid inconsistent expectations between chat and workflow budgets.
</issue_to_address>
### Comment 2
<location path="docs/roadmap/deferred-tasks.md" line_range="75-79" />
<code_context>
+ never imported by shared from llm — which would invert the package dependency. The seam doc
+ ([llm-provider-seam.md](../reference/shared-core/llm-provider-seam.md)) currently shows
+ `ContentPart` only as a TS shape with no ownership statement; codify it there so the code
+ comment in `run-event.ts` and the spec stay aligned. *(nit → 1.V/1.X · llm-provider-seam.md)*
## Test depth
</code_context>
<issue_to_address>
**nitpick (typo):** Fix subject–verb agreement in "the spec stay aligned."
This should be “the spec stays aligned” to agree with the singular subject (“so the code comment in `run-event.ts` and the spec stays aligned”).
```suggestion
`@relavium/shared`** and re-exported by the `@relavium/llm` seam (the `StopReason` precedent),
never imported by shared from llm — which would invert the package dependency. The seam doc
([llm-provider-seam.md](../reference/shared-core/llm-provider-seam.md)) currently shows
`ContentPart` only as a TS shape with no ownership statement; codify it there so the code
comment in `run-event.ts` and the spec stays aligned. *(nit → 1.V/1.X · llm-provider-seam.md)*
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/shared/src/run-event.test.ts (1)
197-200:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep the
partialOutputsnegative case single-cause.This payload now also violates the closed
error.codeand requiredretryablerules, so it no longer proves thatpartialOutputsis required. Use a valid error object here to keep that contract covered.🧪 Proposed fix
'run:failed (missing partialOutputs)': { type: 'run:failed', ...env, - error: { code: 'E', message: 'm' }, + error: { code: 'internal', message: 'm', retryable: false }, },🤖 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 `@packages/shared/src/run-event.test.ts` around lines 197 - 200, The failing negative test case labeled 'run:failed (missing partialOutputs)' currently uses an error object that also violates the closed error.code and missing retryable rules; update the error payload in that test to be a valid error object (include a permitted code string and the required retryable boolean plus message) so the test only asserts the absence of partialOutputs, e.g. change the error value used in that case inside run-event.test.ts to a compliant error shape.
🧹 Nitpick comments (2)
packages/shared/src/run-event.ts (1)
339-352: ⚡ Quick winExport a schema for the complete session stream.
SessionEventSchemacurrently covers only thesession:*lifecycle variants, but the shared turn events (agent:token,agent:tool_call,agent:tool_result,cost:updated) are still session-correlated. Without a first-class union here, downstream code has to duplicate this composition or parse session traffic withRunEventSchema, which also accepts run-only variants.♻️ Proposed addition
export const SessionEventSchema = z.discriminatedUnion('type', [ SessionStartedEventSchema, SessionTurnStartedEventSchema, SessionTurnCompletedEventSchema, SessionCancelledEventSchema, SessionExportedEventSchema, ]); export type SessionEvent = z.infer<typeof SessionEventSchema>; + +export const SessionStreamEventSchema = z.union([ + SessionEventSchema, + AgentTokenEventSchema, + AgentToolCallEventSchema, + AgentToolResultEventSchema, + CostUpdatedEventSchema, +]); +export type SessionStreamEvent = z.infer<typeof SessionStreamEventSchema>;🤖 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 `@packages/shared/src/run-event.ts` around lines 339 - 352, SessionEventSchema only includes the five session:* lifecycle schemas but must also include the shared turn event schemas so downstreams can parse a full session stream; update the discriminated union for SessionEventSchema to include the four shared turn schemas (e.g., AgentTokenEventSchema, AgentToolCallEventSchema, AgentToolResultEventSchema, CostUpdatedEventSchema—or the exact names used in the file) alongside SessionStartedEventSchema, SessionTurnStartedEventSchema, SessionTurnCompletedEventSchema, SessionCancelledEventSchema, and SessionExportedEventSchema, then keep exporting the inferred SessionEvent type so it represents the complete session stream.packages/shared/src/workflow.test.ts (1)
356-377: ⚡ Quick winAdd regression cases for semantic validation failures.
These tests only cover the happy path and unknown-key strictness. Please also lock down contradictory bounds and type-incompatible validation blocks so the new contract doesn't regress once the schema is tightened.
🤖 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 `@packages/shared/src/workflow.test.ts` around lines 356 - 377, Add tests to cover semantic validation failures by extending workflow.test.ts: use the existing accepts and withWorkflow helpers to assert that validation objects with contradictory bounds (e.g., validation: { min: 10, max: 5 } on an input like name: 'sev', type: 'number') are rejected, and that type-incompatible validation keys are rejected (e.g., apply string-only keys like format or max_length to a number input, or numeric keys like min/max to a string input). Place these new expect(...).toBe(false) cases next to the existing tests so accepts(withWorkflow({...})) returns false for both contradictory min/max and mismatched validation-key vs input type.
🤖 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 `@packages/shared/src/run.ts`:
- Around line 56-62: The error object schema currently allows an empty nodeId
(nodeId: z.string().optional()); change it to use the same non-empty identifier
contract as the rest of the package (e.g., replace with
nonEmptyString().optional() or z.string().min(1).optional() using the existing
nonEmptyString helper) inside the error schema and make the identical change in
RunFailedEventSchema.error.nodeId so both contracts remain aligned with
ErrorCodeSchema and other node id validations.
In `@packages/shared/src/workflow.ts`:
- Around line 65-85: Add schema-level validations to reject contradictory or
type-incompatible constraints: update InputValidationSchema to include
refinements that enforce numeric bounds (if both min and max are present,
require min <= max) and length bounds (if both min_length and max_length are
present, require min_length <= max_length), and update WorkflowInputSchema to
superRefine the pair (type, validation) so string-only constraints (format,
pattern, min_length, max_length) are only allowed when the declared
InputTypeSchema corresponds to a string-type; use z.refine/z.superRefine on
InputValidationSchema and WorkflowInputSchema respectively, referencing the
existing symbols InputValidationSchema, WorkflowInputSchema, validation, min,
max, min_length, max_length, format, and pattern to find where to add the
checks.
---
Outside diff comments:
In `@packages/shared/src/run-event.test.ts`:
- Around line 197-200: The failing negative test case labeled 'run:failed
(missing partialOutputs)' currently uses an error object that also violates the
closed error.code and missing retryable rules; update the error payload in that
test to be a valid error object (include a permitted code string and the
required retryable boolean plus message) so the test only asserts the absence of
partialOutputs, e.g. change the error value used in that case inside
run-event.test.ts to a compliant error shape.
---
Nitpick comments:
In `@packages/shared/src/run-event.ts`:
- Around line 339-352: SessionEventSchema only includes the five session:*
lifecycle schemas but must also include the shared turn event schemas so
downstreams can parse a full session stream; update the discriminated union for
SessionEventSchema to include the four shared turn schemas (e.g.,
AgentTokenEventSchema, AgentToolCallEventSchema, AgentToolResultEventSchema,
CostUpdatedEventSchema—or the exact names used in the file) alongside
SessionStartedEventSchema, SessionTurnStartedEventSchema,
SessionTurnCompletedEventSchema, SessionCancelledEventSchema, and
SessionExportedEventSchema, then keep exporting the inferred SessionEvent type
so it represents the complete session stream.
In `@packages/shared/src/workflow.test.ts`:
- Around line 356-377: Add tests to cover semantic validation failures by
extending workflow.test.ts: use the existing accepts and withWorkflow helpers to
assert that validation objects with contradictory bounds (e.g., validation: {
min: 10, max: 5 } on an input like name: 'sev', type: 'number') are rejected,
and that type-incompatible validation keys are rejected (e.g., apply string-only
keys like format or max_length to a number input, or numeric keys like min/max
to a string input). Place these new expect(...).toBe(false) cases next to the
existing tests so accepts(withWorkflow({...})) returns false for both
contradictory min/max and mismatched validation-key vs input type.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: a1845718-46dd-4a46-bd59-c2097d5bae69
📒 Files selected for processing (15)
docs/reference/contracts/agent-session-spec.mddocs/reference/contracts/workflow-yaml-spec.mddocs/roadmap/deferred-tasks.mddocs/roadmap/phases/phase-1-engine-and-llm.mdpackages/shared/src/config.test.tspackages/shared/src/config.tspackages/shared/src/constants.tspackages/shared/src/node.test.tspackages/shared/src/node.tspackages/shared/src/run-event.test.tspackages/shared/src/run-event.tspackages/shared/src/run.test.tspackages/shared/src/run.tspackages/shared/src/workflow.test.tspackages/shared/src/workflow.ts
Comment on lines
+65
to
+85
| export const InputValidationSchema = z | ||
| .object({ | ||
| format: nonEmptyString.optional(), // e.g. 'email' | ||
| pattern: nonEmptyString.optional(), // a regex source | ||
| enum: z.array(z.unknown()).optional(), | ||
| min: z.number().optional(), | ||
| max: z.number().optional(), | ||
| min_length: nonNegativeInt.optional(), | ||
| max_length: nonNegativeInt.optional(), | ||
| }) | ||
| .strict(); | ||
| export type InputValidation = z.infer<typeof InputValidationSchema>; | ||
|
|
||
| export const WorkflowInputSchema = z | ||
| .object({ | ||
| name: nonEmptyString, | ||
| type: InputTypeSchema, | ||
| required: z.boolean().optional(), | ||
| default: z.unknown().optional(), | ||
| description: z.string().optional(), | ||
| validation: InputValidationSchema.optional(), |
There was a problem hiding this comment.
Reject contradictory and type-incompatible input constraints.
validation currently only checks field presence, so impossible or nonsensical authored inputs still parse here — e.g. { min: 10, max: 1 }, { min_length: 8, max_length: 3 }, or a non-string input carrying string-only constraints. Since this package is the contract boundary, these should fail during schema validation instead of being deferred to the engine.
🤖 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 `@packages/shared/src/workflow.ts` around lines 65 - 85, Add schema-level
validations to reject contradictory or type-incompatible constraints: update
InputValidationSchema to include refinements that enforce numeric bounds (if
both min and max are present, require min <= max) and length bounds (if both
min_length and max_length are present, require min_length <= max_length), and
update WorkflowInputSchema to superRefine the pair (type, validation) so
string-only constraints (format, pattern, min_length, max_length) are only
allowed when the declared InputTypeSchema corresponds to a string-type; use
z.refine/z.superRefine on InputValidationSchema and WorkflowInputSchema
respectively, referencing the existing symbols InputValidationSchema,
WorkflowInputSchema, validation, min, max, min_length, max_length, format, and
pattern to find where to add the checks.
Triage of a batch of PR #6 review comments on the 1.L.0 reconciliation. Verified each against the canonical contracts; fixed the valid ones, skipped the rest with reasons. Fixed: - error.nodeId is now nonEmptyString (run:failed.error + RunSchema.error) — a node id is never empty, matching every other nodeId field. - agent:file_patch_proposed.patches is .min(1) — an empty proposal is meaningless (mirrors run:paused.gateIds.min(1)); noted in sse-event-schema.md. - SessionContext.selection enforces startLine <= endLine. - BudgetSchema.max_cost_microcents is positiveInt — a declared workflow budget caps at a positive value; omit `budget` for no cap. (Distinct from [chat].max_cost_microcents, an always-present default where 0 = unbounded — documented inline.) - InputValidationSchema rejects contradictory bounds (min > max, min_length > max_length) at parse (ADR-0023). - Test: the run:failed (missing partialOutputs) reject fixture now uses a compliant error, so it isolates the intended failure. +reject tests for every refinement (160 shared tests). Skipped, with reason: - Add the 4 reused turn events to SessionEventSchema: the spec NAMES `SessionEvent` as exactly the 5 session:* lifecycle events; the reused four validate via RunEventSchema (sessionId). Widening it would diverge from the canonical contract. - Per-input-type validation-key matrix (e.g. `format` only on string): the contract defines no such matrix — tracked in deferred-tasks.md for a follow-up. - allowedDomains FQDN/SSRF format check: the egress guard is engine-side (ADR-0029, 1.T); shared owns the shape. Verified by a 2-skeptic adversarial workflow (triage correctness + over-rejection / regression): both clean — no fix over-reaches, no skip should have been a fix, and no refinement rejects a contract-valid payload (min==max, single-line selections, omitted optionals all pass). Full gate green; seam fence + db drift gate unchanged. Refs: ADR-0023, ADR-0028, ADR-0029 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… accuracy A final comprehensive multi-agent review (8 dimensions → adversarial verify ×2 → completeness critic) surfaced 12 confirmed items on the 1.L.0 change set — no blockers; test-coverage gaps (testing.md accept+reject rule) and comment/doc-accuracy nits. All addressed. Test coverage (every new schema constraint now has a reject that fails on revert): - The four governance events (run:paused, run:timeout, budget:warning, budget:paused) get targeted rejects: empty gateIds, pendingGateCount 0, negative elapsedMs, thresholdPct > 100, negative spentMicrocents. - StopReason closed-enum reject (session:turn_completed); SessionEvent per-variant required-field rejects (session:exported empty workflowPath, session:started missing model, session:turn_completed missing tokensUsed). - attemptNumber loop now covers all five carriers (adds cost:updated, agent:file_patch_proposed). - RunSchema.error.nodeId empty-string reject; [chat].max_cost_microcents: 0 accept (locks the "0 = unbounded" semantic that deliberately differs from the workflow budget's positiveInt). Comment / doc accuracy: - constants.ts: RUN_EVENT_TYPES order comment (agent:file_patch_proposed sits mid-list; only the four governance events close it); StopReason "canonical home" softened to not over-assert (the seam doc still defines it locally — tracked). - node.ts: ExpressionTypeSchema JSDoc drops merge_fn (merge_fn is always js, no expression_type). - phase-1 doc: the 1.L.0 dependency-matrix Enables cell is now the direct-edge set (1.A, 1.L, 1.W) with a transitive-gating note; a footnote marks the matrix authoritative for Lane-C cross-lane feeder edges the waves Mermaid omits. Deferred (tracked, not silently dropped): - AgentSchema input_schema/output_schema (pre-existing spec gap; agent.ts untouched by 1.L.0). - Broadened the ContentPart seam-doc-codification entry to also cover StopReason. 160 -> 167 shared tests. Full gate green; seam fence + db drift gate unchanged. Refs: ADR-0023, ADR-0028 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
cemililik
added a commit
that referenced
this pull request
Jun 5, 2026
Phase 1 Wave 0 — the @relavium/shared reconciliation (1.L.0) — is merged. Flip the phase status to In progress, mark the 1.L.0 workstream ✅ Done, and rewrite current.md's immediate next steps to the two Wave-1 lanes per the sequencing plan: 1.A (freeze the LLMProvider seam types, scaffold packages/llm) and 1.L (WorkflowYAMLParser, scaffold packages/core). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
6 tasks
This was referenced Jun 7, 2026
cemililik
added a commit
that referenced
this pull request
Jun 21, 2026
… and milestones - Mark Phase 1 as complete with all workstreams (1.A–1.AH) merged (PR #6–#38). - Achieve global milestones M1 (LLM seam proven) and M2 (engine end-to-end). - Detail the completion of the critical path and additive sub-spines, including Lane C and multimodal sub-spine. - Document the decision on multimodal I/O and its integration into the roadmap.
This was referenced Jun 22, 2026
cemililik
added a commit
that referenced
this pull request
Jun 25, 2026
… status, defer log, concurrency) Five review findings on the CLI media surface (all low/nit): - #12 (nit, 3-lens): wireSaveToPort called mkdirSync on every write (blocking sync I/O in an async port) while the JSDoc implied once-only. Switch to await mkdir (node:fs/promises) — matches the FilesystemMediaStore.put pattern, keeps the port non-blocking — and reword the JSDoc to "every write". - #13 (nit): TERMINAL_RUN_STATUSES typed Set<RunStatus> (was Set<string>), so a misspelled status is a compile error and .has() narrows the closed union. - #7 (low): createWorkflowModelCatalog deferred silently on a CapabilityFlagsSchema.safeParse failure — indistinguishable from "model absent". Add an optional, per-model-deduped, secret-free warn sink (model id + Zod issue messages) threaded from run/gate via io.writeErr, so a future schema evolution that invalidates previously-valid rows is observable. Still fail-open (FallbackChain backstop). - #6 (low): document the grace-window soft-delete→unlink resurrection gap (within ADR-0042 §3 best-effort) so a future graceMs shortening triggers a re-verify-before-delete. - #5 companion (low, PERF-CONCURRENCY-2): the grace-reclaim and orphan-sweep CAS deletes ran one await at a time on the exit path — fan them out with Promise.all (independent unlinks). Refs: ADR-0042 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
First Phase-1 workstream. Brings
@relavium/sharedup to the agent-first + hardeningcontracts (ADR-0024/0026/0027/0028/0029) that landed the day after the Phase-0 freeze, and
adds the Phase-1 execution-order plan. Additive, plus a few v1.0 tightenings — no engine
behavior (
@relavium/sharedonly, plus docs).Commits
2bda8d6docs(roadmap): Phase-1 sequencing & parallelization plan — three lanes behind the 1.L.0 gate, the 1.O join, an ordered wave table, and a 31-row dependency matrix.45349cdfeat(shared): 1.L.0 reconciliation — the schema catch-up.81728b5fix(shared): address the two code reviews (the run/session correlation-key BLOCKER + 5 findings).What changed (1.L.0)
Run/session events (
run-event.ts← sse-event-schema.md): run/session/dual envelope with exactly one ofrunId/sessionIdenforced at the union level; the 5 missing run events (13→18); theSessionEventunion (5session:*) +SessionContextSchema; the closedErrorCodeenum bound tonode:failed/run:failed/session:turn_completed/RunSchema.error;retryable;attemptNumber?;StopReasonSchema(canonical home — the seam re-exports it).Authored YAML (
workflow.ts,node.ts← workflow-yaml-spec.md):metadata,budget/timeout_ms/max_parallel,allowedCommandGlobs,WorkflowInput.validation,system_prompt_append, agent/transformoutput_schema. Reserved values now rejected at parse:expression_type→js,timeout_action→reject/approve.Config (
config.ts← config-spec.md):defaults.max_tokens_estimate+ the[chat]block.Review handling
.superRefine(a discriminatedUnion member can't be refined, but the union can);.innerType()for the count test; neither/both reject tests.condition/output_schemaspec fix, config-strictness parity (deferred), the agent-session "validated by" clarification.Conformance
any/unsafeas@relavium/llmseam (fence airtight;StopReason/ContentPartkept shared-owned)sequenceNumberpnpm turbo run lint typecheck test buildgreen · 156 shared tests · format clean · seam fence airtight · no DB migration drift (run_events.event_typeis unconstrained text)Notes
docs/roadmap/deferred-tasks.md): config-schema strictness parity; codifyingContentPart's canonical home in the seam doc at 1.V/1.X.SessionMessageSchema/AgentSessionSchemadeliberately deferred to the agent-first sub-spine (1.V/1.X) — they reference the seam'sContentPart, which lands with 1.A.Refs: ADR-0023, ADR-0024, ADR-0026, ADR-0027, ADR-0028, ADR-0029
🤖 Generated with Claude Code
Summary by Sourcery
Reconcile the shared contracts with the latest agent-first and hardening ADRs, extending run/session event schemas, workflow YAML, and config to support governance and agent-session use cases while adding a Phase-1 execution sequencing plan.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests:
Chores:
Summary by CodeRabbit
New Features
Documentation
Tests