Skip to content

Add NackConflict for 409; clarify /catalog/push auth header#169

Merged
anandp504 merged 2 commits into
core-v2.0.0-lts-rc3from
fix/pr168-nack-conflict-and-push-auth-clarification
May 22, 2026
Merged

Add NackConflict for 409; clarify /catalog/push auth header#169
anandp504 merged 2 commits into
core-v2.0.0-lts-rc3from
fix/pr168-nack-conflict-and-push-auth-clarification

Conversation

@anandp504
Copy link
Copy Markdown
Collaborator

@anandp504 anandp504 commented May 22, 2026

Summary

Addresses two review comments from PR #168:

  • NackConflict for 409 on /catalog/subscription POST — The 409 response previously mapped to NackBadRequest, which describes structurally invalid requests. A duplicate subscription (identical networkIds+schemaTypes already active) is a state conflict, not a malformed request. This PR introduces a dedicated NackConflict schema and response component, consistent with the existing per-status-code pattern (NackForbidden, NackNotFound, etc.), and updates the 409 reference and description text accordingly.

  • /catalog/push uses AuthorizationHeader (correct) — Added an explicit "Authentication note" to the /catalog/push description clarifying why it uses AuthorizationHeader rather than CallbackAuthorizationHeader. The CS initiates the push autonomously when a subscription matches — there is no prior DS request whose signature could be chained via request-signature. This is analogous to a PN-initiated notification, which is exactly the use case AuthorizationHeader covers.

…catalog/push auth

- Introduce NackConflict schema and response component for HTTP 409,
  replacing the incorrect NackBadRequest mapping on POST /catalog/subscription.
  A duplicate subscription is a state conflict, not a malformed request.

- Add authentication note to /catalog/push description explaining why it
  correctly uses AuthorizationHeader rather than CallbackAuthorizationHeader:
  the CS initiates the push autonomously with no prior DS request to chain.

Addresses review comments on PR #168.
@nirmay
Copy link
Copy Markdown
Collaborator

nirmay commented May 22, 2026

Review Note — 409 overload: AckNoCallback vs NackConflict

After this PR, HTTP 409 carries two distinct meanings across the API:

Endpoint type 409 maps to Meaning
All 11 async action endpoints + /catalog/publish AckNoCallback Accepted and authenticated, but no callback is coming
POST /catalog/subscription NackConflict Duplicate subscription — state conflict

Why this is pragmatically acceptable

The two uses never co-exist on the same endpoint — AckNoCallback only applies to async request-callback endpoints; NackConflict only applies to a synchronous CRUD endpoint where AckNoCallback semantics can't arise. OpenAPI-generated SDKs are unaffected because per-operation response typing already differentiates them. The place this genuinely causes confusion is generic middleware, gateways, and observability tooling that reads status codes without parsing the body.

Why not use a different unused code?

NackConflict should stay on 409 — HTTP 409 literally means "Conflict" and is the correct code here. The one that shouldn't be on 409 is AckNoCallback — "accepted, no result" is semantically closer to 202 Accepted, which is currently unused in the spec. However, moving AckNoCallback off 409 would be a breaking change across every existing Beckn implementation and is out of scope for this PR.

One gap this PR leaves open

The global response table in Communication_Protocol.md (NFH-013, §3 line 142) still reads:

| 409 | AckNoCallback | Request received and authenticated, but the PN has determined that no callback will follow |

That statement is now universally false. The table needs a NackConflict row and a note distinguishing the two 409 uses by endpoint type, otherwise implementers reading NFH-013 as their primary reference will be surprised by the subscription endpoint's behaviour.

Suggested follow-up (not a blocker for this PR): Update the NFH-013 response table to document both 409 uses. If a v2.1 revision window opens, consider migrating AckNoCallback to 202.

@anandp504 @ravi-prakash-v

@ravi-prakash-v
Copy link
Copy Markdown
Contributor

I think we can map AckNoCallback to a 403 Forbidden status and reserve 409 for NackConflict. The former signifies a Provider Node's reason-free refusal to call back the Consumer Node. Right now, I don't believe this use case is being utilized anywhere so we have a window to push that change. @anandp504 @nirmay what do you think?

@anandp504
Copy link
Copy Markdown
Collaborator Author

anandp504 commented May 22, 2026

@ravi-prakash-v I am not sure if I understand it correctly. AckNoCallback essentially signifies that the request was accepted. It should not be mapped to 403 Forbidden. 409 arises only in the case of conflict and hence I have created NackConflict.

@ravi-prakash-v
Copy link
Copy Markdown
Contributor

Makes sense @anandp504. Can you change it to 202 Accepted and push?

409 Conflict is now used exclusively by NackConflict (duplicate
subscription). AckNoCallback represents an accepted-but-no-callback
outcome, which aligns with HTTP 202 Accepted.

Updates all 11 endpoint response blocks, the standard response families
table, and description references in the Error, AckNoCallback, and
/catalog/push schemas.
Copy link
Copy Markdown
Contributor

@ravi-prakash-v ravi-prakash-v left a comment

Choose a reason for hiding this comment

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

Looks good @anandp504. Approved. Please go ahead and merge. Thanks!

@anandp504 anandp504 merged commit bdf5285 into core-v2.0.0-lts-rc3 May 22, 2026
@anandp504 anandp504 deleted the fix/pr168-nack-conflict-and-push-auth-clarification branch May 22, 2026 13:43
ravi-prakash-v added a commit that referenced this pull request May 23, 2026
…NFH-013

Anand's earlier commit moved AckNoCallback from HTTP 409 to HTTP 202 in
beckn.yaml but didn't update the corresponding docs. This commit syncs:

- docs/API.md: AckNoCallback listed at 202 in the standard response
  families.
- docs/Communication_Protocol.md (NFH-013):
  - Response families table updated 409 -> 202 for AckNoCallback.
  - CON-013-09 conformance row updated 409 -> 202.
  - FAQ entry contrasting AckNoCallback vs NackDiscretionary updated
    to 202.
  - Added a NackConflict (409) row to the response families table so it
    now reflects the full response surface introduced by PR #169.
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.

3 participants