feat: an error in an after hook returns the default value

[kripa-sindhu-007]

This PR

• adds Requirement 4.4.8: an error in an after hook returns the default value

Related Issues

Fixes #427

Notes

I added a new point rather than widening 4.4.7, but that is the part worth deciding rather than assuming.

Widening 4.4.7 to say "before or after" would be the smaller diff, and it would match 4.4.5 and 4.4.6, which already cover both stages in one line. What put me off it is that it changes an existing requirement's meaning in place. SDKs name their conformance tests after the requirement, so every existing 4.4.7 test would silently stop covering the whole requirement, with nothing to tell the maintainers. A new requirement_4_4_8 gives each SDK an id to implement and track against.

It also gives the rationale somewhere to live. Before hooks are abnormal execution because they can mutate the evaluation context; after hooks are abnormal execution because they can reject a resolution they consider invalid. Those are different arguments and they read poorly merged into one sentence.

The third paragraph is for your point about error and finally. It states that their failures are contained by 4.4.4 and 4.4.3 and do not change the value returned to the application author, so the new requirement does not read as widening those too.

Happy to switch to widening 4.4.7 instead if the TSC would rather not add a number.

For cross-SDK context, on go-sdk#566 I checked how this is implemented today: JS server and web, Java and Python all treat an after-hook error as abnormal and return the default with reason ERROR and code GENERAL. Rust builds the error details and then discards them, returning the original Ok, which looks unintentional. Go is the one this came from.

Follow-up Tasks

• SDKs will need a requirement_4_4_8 conformance test. go-sdk already behaves this way, so for Go it is a test rather than a behaviour change.
• The Rust SDK returns the original Ok after building the error details. If this is agreed, that is worth its own issue against open-feature/rust-sdk. I can file it with the details.

How to test

make parse
git diff specification.json

4.4.7 stays byte for byte unchanged and requirement_4_4_8 is appended before requirement_4_5_1.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 2355e181-91ee-417b-b78c-1cbf38f9fbca

📥 Commits

Reviewing files that changed from the base of the PR and between dd23583 and 610d53f.

📒 Files selected for processing (2)

• specification.json
• specification/sections/04-hooks.md

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.

---

📝 Walkthrough

Walkthrough

The Hooks specification adds Requirement 4.4.8. Errors in after hooks now require returning the default value. The specification also clarifies the handling of errors in error and finally hooks.

Changes

Hooks specification

Layer / File(s)	Summary
After-hook error contract specification/sections/04-hooks.md, specification.json	Adds Requirement 4.4.8, which requires returning the default value when an after hook errors. The text identifies after-hook errors as abnormal execution and preserves existing handling for error and finally hooks.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 610d5

The Hooks specification now defines that after-hook errors return the default value, consistently in both published representations. No merge-blocking risk remains.

Suggested reviewers: toddbaert, kinyoklion

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the primary change: errors in after hooks return the default value.
Description check	✅ Passed	The description directly explains the new requirement, its rationale, linked issue, and intended scope.
Linked Issues check	✅ Passed	The changes satisfy issue #427 by explicitly requiring after-hook errors to return the default value and by clarifying that error and finally hook failures do not change the returned value.
Out of Scope Changes check	✅ Passed	The changes are limited to the specification files and directly support the linked issue. No unrelated changes are present.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

---

_{Comment @coderabbitai help to get the list of available commands.}

[kripa-sindhu-007]

The json-lint failure is not from this PR.

make lint ends with markdown-link-check, and two CNCF Slack archive URLs now return 403 to unauthenticated CI:

README.md                          https://cloud-native.slack.com/archives/C0344AANLA1
specification/appendix-c/index.md  https://cloud-native.slack.com/archives/C066A48LK35

Neither file is touched here. This branch changes only specification/sections/04-hooks.md and specification.json, and both 403s reproduce locally against unmodified content, so Slack appears to have started refusing these the way it did before #419. The links were added in #232 and #347 respectively.

Everything else is green, including lint, json-synchronized, markdown-toc and DCO.

Happy to send a separate PR that swaps them for slack.cncf.io invite links, or adds them to .markdown-link-check-config.json, whichever you prefer. I have kept it out of this one so the spec change stays reviewable on its own.

[toddbaert]

The json-lint failure is not from this PR.

make lint ends with markdown-link-check, and two CNCF Slack archive URLs now return 403 to unauthenticated CI:

README.md                          https://cloud-native.slack.com/archives/C0344AANLA1
specification/appendix-c/index.md  https://cloud-native.slack.com/archives/C066A48LK35

Neither file is touched here. This branch changes only specification/sections/04-hooks.md and specification.json, and both 403s reproduce locally against unmodified content, so Slack appears to have started refusing these the way it did before #419. The links were added in #232 and #347 respectively.

Everything else is green, including lint, json-synchronized, markdown-toc and DCO.

Happy to send a separate PR that swaps them for slack.cncf.io invite links, or adds them to .markdown-link-check-config.json, whichever you prefer. I have kept it out of this one so the spec change stays reviewable on its own.

If you can find some other links that work, that would be great, and we can rebase on that.

[kripa-sindhu-007]

@toddbaert #429 is up for the links.

cloud-native.slack.com 403s for unauthenticated clients at the host level, not just on those two archive URLs, so there was no channel link to swap in:

403  https://cloud-native.slack.com/archives/C0344AANLA1
403  https://cloud-native.slack.com/archives/C066A48LK35
403  https://cloud-native.slack.com/
200  https://slack.cncf.io/

The README badge now points at slack.cncf.io, which is where #419 moved appendix-c's other link in August, and openfeature.dev's community page uses it for joining as well. In appendix-c the dead link only carried the channel name,
and that sentence already links the workspace right after it, so the name is plain code now rather than two links to the same URL.

json-lint is green on #429 along with the rest of the checks.

I will rebase this one on main once that lands.