Skip to content

task: default to JSON in and JSON out #62

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Jun 18, 2025
Merged

task: default to JSON in and JSON out #62

merged 11 commits into from
Jun 18, 2025

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Jun 18, 2025

This PR implements the breaking change to default the Hooks webhook server to JSON format for both input and output, replacing the previous plain text defaults.

Changes Made

Configuration Options

  • Added format and default_format configuration options that default to :json
  • Added environment variable support via HOOKS_FORMAT and HOOKS_DEFAULT_FORMAT
  • Maintains backward compatibility through configuration overrides

API Updates

  • Updated main API class to use configurable format instead of hardcoded :txt
  • Modified health, version, and catchall endpoints to work with JSON default format
  • Ensured all endpoint classes properly handle JSON serialization

Authentication Responses

Authentication failures now return structured JSON responses instead of plain text:

Before:

HTTP/1.1 401 Unauthorized
content-type: text/plain

authentication failed

After:

HTTP/1.1 401 Unauthorized
content-type: application/json

{
  "error": "authentication_failed",
  "message": "authentication failed", 
  "request_id": "uuid-here"
}

Test Updates

  • Updated 21 acceptance tests to expect JSON authentication failure responses
  • Added helper method expect_json_auth_failure for consistent JSON response validation
  • Updated handler error test to work with JSON-encoded string responses
  • Added unit tests for new configuration options

Documentation

  • Added documentation for format and default_format configuration options
  • Included examples and default values in configuration guide

Breaking Changes

  • Authentication failures now return JSON instead of plain text (intended behavior)
  • Handler error! calls with strings now return JSON-encoded strings for consistency
  • Default API format changed from :txt to :json

Testing

  • ✅ All unit tests pass (591 examples, 100% coverage)
  • ✅ All integration tests pass (10 examples)
  • ✅ All acceptance tests pass (52 examples)
  • ✅ Linting passes with no offenses

Sample Usage

The server now defaults to JSON but can be configured:

# hooks.yaml
format: json          # defaults to json if unset
default_format: json  # defaults to json if unset

Or via environment variables:

export HOOKS_FORMAT=txt
export HOOKS_DEFAULT_FORMAT=txt

This change delivers on the "JSON in and JSON out" philosophy while maintaining configurability for edge cases.

Fixes #61.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

@Copilot Copilot AI changed the title [WIP] task: default to JSON in and JSON out task: default to JSON in and JSON out Jun 18, 2025
@Copilot Copilot AI requested a review from GrantBirki June 18, 2025 06:03
@GrantBirki GrantBirki marked this pull request as ready for review June 18, 2025 06:24
@Copilot Copilot AI review requested due to automatic review settings June 18, 2025 06:24
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR changes the default request/response format of the Hooks webhook server from plain text to JSON, adds configuration and environment variable support for formats, and updates all endpoints, tests, and documentation accordingly.

  • Introduces format and default_format config options (default to :json) with HOOKS_FORMAT/HOOKS_DEFAULT_FORMAT env var support.
  • Updates the main API and built-in endpoints (version, health, catchall) to use configurable JSON formatting instead of hardcoded text.
  • Refactors tests with a new expect_json_auth_failure helper, updates acceptance tests for JSON responses, and documents the new options.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
spec/unit/lib/hooks/core/config_loader_spec.rb Added tests for format/default_format defaults and env-var parsing
spec/acceptance/acceptance_tests.rb New expect_json_auth_failure helper; updated tests to use JSON output
lib/hooks/core/config_loader.rb Added format/default_format defaults and env-var conversion
lib/hooks/app/endpoints/version.rb Configured JSON content types and default format
lib/hooks/app/endpoints/health.rb Configured JSON content types and default format
lib/hooks/app/endpoints/catchall.rb Configured JSON content types/default format; removed manual .to_json
lib/hooks/app/api.rb Made format and default_format configurable via settings
docs/configuration.md Documented format and default_format options
.bundle/config CI-specific Bundler settings updated
Comments suppressed due to low confidence (2)

.bundle/config:3

  • This .bundle/config contains an environment-specific absolute path. Consider removing it from version control or using a relative path to avoid CI/local environment conflicts.
BUNDLE_PATH: "/home/runner/work/hooks/hooks/vendor/bundle"

spec/unit/lib/hooks/core/config_loader_spec.rb:203

  • This test sets ENV["HOOKS_FORMAT"] and ENV["HOOKS_DEFAULT_FORMAT"] without cleaning them up. Consider using an around hook or resetting these env vars after the example to prevent side effects on other tests.
      it "converts format environment variables to symbols" do

@GrantBirki
Copy link
Member

@copilot thank you for these changes! They look excellent and do exactly what I was after. One more thing though, please update the docs/handler_plugins.md docs to reflect these changes and inform users that when returning responses from the handler plugins, they should always return a hash (by default) and that has will be automatically converted to JSON and it will have the correct headers set via Grape.

Copy link
Contributor Author

Copilot AI commented Jun 18, 2025

@copilot thank you for these changes! They look excellent and do exactly what I was after. One more thing though, please update the docs/handler_plugins.md docs to reflect these changes and inform u...

The docs/handler_plugins.md documentation has been updated to reflect the JSON-first behavior. The documentation now emphasizes that handler plugins should return a hash by default, which is automatically converted to JSON with correct headers set via Grape. Added comprehensive examples and a dedicated "Default JSON Format" section explaining the new behavior.

Commit: 24217a4

Copilot finished work on behalf of GrantBirki June 18, 2025 06:39
@GrantBirki GrantBirki merged commit 73cc4ed into main Jun 18, 2025
22 checks passed
@GrantBirki GrantBirki deleted the copilot/fix-61 branch June 18, 2025 17:46
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.

task: default to JSON in and JSON out
2 participants