Skip to content

fix(aider,openhands): close client on exit + OpenHands Docker MCP docs#2417

Merged
DK09876 merged 1 commit into
mainfrom
fix/integration-testing-polish
Jun 27, 2026
Merged

fix(aider,openhands): close client on exit + OpenHands Docker MCP docs#2417
DK09876 merged 1 commit into
mainfrom
fix/integration-testing-polish

Conversation

@DK09876

@DK09876 DK09876 commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Polish from the real-app integration test sweep (separate from the Devin Desktop rename + Continue fix in #2410).

aider — close the client on exit

The wrapper opened a Hindsight client (aiohttp session) and never closed it, so aider printed Unclosed connector warnings on exit. Now run() closes the client when it owns it (a test/caller-injected client is left alone). Regression tests cover both paths. Bump hindsight-aider → 0.1.1.

openhands — document the Docker-app MCP path

Real testing showed the OpenHands Docker app ignores the project config.toml our init writes — it only loads MCP from UI settings — and adding the server as SSE silently fails (Hindsight's endpoint is Streamable HTTP). Added:

  • a README section telling Docker users to add the server in Settings → MCP as Streamable HTTP via host.docker.internal, and
  • the same hint printed by init.

Bump hindsight-openhands → 0.1.1.

Verification

  • aider: 36 tests pass (2 new) · openhands: 27 pass · ruff check + format clean
  • Both findings came from driving the real apps (aider CLI; OpenHands in OrbStack)

Not included here (tracked separately)

  • MCP tool-selection dilution — across Copilot/Zed/Devin/OpenHands the agent only reliably calls recall when explicitly nudged (29 tools + a soft rule). This is a design decision (stronger instruction vs. trimmed toolset) and deserves its own discussion/PR.
  • Devin Desktop "click Connect" doc note — deferred until the rename PR feat(devin-desktop): rename Windsurf→Devin Desktop + fix(continue) thread-safe adapter #2410 lands (the devin-desktop dir doesn't exist on main yet).

🤖 Generated with Claude Code

From real-app integration testing:

- aider: close the Hindsight client when the wrapper owns it, so aiohttp no
  longer prints 'Unclosed connector' warnings after aider exits. Test-injected
  clients are left to the caller. Bump 0.1.1.
- openhands: document that the OpenHands Docker app loads MCP from UI settings
  (not the project config.toml), and that the server must be added as a
  Streamable HTTP server (not SSE) reachable via host.docker.internal. Same hint
  printed by 'init'. Bump 0.1.1.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@DK09876 DK09876 force-pushed the fix/integration-testing-polish branch from 9eef9d3 to 9cf0a80 Compare June 26, 2026 20:08
@benfrank241

Copy link
Copy Markdown
Member

The two red checks here aren't from this PR — they're pre-existing breakage on main, and this branch is level with it. This diff only touches the aider/openhands packages, but both failures are in files it never modifies:

  • verify-generated-fileshindsight-api-slim/tests/test_retain_append_mode.py:45 has an unused local v2_units (F841). The job runs lint.sh, which auto-removes it, producing a diff. Fix: drop the assignment (or run ./scripts/hooks/lint.sh).
  • test-doc-examples (cli)hindsight-cli/src/commands/mental_model.rs (lines 120 and 198) is missing the refresh_cron field that was added to the generated MentalModelTriggerInput, so the CLI no longer compiles. Fix: set refresh_cron: None in both initializers.

Both currently block every open PR. Worth a tiny standalone fix PR against main. The actual changes in this PR — closing the aider-owned client in a finally (leaving caller-injected clients alone, with tests for both paths) and the OpenHands Docker MCP docs — look good and are appropriately scoped.

@DK09876 DK09876 merged commit a0af096 into main Jun 27, 2026
85 of 87 checks passed
@DK09876 DK09876 deleted the fix/integration-testing-polish branch June 27, 2026 00:28
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.

2 participants