feature/update-spring

[tmseidel]

No description provided.

[max-california]

🤖 Agentic Code Review

Summary

This PR is not ready to merge as-is. The Spring Boot version bump itself may be fine, but the accompanying Jackson API migration introduces at least one likely compile/runtime break and removes a public parser method without showing any replacement. The Thymeleaf changes look reasonable and low risk.

Blocking issues

1. AiUsageService.exportErrors now swallows/changes I/O failure behavior and appears to use a nonstandard generator construction

File: src/main/java/org/remus/giteabot/aiusage/AiUsageService.java

Two concerns here:

• The signature changed from:

  public void exportErrors(...) throws IOException

  to:

  public void exportErrors(...)

  while the method still performs streaming I/O. That changes the contract materially: callers can no longer distinguish a partial/failed export from a successful one.

• The implementation switched from:

  new ObjectMapper().getFactory().createGenerator(outputStream)

  to:

  tools.jackson.core.ObjectWriteContext.empty().createGenerator(outputStream)

  This is a much less common construction path, and without any surrounding compatibility changes it looks brittle. If createGenerator(OutputStream) on that context is not available in the version pulled in by Boot 4.1, this won’t compile. Even if it does compile, it bypasses the mapper/factory configuration you were previously using.

Also, the method still has a try (...) block over a generator that can fail to create/flush/close, but now the checked IOException has nowhere to go. If Jackson 3 surfaces those as checked exceptions here, this is a compile error; if it wraps them, callers still lose failure visibility.

Suggested fix: keep the method contract as I/O-producing (throws IOException), and construct the generator from a shared/configured Jackson factory/mapper unless there is a strong reason not to.

---

2. AiResponseParser.parseRequestedFiles was removed, but I don’t see evidence that it’s dead code

File: src/main/java/org/remus/giteabot/agent/issueimpl/AiResponseParser.java

This PR deletes parseRequestedFiles(...) entirely. That may be fine if all call sites were removed in an earlier change, but in this diff there’s no replacement or migration note, and this class still contains file-request parsing concepts (requestFiles) in parseAiResponse.

Because the review diff only shows the deletion, this is risky for two reasons:

• If anything outside this file still calls parseRequestedFiles, this is a compile break.
• Even if there are no current call sites, this is a public method removal from a utility/parser class and may be an unintended API break.

Suggested fix: either restore the method, or explicitly show/remind in the PR that all usages were removed and the method is intentionally obsolete. If it’s intentionally deprecated functionality, deprecate first before removing unless this is an internal-only API with verified zero callers.

Non-blocking concerns

3. Be careful with broad asText → asString migrations where non-string JSON used to be tolerated

Files: multiple (CriticAgent, deployment strategies, MCP config parsing, tests, etc.)

Most of the replacements look directionally correct for Jackson 3, but semantics can differ:

• asText() historically coerces many scalar node types to text.
• asString() is often stricter / more string-oriented.

That matters in places like:

• CiActionTriggerStrategy.renderInputs
• WebhookTriggerStrategy
• StaticPreviewUrlStrategy
• WorkflowCallbackController

If these configs may contain numbers/booleans that were previously accepted and stringified, the new code may change behavior. I don’t see tests added around those coercion edge cases.

Suggestion: add a couple of focused tests for config values that are not JSON strings but are still expected to work when rendered into headers/inputs.

Looks good

• The Thymeleaf updates in form.html templates look like sensible compatibility changes for newer Spring/Thymeleaf expression handling.
• The isTextual → isString and properties() updates in JSON tree traversal generally look consistent with a Jackson 3 migration.

Recommendation

Hold this PR until the AiUsageService export path is verified/fixed and the parseRequestedFiles removal is justified or reverted.

---

Read-only agentic review by AI Git Bot