Skip to content

Conversation

@GrantBirki
Copy link
Contributor

No description provided.

Copilot AI review requested due to automatic review settings June 9, 2025 22:37
Copy link
Contributor

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 introduces a proof-of-concept setup for running acceptance tests against a newly built Hooks framework, adds core framework modules (config loading/validation, logging, request validation, lifecycle, builder, and Grape-based API), and updates project packaging and CI workflows.

  • Add acceptance Dockerfile and scripts for spinning up tests in CI
  • Implement core Hooks framework components (plugins, validators, loader, builder, API)
  • Update gemspec, root Dockerfile removal, and GitHub Actions for acceptance testing

Reviewed Changes

Copilot reviewed 32 out of 32 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
spec/acceptance/Dockerfile New slim Ruby image setup for acceptance tests
script/server Revised server launcher to support --dev and use Puma
script/acceptance Bash script to run Docker compose and acceptance RSpec
lib/hooks/plugins/request_validator/ New base and GitHubWebhooks validator implementations
lib/hooks/plugins/lifecycle.rb Base lifecycle plugin class
lib/hooks/handlers/base.rb Base handler class
lib/hooks/core/{signal_handler,logger_factory,config_*.rb,builder}.rb Core building blocks: logging, config, signals, builder
lib/hooks/app/api.rb Full Grape API class with dynamic routing and handlers
lib/hooks.rb Entry point loading plugins and builder
hooks.gemspec Expanded files and executables
handlers/test_handler.rb Example handler
docs/design.md Updated docs renaming verify_signature to request_validator
config.ru Rack configuration file
.github/workflows/acceptance.yml CI workflow for acceptance tests
Comments suppressed due to low confidence (2)

lib/hooks/plugins/request_validator/github_webhooks.rb:23

  • This new validation logic should be covered by unit tests, especially cases for missing headers, invalid signatures, and error rescues.
def self.valid?(payload:, headers:, secret:, config:)

lib/hooks/app/api.rb:196

  • The helper method is named request_validator but you're calling validate_request, which will trigger a NoMethodError. Rename one to match the other.
validate_request(raw_body, headers, endpoint_config) if endpoint_config[:request_validator]

@GrantBirki GrantBirki merged commit 2613de8 into main Jun 10, 2025
5 of 16 checks passed
@GrantBirki GrantBirki deleted the poc branch June 10, 2025 06:26
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.

2 participants