Skip to content

Fix OpenAPI docs#1375

Open
skyfallwastaken wants to merge 4 commits into
mainfrom
codex/fix-openapi-docs
Open

Fix OpenAPI docs#1375
skyfallwastaken wants to merge 4 commits into
mainfrom
codex/fix-openapi-docs

Conversation

@skyfallwastaken
Copy link
Copy Markdown
Member

Summary of the problem

The generated OpenAPI docs included browser-only and Slack webhook routes, and several API endpoints had incorrect auth/security metadata.

Describe your changes

  • Removed non-API browser and Slack-command paths from rswag output.
  • Fixed endpoints that incorrectly documented Bearer and API-key auth as jointly required instead of alternatives.
  • Removed auth requirements from public endpoints.
  • Added missing public API specs for currently hacking, banned user counts, and project badge endpoints.
  • Regenerated swagger/v1/swagger.yaml from the updated rswag specs.

Validation:

  • docker compose exec web env RAILS_ENV=test bin/rails rswag:specs:swaggerize (114 examples, 0 failures)
  • Subagent also ran focused RSpec/RuboCop and checked the generated YAML for non-API paths and conjunctive auth entries.

Screenshots / Media

Not included; this changes generated API documentation.

Copilot AI review requested due to automatic review settings May 28, 2026 18:22
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 28, 2026

Greptile Summary

This PR corrects the rswag-generated OpenAPI docs by fixing the most common mistake in OpenAPI 3.0 security declarations (conjunctive AND vs. disjunctive OR), removing non-API browser and Slack-command paths, and adding missing public endpoint specs.

  • Security format fix: security [ Bearer: [], ApiKeyAuth: [] ] (single object = both required) is replaced with security [ { Bearer: [] }, { ApiKeyAuth: [] } ] (array of objects = either accepted) across all WakaTime-compat and admin endpoints.
  • Route cleanup: Browser-only paths (/my/heartbeats/export, /deletion, project-mapping routes, etc.) and the Slack command route are removed from the OpenAPI output, scoping it to genuine API consumers.
  • New public specs: public_spec.rb adds documentation for /api/v1/currently_hacking, /api/v1/banned_users/counts, and the badge redirect endpoint.

Confidence Score: 4/5

Safe to merge; all changes are OpenAPI documentation only — no application logic is touched.

The email and Slack UID lookup endpoints retain 'Requires STATS_API_KEY' in their descriptions but now only advertise Bearer auth in the security block, while the equivalent admin stats endpoint correctly lists both Bearer and ApiKeyAuth as alternatives. This inconsistency will mislead API consumers who rely on the generated docs to know which credentials are accepted for those two endpoints.

spec/requests/api/v1/users_spec.rb — verify whether the email/Slack lookup endpoints truly accept only Bearer or also accept STATS_API_KEY (ApiKeyAuth), and align the security declaration with the description.

Important Files Changed

Filename Overview
spec/requests/api/v1/users_spec.rb Removes auth from public user-data endpoints correctly, but email/Slack lookup endpoints still say 'Requires STATS_API_KEY' while security is narrowed to only Bearer — mismatched with the stats pattern.
spec/requests/api/v1/public_spec.rb New file adding public API specs for currently_hacking, banned_users/counts, and the badge redirect endpoint; schemas look correct.
spec/swagger_helper.rb Adds BasicApiKey (HTTP Basic) security scheme to support WakaTime-style API key auth; straightforward addition.
swagger/v1/swagger.yaml Regenerated YAML correctly reflects all spec changes; user lookup endpoints still show only Bearer security while descriptions reference STATS_API_KEY.
spec/requests/api/v1/my_spec.rb Removes non-API browser routes from swagger output and switches heartbeat endpoints to the new BasicApiKey alternative scheme; cleanly scoped.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[API Request] --> B{Endpoint type?}
    B -->|Public| C[No auth required]
    B -->|Authenticated /api/v1/| D{Auth method?}
    B -->|WakaTime-compat| E{Auth method?}
    B -->|Admin endpoints| F{Auth method?}
    D -->|Bearer token| G[Accepted]
    D -->|OR BasicApiKey| G
    E -->|Bearer token| G
    E -->|OR ApiKeyAuth| G
    F -->|Bearer token| G
    F -->|OR ApiKeyAuth| G
Loading
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
spec/requests/api/v1/users_spec.rb:7-8
Security scheme inconsistency on admin lookup endpoints: both `/api/v1/users/{email}/lookup` and `/api/v1/users/{slack_uid}/lookup` still say "Requires STATS_API_KEY" in their descriptions, but their security was narrowed to only `Bearer: []`. Compare with `/api/v1/stats`, which correctly documents both alternatives as `{ Bearer: [] }, { ApiKeyAuth: [] }`. If STATS_API_KEY is the intended auth method for these endpoints, consumers reading the OpenAPI docs will incorrectly believe only a Bearer token is accepted.

```suggestion
      description 'Find a user ID by their email address. Useful for integrations that need to map emails to Hackatime users. Requires STATS_API_KEY.'
      security [ { Bearer: [] }, { ApiKeyAuth: [] } ]
```

### Issue 2 of 2
spec/requests/api/v1/users_spec.rb:49-50
Same inconsistency for the Slack UID lookup — description says "Requires STATS_API_KEY" but security is `Bearer`-only, unlike the stats endpoint pattern.

```suggestion
      description 'Find a user ID by their Slack User ID. Requires STATS_API_KEY.'
      security [ { Bearer: [] }, { ApiKeyAuth: [] } ]
```

Reviews (1): Last reviewed commit: "Fix OpenAPI docs" | Re-trigger Greptile

Comment thread spec/requests/api/v1/users_spec.rb
Comment thread spec/requests/api/v1/users_spec.rb
Copy link
Copy Markdown
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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@skyfallwastaken skyfallwastaken enabled auto-merge (squash) May 28, 2026 18:33
@skyfallwastaken skyfallwastaken disabled auto-merge May 28, 2026 18:33
- Revert incorrect dual-auth on /api/v1/users/lookup_* endpoints; the
  controller disables ?api_key= query, so Bearer-only security is correct.
- Remove duplicate path declarations between users_spec and stats_spec
  (trust_factor, projects, project/:name, projects/details,
  heartbeats/spans) -- kept in stats_spec since controller is StatsController.
- Remove duplicate /user/info and /user/heartbeats path declarations from
  admin_users_spec (kept in admin_user_utils_spec).
- admin_user_utils_spec: fix /user/stats schema (date -> start_date/end_date);
  add trust_level and notes to /user/convict request body; delete redundant
  third response(200) block on /heartbeats/by_user_agent_segment; remove
  nullable: true from lineno/cursorpos (controller coerces to 0).
- admin_timeline_spec: delete duplicated dead schema declaration in
  /timeline response block.
- compatibility_spec: add accepted heartbeat keys to request body schema
  (created_at, dependencies, editor, line_additions, line_deletions,
  machine, operating_system, project_root_count, user_agent, plugin).
- internal_spec: remove nullable: true from owner_email/key_name (controller
  uses compact_blank, so absent keys are omitted, not null).
- summary_spec: document additional accepted query params (from, to, range).

Skipped (flagged for follow-up):
- summary_controller.rb:27 uses params[:user] vs params[:user_id] -- looks
  like a controller bug, not a doc bug.
- internal_spec.rb:52 test assertion may contradict controller behavior.
- compatibility_spec.rb Basic auth scheme would require defining a new
  scheme in swagger_helper.rb.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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