WI-001: #15 No exported setter for the package-level ProblemConfig

[rluders]

Work item WI-001 of work package WP-001 (Cycle 7 pilot: httpsuite #15).

From GitHub issue #15 (#15), copied by the operator because the issue is not labelled for intake:

GetProblemTypeURL reads from a package-level defaultProblemConfig (built once via NewProblemConfig()), but there's no exported way to replace it — only DefaultProblemConfig() (read-only copy) and the per-call ParseOptions.Problems override on ParseRequest.

Consumers that want a custom BaseURL (to emit real RFC 9457 type URIs instead of bare words like "unauthorized") or additional ErrorTypePaths globally have no way to configure this once at startup — they'd have to pass ParseOptions.Problems on every single ParseRequest call, and there's no equivalent hook at all for the direct helpers (NewProblemDetails, ProblemBadRequest, etc.), which always resolve via the package-level default.

Found while auditing usage across a ~150k LOC service (GolgiMed): 60+ call sites construct ProblemDetails with a bare-word type ("unauthorized", "forbidden", ...) instead of a proper URI, because there's no way to register a base URL once and have every helper pick it up.

Requested: SetProblemConfig(cfg ProblemConfig) (mirroring SetValidator/ClearValidator's pattern) so an app can configure BaseURL + ErrorTypePaths once at composition root.

Acceptance criteria:

1. An exported SetProblemConfig(cfg ProblemConfig) replaces the package default used by GetProblemTypeURL, DefaultProblemConfig and the helpers that resolve problem types; a per-call ParseOptions.Problems still overrides it.
2. The stored config is a copy: changing the caller's ErrorTypePaths map afterwards does not change the default.
3. Setting and reading the default are safe for concurrent use (go test -race stays clean).
4. Restoring the built-in default is possible and documented, following the SetValidator/ClearValidator pattern.
5. Tests cover: set then GetProblemTypeURL, copy isolation, ParseOptions override still wins, restore; tests leave the global default as they found it.
6. Doc comments and a short README note. No other public API change.
7. go vet ./..., go test -race ./... and the validation/playground tests pass.

Implemented by the Weave Autoloop, attempt 1, execution exe_c3a1e6bcf0b4.

Changed files:

• README.md
• problem_config.go
• problem_config_test.go

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.28%. Comparing base (9b55b5e) to head (8600f44).
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #24      +/-   ##
==========================================
+ Coverage   83.60%   84.28%   +0.68%     
==========================================
  Files          14       14              
  Lines         500      509       +9     
==========================================
+ Hits          418      429      +11     
+ Misses         63       62       -1     
+ Partials       19       18       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[rluders]

Weave local checks: passed at 8600f44c31 — vet passed; test-race passed; playground passed. These ran locally in the item's worktree; they are not hosted CI.

[rluders]

Weave review: changes_requested (execution exe_914ca42ed0a0)

• Reviewer by agent go-reviewer: changes_requested — Review of WI-001 (No exported setter for the package-level ProblemConfig #15), attempt 1: SetProblemConfig

The implementation looks correct on inspection. The step output doesn't prove acceptance criterion 7, because it never ran the race detector.

#	Criterion	Implementation	Test
1	SetProblemConfig replaces the default; a per-call override still wins	problem_config.go:45-50. GetProblemTypeURL (:95-99), DefaultProblemConfig (:32-36) and mergeProblemConfig (:52) all read the same guarded variable.	TestSetProblemConfig (problem_config_test.go:14) covers GetProblemTypeURL, DefaultProblemConfig and the NewBadRequestProblem helper. TestParseOptionsProblemsOverrideSetProblemConfig (:45) covers the override.
2	The stored config is a copy	problem_config.go:46, via Clone()	TestSetProblemConfigCopiesInput (:33)
3	Concurrency-safe	The RWMutex at problem_config.go:9, taken at :33, :47 and :96. No other code touches the variable unguarded, according to grep.	TestSetProblemConfigConcurrent (:76)
4	Restore is possible and documented	SetProblemConfig(NewProblemConfig()) is documented at problem_config.go:43-44. The README shows it at :302-303.	TestSetProblemConfigRestore (:62)
5	Tests leave the global as they found it	Not applicable	The restoreProblemConfig helper (:8-12) resets the default on entry and on cleanup.
6	Doc comment and README note	problem_config.go:38-44 and README.md:292-304. No other public API was added.	Not applicable
7	go vet, go test -race, validation and playground tests	The step output shows go vet ./... and go test -count=1 ./... both exiting 0.	Gap: see below.

Defect: criterion 3 and 7 are unverified. report.json:9 ran go test -count=1 ./... with no -race. The work item requires go test -race ./... to stay clean. TestSetProblemConfigConcurrent only proves anything under the race detector, since it has no assertio…

[rluders]

Weave review: changes_requested (execution exe_a4c72d82673b)

• Reviewer by agent go-reviewer: changes_requested — Every criterion is implemented and tested in the code, but the step's evidence doesn't show the -race run, so I'm requesting changes.

Criteria, with implementation and proof

1. Exported setter replaces the default.
   • SetProblemConfig is at problem_config.go:45. GetProblemTypeURL (:95) and DefaultProblemConfig (:32) both read the guarded default.
   • The helpers resolve through GetProblemTypeURL, e.g. problem_helpers.go:8.
   • mergeProblemConfig (:52) starts from the default, so a per-call ParseOptions.Problems still overrides it.
   • Tests: TestSetProblemConfig (problem_config_test.go:14) covers GetProblemTypeURL, DefaultProblemConfig and NewBadRequestProblem. TestParseOptionsProblemsOverrideSetProblemConfig (:45) covers the override.
   • That override test calls mergeProblemConfig directly, not ParseRequest. I accept that, since request_internal.go:28 is the only call site.
2. Stored config is a copy. problem_config.go:46 clones before storing. TestSetProblemConfigCopiesInput (problem_config_test.go:33) proves it.
3. Concurrency safety. An RWMutex (problem_config.go:9) guards every read and write. TestSetProblemConfigConcurrent (problem_config_test.go:76) exercises it, but only a -race run can prove it (see the defect below).
4. Restore, documented. The doc comment at problem_config.go:43-44 says to call SetProblemConfig(NewProblemConfig()). TestSetProblemConfigRestore (problem_config_test.go:62) covers it. This is a documented idiom rather than a dedicated ClearProblemConfig, and it keeps the public API to one addition, as criterion 6 requires.
5. Tests leave the global as found. restoreProblemConfig (problem_config_test.go:8) resets the default at setup and in t.Cleanup.
6. Docs and README. The doc comment is at problem_config.go:38-44, and the README mentions the setter at lines 300 and 303. I only grepped the README, so I didn't read the surroundin…

[rluders]

Weave review: changes_requested (execution exe_5e86c91426de)

• Reviewer by agent go-reviewer: changes_requested — The code and tests are sound, but the step's report doesn't prove criteria 3 and 7. I could not run anything myself, so I checked the criteria by reading the files.

Criteria

1. Set and override: implemented and tested.
   • SetProblemConfig is at problem_config.go:45. GetProblemTypeURL (:95) and DefaultProblemConfig (:32) read the replaced default. The helpers read it through GetProblemTypeURL (problem_helpers.go:8,16, response_write.go).
   • The per-call override is mergeProblemConfig, called from request_internal.go:28.
   • Tests: TestSetProblemConfig (problem_config_test.go:14) and TestParseOptionsProblemsOverrideSetProblemConfig (:45).
   • The override test calls mergeProblemConfig directly, not ParseRequest. That is acceptable, because it is the only path ParseRequest uses.
2. Copy isolation: implemented and tested. Clone() runs before the assignment (problem_config.go:46,68). TestSetProblemConfigCopiesInput (:33) proves it.
3. Concurrency: implemented, but not proven by the run. An RWMutex guards the set (:47), the default read (:33) and GetProblemTypeURL (:96). TestSetProblemConfigConcurrent (:76) exercises it, but it only catches a race when run with -race.
4. Restore: implemented, documented and tested. SetProblemConfig(NewProblemConfig()) is documented at problem_config.go:43-44 and README.md:302-303. TestSetProblemConfigRestore (:62) covers it. The pattern differs from SetValidator/ClearValidator, which has a dedicated clear function. The work item does not require a new exported function and says "no other public API change", so I accept this.
5. Tests leave the global default as they found it. The restoreProblemConfig helper (:8) uses t.Cleanup. Set, copy, override and restore are all covered.
6. Docs: done. There is a doc comment at problem_config.go:38-44 and a README section at README.md:292-304. The only public API addition is `SetPro…

[rluders]

Weave review: approved (execution exe_6525ef79716f)

• Reviewer by agent go-reviewer: approved — I'm approving this change. Every acceptance criterion is implemented and tested, and I found no defects. I couldn't run anything myself. For criteria 3 and 7 I'm relying on the Runtime's recorded results for head 8600f44, which show go vet, go test -race and the playground tests all passing, plus hosted CI green.

1. Setter replaces the default; per-call override still wins.

   • SetProblemConfig is at problem_config.go:45. Its package default is read by GetProblemTypeURL (:95), DefaultProblemConfig (:32) and mergeProblemConfig (:52).
   • mergeProblemConfig is what ParseRequest calls at request_internal.go:28.
   • Tests: TestSetProblemConfig (problem_config_test.go:14) checks GetProblemTypeURL, DefaultProblemConfig and the NewBadRequestProblem helper. TestParseOptionsProblemsOverrideSetProblemConfig (:45) checks that the per-call config wins and that nil falls back to the global.
   • That override test calls mergeProblemConfig directly instead of going through ParseRequest. It exercises the same code, so I don't consider it a defect.

2. Stored config is a copy. cfg.Clone() runs before the lock is taken (problem_config.go:46). TestSetProblemConfigCopiesInput (:33) mutates the caller's map afterwards and checks the default is unchanged.

3. Concurrency safety.

   • An RWMutex guards the default at problem_config.go:9, and every read and write takes it.
   • TestSetProblemConfigConcurrent (:76) runs concurrent sets and reads under -race, which the Runtime recorded as passing.

4. Restore is possible and documented. The doc comment at problem_config.go:43-44 and the README (:302-303) both say to call SetProblemConfig(NewProblemConfig()). TestSetProblemConfigRestore (:62) proves it. It also confirms that an empty ErrorTypePaths yields BlankURL, which is the expected behaviour of replacing the config wholesale.

5. Tests leave the global as they found it. `restoreProblem…

• Person: approved