feat: auto fix api v2 version to support session token

[ravverma]

Summary

Adds autofixSuggestionsV2 endpoint that reads the promise token from the promise_token cookie instead of creating it from the Authorization header via IMS.

• New endpoint: PATCH /sites/:siteId/opportunities/:opportunityId/suggestions/auto-fix-v2
• Promise token: Read from promise_token cookie; returns 400 if missing or empty
• Deprecation: Original autofixSuggestions and /suggestions/auto-fix marked deprecated

Files changed

File	Change
src/controllers/suggestions.js	Added autofixSuggestionsV2, deprecated autofixSuggestions
src/routes/index.js	Added route for auto-fix-v2
docs/openapi/api.yaml	Added path for auto-fix-v2
docs/openapi/site-opportunities.yaml	Deprecated v1 spec, added v2 spec
test/controllers/suggestions.test.js	20 tests for autofixSuggestionsV2
test/routes/index.test.js	Added auto-fix-v2 to expected routes

Test plan

• Cookie validation: missing, empty, absent → 400
• Happy path: grouped (alt-text) and ungrouped (broken-backlinks)
• Validation: siteId, opportunityId, suggestionIds, variations, action
• Error paths: 404 (site/opportunity), 403 (access), 400 (handler disabled)
• Partial success: suggestion not found, not in NEW status
• No valid suggestions: bulkUpdateStatus not called

[github-actions]

This PR will trigger a minor release when merged.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[bhellema]

Let's discuss api versioning in #1887.

[solaris007]

Hey @ravverma,

Thanks for working on decoupling the promise token from server-side IMS. The direction is right - removing the IMS dependency from the API service and letting the client manage the token lifecycle simplifies this flow. Tests are thorough (20 cases covering all the major paths), and the OpenAPI spec for v2 is well-documented.

That said, we found several issues that need addressing before this can merge.

Critical

1. State corruption: bulkUpdateStatus runs before promise token validation

In autofixSuggestionsV2, the x-promise-token header is checked after Suggestion.bulkUpdateStatus() has already transitioned suggestions to IN_PROGRESS. If the header is missing or empty, the function returns 400 - but suggestions are already mutated with no SQS job queued. They're stuck in IN_PROGRESS permanently, since both v1 and v2 filter for NEW status only.

The tests confirm this: the three missing-token test cases set up bulkUpdateStatus mocks and expect them to be called, meaning the test suite validates the broken ordering rather than catching it.

Ironically, PR #1887's description explicitly highlights "Early exit - first thing checked" as a key v2 advantage. This PR should follow that same pattern. Move the header check to right after input validation, before any database operations:

const autofixSuggestionsV2 = async (context) => {
  // ... validate siteId, opportunityId
  
  // Validate promise token BEFORE any side effects
  const promiseToken = context.request?.headers?.get?.('x-promise-token');
  if (!hasText(promiseToken)) {
    return badRequest('x-promise-token header is required');
  }
  
  // ... then proceed with site lookup, access check, bulkUpdateStatus, SQS

2. action: "assess" silently broken in v2

The v1 function has an isAssessAction code path that skips bulkUpdateStatus when action === 'assess' - the assess action is a read-only fixability check that should not transition suggestion status. The v2 function drops this entirely and always calls bulkUpdateStatus.

The OpenAPI spec for v2 still documents action: "assess" and says "the suggestion status transition to IN_PROGRESS is skipped" - but the code does the opposite. Clients migrating from v1 to v2 with action: "assess" would get their suggestions incorrectly moved to IN_PROGRESS. No test covers this path for v2.

3. API versioning pattern - @bhellema's feedback

The -v2 suffix (/suggestions/auto-fix-v2) conflicts with the codebase's existing /v2/ path prefix convention (see the LLMO/brands endpoints in routes). More importantly, @bhellema already flagged this on both PRs - you acknowledged it on #1887 and closed that PR with intent to use a backward-compatible approach instead, but #1863 still uses the same pattern.

Consider making this backward-compatible instead: have the single /auto-fix endpoint detect whether x-promise-token is present, and fall back to the IMS flow when it's not. This avoids the versioning question entirely and provides a natural migration path.

Important

4. ~170 lines of duplicated logic

autofixSuggestionsV2 is a near-verbatim copy of autofixSuggestions. The only intended difference is the token source. All other logic - parameter validation, site lookup, access control, opportunity lookup, handler check, suggestion filtering, grouping, SQS dispatch, response building - is identical.

This already caused issues: the isAssessAction branch was missed in the copy, and the url: requestUrl parameter was silently dropped from the ungrouped sendAutofixMessage call (intentional or not?). Extract the shared logic into a private function that accepts the resolved promise token:

async function autofixSuggestionsCore(context, promiseTokenResponse) {
  // all shared validation, lookup, grouping, SQS logic
}

const autofixSuggestions = async (ctx) => {
  const token = await getIMSPromiseToken(ctx);
  return autofixSuggestionsCore(ctx, token);
};

const autofixSuggestionsV2 = async (ctx) => {
  const token = ctx.request?.headers?.get?.('x-promise-token');
  if (!hasText(token)) return badRequest('x-promise-token header is required');
  return autofixSuggestionsCore(ctx, { promise_token: token });
};

5. Promise token trust boundary shift

In v1, the server creates the promise token with the correct IMS emitter credentials, guaranteeing scope alignment. In v2, the client provides the token - the API service only checks it's non-empty, with no format or content validation.

The good news: IMS validates at exchange time in the autofix worker, so a forged/expired token will fail there. But the worker failure happens after suggestions are already IN_PROGRESS (compounding issue #1).

Consider: (a) adding basic format validation (e.g., max length bound), and (b) documenting the trust assumption that the calling client uses the correct emitter credentials. Also document expected token lifetime - if the IMS promise definition has a short TTL, the additional latency of client-create -> API call -> SQS -> worker could cause expiry.

6. PR description says "cookie", code reads a header

The description says "reads the promise token from the promise_token cookie" but the code reads from the x-promise-token header. The OpenAPI spec correctly documents it as a header.

Minor

• Inconsistent header access pattern: v1 uses context.pathInfo?.headers?.['x-promise-token'], v2 uses context.request?.headers?.get?.('x-promise-token'). Pick one and use it consistently.
• OpenAPI doesn't clarify dual-auth: The caller needs both an IMS Authorization header (for access control) AND the x-promise-token header (for the worker). The spec should clarify these are independent requirements.
• No deprecation timeline: v1 is marked deprecated but there's no sunset date or migration plan.