Skip to content

fix(webhook): document response_mode as required; drop dead default#66

Merged
Chosen9115 merged 1 commit into
mainfrom
fix/webhook-response-mode-required
Jun 10, 2026
Merged

fix(webhook): document response_mode as required; drop dead default#66
Chosen9115 merged 1 commit into
mainfrom
fix/webhook-response-mode-required

Conversation

@Chosen9115

Copy link
Copy Markdown
Owner

Summary

Makes the webhook response_mode contract explicit and divergence-free (pairs with Chosen9115/opensop-cli#16).

  • validate_webhook! already requires response_mode; this documents it as required (no default) in SPEC.md (webhook section + capability matrix) and removes the now-dead || "callback" fallback in webhook.rb.
  • The CLI local engine now also requires it — no more silent local-sync / runtime-callback split.

Self-rating

  • correctness 9 · simplicity 9 · test coverage 9 · naming 9 · performance risk 10 · security risk 9 · process-model fidelity 9
  • Lowest tie; improvement applied: added the negative spec cases (missing + invalid response_mode) so the requirement is regression-locked.

Test plan

  • bundle exec rspec spec/services/opensop/definition_parser_spec.rb77 examples, 0 failures.

🤖 Generated with Claude Code

The parser already requires webhook.response_mode (validate_webhook!
checks method/url/response_mode). Make that explicit and remove the
silent fallback so server and CLI agree (no default divergence).

- SPEC.md: webhook section + matrix mark response_mode REQUIRED (no default)
- webhook.rb: remove the dead `|| "callback"` fallback
- parser spec: add cases — missing response_mode rejected, invalid value
  rejected, all-required-fields accepted (77 examples, 0 failures)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Chosen9115 Chosen9115 merged commit 53ceb7b into main Jun 10, 2026
@Chosen9115 Chosen9115 deleted the fix/webhook-response-mode-required branch June 10, 2026 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant