Skip to content

fix(synology-chat): resolve duplicate makeReq test conflict#32122

Closed
markfietje wants to merge 2 commits intoopenclaw:mainfrom
markfietje:fix/upstream-tui-regression
Closed

fix(synology-chat): resolve duplicate makeReq test conflict#32122
markfietje wants to merge 2 commits intoopenclaw:mainfrom
markfietje:fix/upstream-tui-regression

Conversation

@markfietje
Copy link
Contributor

@markfietje markfietje commented Mar 2, 2026

Summary

This PR fixes a duplicate declaration conflict in the Synology Chat tests where a local makeReq helper conflicted with an identical helper exported from test-http-utils.js.

Problem

After recent upstream changes, extensions/synology-chat/src/webhook-handler.test.ts fails to compile because it defines makeReq which is also imported (or intended to be shared) via test-http-utils.js.

Solution

  • Updated test-http-utils.ts to support optional headers and URL in makeReq.
  • Removed the redundant local makeReq implementation in webhook-handler.test.ts.

Verification

  • Verified: pnpm check and pnpm tsgo pass 100% green.
  • Tests: Verified with extensions/synology-chat/src/webhook-handler.test.ts (14 passed).

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 2, 2026

Greptile Summary

This PR fixes two targeted regressions introduced by upstream API drift. Both changes are minimal, well-scoped, and do not introduce new logic.

  • src/tui/gateway-chat.ts: The call to ensureExplicitGatewayAuth was passing a non-existent property auth (should be explicitAuth) and omitting urlOverrideSource. Without urlOverrideSource: "cli", the function's early-return guard (if (params.urlOverrideSource === "cli" && (explicitToken || explicitPassword)) { return; }) would never trigger, causing the function to throw an error for every CLI URL override — even when valid explicit credentials were supplied. The fix passes the correctly-named explicitAuth property and adds urlOverrideSource: "cli", restoring the intended auth-bypass behavior for credentialed CLI users.
  • extensions/synology-chat/src/webhook-handler.test.ts: The local makeReq helper is removed in favour of the identical implementation already exported from test-http-utils.ts (imported on line 4). The two implementations are functionally equivalent, so no test behaviour changes.

Confidence Score: 5/5

  • This PR is safe to merge — both changes are minimal, verified against the actual function signatures, and fix clear type/declaration errors without introducing new logic.
  • Both changes are straightforward corrections to API mismatch and duplicate declaration issues. The ensureExplicitGatewayAuth fix is verified against the actual function signature in src/gateway/call.ts (lines 87–123); the property rename and the addition of urlOverrideSource: "cli" exactly match what the function expects. The makeReq removal in the Synology test is equally safe — the shared utility is functionally identical, and the import was already present on line 4.
  • No files require special attention.

Last reviewed commit: 2c49aeb

@markfietje markfietje force-pushed the fix/upstream-tui-regression branch 2 times, most recently from c165c60 to e0cd305 Compare March 2, 2026 20:12
@markfietje markfietje changed the title fix(tui): resolve gateway auth regression and synology test conflict fix(synology-chat): resolve duplicate makeReq test conflict Mar 2, 2026
@markfietje
Copy link
Contributor Author

⚠️ Critical Dependency Note
This PR fixes a blocking regression in main (duplicate test helper conflict) that is causing CI to fail across multiple active PRs, including PR #32119.

Merging this PR is a prerequisite for unblocking the CI status of other scoped fixes. Keeping these separate ensures each change maintains the smallest possible scope and is independently verifiable.

@markfietje
Copy link
Contributor Author

⚠️ Note on CI Failure
In addition to the Synology regression, CI is also intermittently failing due to a flake in src/security/skill-scanner.test.ts. A fix for this flake is in PR #32159.

@markfietje
Copy link
Contributor Author

⚠️ Note on CI Failure
Intermittent CI failures are also being caused by a flake in src/cron/isolated-agent.uses-last-non-empty-agent-text-as.test.ts. A fix is in PR #32165.

@markfietje markfietje force-pushed the fix/upstream-tui-regression branch from d804296 to 4578f51 Compare March 2, 2026 21:00
@markfietje
Copy link
Contributor Author

Closing as recent refactors in main have resolved the immediate CI block. Glad to see the combined stack is now healthy.

@markfietje markfietje closed this Mar 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant