chore: consolidate specs into OpenSpec#23
Closed
coji wants to merge 1 commit into
Closed
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughThis PR migrates documentation specifications from the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
openspec/changes/add-human-in-the-loop/specs/core/spec.md (2)
35-38: Avoid SHOULD for normative requirement wording.Spec guidance here prefers SHALL/MUST unless intentionally non‑normative. Consider promoting to MUST or explicitly marking it non‑normative. As per coding guidelines, avoid should/may in spec requirements.
38-41: Fix list indentation to keep bullets consistent.The extra leading space before the bullets can break list rendering.
🔧 Suggested fix
- - The system MUST return `404` when `runId` does not exist - - The system MUST return `410` when `wait_deadline_at` has expired +- The system MUST return `404` when `runId` does not exist +- The system MUST return `410` when `wait_deadline_at` has expired
🤖 Fix all issues with AI agents
In `@openspec/changes/add-human-in-the-loop/specs/react/spec.md`:
- Around line 11-14: The spec uses non-normative modal verbs (SHOULD) for what
are clearly required behaviors: update the lines referencing the hook parameters
and return values—specifically the `jobName` parameter, the `payloadSchema`
parameter, the returned object `{ waits, isLoading, reload, respond }`, and the
`respond(runId, payload)` behavior—to use normative language (replace SHOULD
with MUST) or explicitly mark them as non‑normative; ensure `respond(runId,
payload)` remains a MUST for resuming a Run and that the return contract `{
waits, isLoading, reload, respond }` is stated as mandatory.
Comment on lines
+11
to
+14
| - The hook SHOULD accept `jobName` to filter waits by job | ||
| - The hook SHOULD accept `payloadSchema` to enable type inference and client-side validation | ||
| - The hook MUST return `{ waits, isLoading, reload, respond }` | ||
| - `respond(runId, payload)` MUST resume the Run with the given payload |
Contributor
There was a problem hiding this comment.
Use SHALL/MUST for normative requirements.
The SHOULDs read like normative requirements. Consider upgrading to MUST or explicitly noting non‑normative intent. As per coding guidelines, avoid should/may in spec requirements.
✏️ Possible wording update
-- The hook SHOULD accept `jobName` to filter waits by job
-- The hook SHOULD accept `payloadSchema` to enable type inference and client-side validation
+- The hook MUST accept `jobName` to filter waits by job
+- The hook MUST accept `payloadSchema` to enable type inference and client-side validation
...
-- A WaitingRun SHOULD include `wait_schema` when available
+- A WaitingRun MUST include `wait_schema` when availableAlso applies to: 73-75
🤖 Prompt for AI Agents
In `@openspec/changes/add-human-in-the-loop/specs/react/spec.md` around lines 11 -
14, The spec uses non-normative modal verbs (SHOULD) for what are clearly
required behaviors: update the lines referencing the hook parameters and return
values—specifically the `jobName` parameter, the `payloadSchema` parameter, the
returned object `{ waits, isLoading, reload, respond }`, and the `respond(runId,
payload)` behavior—to use normative language (replace SHOULD with MUST) or
explicitly mark them as non‑normative; ensure `respond(runId, payload)` remains
a MUST for resuming a Run and that the return contract `{ waits, isLoading,
reload, respond }` is stated as mandatory.
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\n\n- Remove duplicated docs/spec-*.md files and keep OpenSpec as the single source of truth\n- Add for existing @coji/durably-react behavior\n- Update HITL change set with React spec deltas and job filter notes\n\n## Test plan\n\n- [x] Specification 'react' is valid\n- [x] Change 'add-human-in-the-loop' is valid\n
Summary by CodeRabbit
New Features
useHumanWaitshook for browser and server modesDocumentation
✏️ Tip: You can customize this high-level summary in your review settings.