Skip to content

feat(devin-cli): add Devin CLI hooks integration - #3116

Closed
ichoosetoaccept wants to merge 1 commit into
vectorize-io:mainfrom
ichoosetoaccept:feat/devin-cli-memory-plugin
Closed

feat(devin-cli): add Devin CLI hooks integration#3116
ichoosetoaccept wants to merge 1 commit into
vectorize-io:mainfrom
ichoosetoaccept:feat/devin-cli-memory-plugin

Conversation

@ichoosetoaccept

Copy link
Copy Markdown

Why

Devin CLI users have no first-party way to give their agent persistent memory across
sessions. Every new session starts cold: stack, preferences, and past decisions have to be
re-explained, and whatever context existed is lost when the window compacts.

This adds a Hindsight integration for Devin CLI in the same shape as the existing
coding-agent integrations — Python hook scripts that read JSON from stdin and write JSON to
stdout, plus an MCP server for explicit knowledge tools. After install, every prompt
reaches the model with the most relevant memories from past sessions pre-injected, and
conversations are retained during the session and again at session end.

It is a port of the Claude Code plugin: same four lifecycle hooks, same settings.json
schema, same env var names, same agent_knowledge_* MCP tools. A shared external Hindsight
server and a single ~/.hindsight config can serve both integrations, and a parity test
keeps the schemas from drifting apart.

How memory flows

Devin hook Script What it does Why this hook
SessionStart session_start.py Registers the session, verifies Hindsight is reachable, and pre-warms the local hindsight-embed daemon in the background. Fire-and-forget setup that must never block the agent — 5s budget.
UserPromptSubmit recall.py Queries Hindsight for memories relevant to the prompt and injects them as hookSpecificOutput.additionalContext. Synchronous, runs every prompt; the only place recall can reach the model.
Stop retain.py Reads the session's unretained messages and POSTs them to the retain API with async=true, every configured N turns. Fires each turn; the server processes retention in the background.
SessionEnd session_end.py Forces a final retain (so sessions shorter than retainEveryNTurns still land), deregisters the session, and stops the daemon once the last session is gone. Guarantees short sessions are stored before the process exits.

Explicit memory is available too: the MCP server exposes the agent_knowledge_* tools
(recall, ingest, and knowledge-page CRUD) over stdio.

Two Devin-specific constraints

Both are worked around rather than solved, because Devin CLI's plugin system shipped in
v2026.7.16 (June 16, 2026) and is still labelled beta — "behavior and configuration may
change in future releases". Both were re-verified against CLI 3000.3.22 before submitting.

Setup is two steps. Devin exposes exactly one environment variable to hook processes,
DEVIN_PROJECT_DIR (the user's project, not the plugin), and offers no plugin-root
substitution for hook commands. A relative command is handed to the shell verbatim and
resolves against the directory the session was launched from, not the directory the hooks
file lives in — confirmed with a probe hook whose hooks.v1.json sat at a repo root while
the script it named existed only in the subdirectory devin was started from: it ran,
reporting PWD as the launch directory. So a plugin cannot name its own scripts. After
devin plugins install, scripts/install.py writes absolute-path entries into Devin's
documented config locations; it is idempotent, replaces a previous version's entries on
upgrade, and leaves unrelated third-party entries alone.

One qualifier, since it is easy to check and conclude otherwise: the CLI binary does
contain ${CLAUDE_PLUGIN_ROOT}, alongside the ${env:VAR} / ${file:...} interpolation
the MCP docs describe — but it sits in the MCP config importer, not the hook path. If it
resolves for a plugin's mcp_config.json, the MCP server could be declared rather than
installed, and only the four hooks would need the installer. It is undocumented and I could
not verify it here (devin plugins install requires an org policy check this environment
cannot reach), so the installer registers both — one install path, one uninstall path.
If you or the Devin team can confirm the substitution, I will move the MCP half into
mcp_config.json and shrink the installer.

Retain reads Devin CLI's session SQLite database. The shipped reference documents the
entire Stop payload as stop_hook_active, plus the universal session_id / prompt_id
there is no transcript_path. Devin does persist every session locally keyed by that same
session_id, and lib/devin_transcript.py reads it. The read path is deliberately
defensive: a missing file, table, or column, or an unparseable payload, degrades to an empty
transcript, so a future CLI release that changes the schema switches auto-retain off rather
than breaking sessions. session_end.py keys off session_id for the same reason.

Everything else matches the Claude Code plugin's design: a local stdio MCP server via
run_mcp.sh, and a local hindsight-embed daemon (or an external server). The daemon runs
on port 9078 so it does not collide with the Claude Code plugin's on 9077.

Configuration

~/.hindsight/devin-cli.json for personal overrides, stable across plugin updates and
matching ~/.hindsight/claude-code.json:

{
  "hindsightApiUrl": "https://api.hindsight.vectorize.io",
  "hindsightApiToken": "your-api-key",
  "bankId": "my-devin-memory"
}

settings.json at the plugin root holds the shipped defaults. Every option also has an
env-var override (HINDSIGHT_API_URL, HINDSIGHT_BANK_ID, HINDSIGHT_RECALL_BUDGET, …) —
the same set the Claude Code plugin documents, which is what the parity test pins.

What's in the integration

hindsight-integrations/devin-cli/
├── README.md, CHANGELOG.md, LICENSE, settings.json
├── pyproject.toml, requirements.txt      # dual-distributed: plugin + pip package
├── .devin-plugin/plugin.json             # plugin manifest
├── scripts/
│   ├── session_start.py                  # SessionStart
│   ├── recall.py                         # UserPromptSubmit
│   ├── retain.py                         # Stop
│   ├── session_end.py                    # SessionEnd
│   ├── install.py / uninstall.py         # hook + MCP registration
│   ├── mcp_server.py, run_mcp.sh         # stdio MCP server + venv bootstrap
│   └── lib/                              # bank, client, config, content, daemon,
│                                         # devin_transcript, llm, state
├── skills/setup/, skills/create-agent/
└── tests/                                # 398 tests

Install

devin plugins install vectorize-io/hindsight#hindsight-integrations/devin-cli

Then register the hooks and MCP server once — easiest by starting a session and asking Devin
to set up Hindsight memory, which invokes the plugin's setup skill. The README documents
the direct command too.

Relationship to hindsight-devin-desktop

Worth a maintainer decision before merge. hindsight-devin-desktop v0.2.0 (#2692) added
devin_local.py, targeting the Devin Local agent — which its own docstring describes as
"the successor agent, shared with the Devin CLI". That is the same runtime this plugin
targets, and it writes the same config file (~/.config/devin/config.json).

Both installers preserve each other's entries, which is the problem: this plugin's
_OURS_RE (scripts/install.py:55) only matches its own python3 "…/scripts/<hook>.py"
command shape, and devin-desktop's _is_ours_for (devin_local.py:182) only matches
sys.executable -m hindsight_devin_desktop.hook. Neither strips the other, so a user with
both installed gets two SessionStart recall injections and two Stop retain paths against
the same session.

Suggested split, by agent runtime rather than by product: hindsight-devin-desktop narrows
to Cascade (~/.codeium/windsurf/) and drops devin_local.py; this plugin owns the Devin
CLI runtime (~/.config/devin/) however it is launched — standalone or driven by Desktop
over ACP. Happy to send that as a follow-up if you agree; it is deliberately out of scope
here so the new plugin can be reviewed on its own.

Separately: hindsight-docs/blog/2026-07-02-devin-desktop-persistent-memory.md predates
devin_local.py by 11 days and still says Devin Desktop "doesn't expose lifecycle hooks to
third parties"
. No longer accurate for Devin Local; needs a refresh independently of this
PR.

Prior art: fixes already reported or proposed against claude-code

I went through the open issues and PRs before submitting, because a port that silently
re-fixes what someone has already proposed is worse than useless. Three overlaps:

Already fixed here, independently and identically — #2999. retain.py's
retain_full_window = retention_progress.start_index == 0 narrows the retain slice a second
time after plan_retention() has already narrowed it, while commit_retention() advances
the cursor past the whole suffix. #2999 changes it to = True with the same reasoning; so
does this port. I reached it from a different symptom (an assistant-only tail formatting to
nothing) and found #2999 afterwards. If #2999 lands first this needs no rebase — the line
is already what that PR makes it.

Inherited, and fixed here on the strength of your report — #3026 / #3027. This plugin
shipped mcp>=1.0.0, the same unbounded spec. mcp 2.0 removed mcp.server.fastmcp, so a
fresh install resolves to a release mcp_server.py cannot start under — and run_mcp.sh
only re-pips when import mcp fails, which succeeds under 2.x, so the venv is never
repaired. Now mcp>=1.0.0,<2, with a second test asserting mcp_server.py still imports
from that path, so the pin is lifted when the reason for it goes away rather than outliving
it.

Adjacent, not conflicting — #2900 / #2913. #2913 adds a --check release guard to
scripts/release-integration.sh; this PR changes the version-bump block in the same file.
Different concerns, non-overlapping hunks, but whichever merges second may want a look.

Six further fixes proposed upstream are deliberately not adopted here: #2812
(nearest-ancestor directoryBankMap), #2897 (HINDSIGHT_RETAIN_TAGS), #3024 / #2487 (filter
injected harness turns), #2869 (detach SessionEnd), #3110 / #3098 (project resolution when
the worktree is gone), #2493 (seed bank missions only when unset). All apply equally here.
They are left out because adopting them would mean picking a winner between competing PRs —
#3110 and #3098 both fix #3096 — and a new-integration PR is the wrong place to pre-empt
those reviews. This port matches current main and will follow whichever lands.

Other divergences from the Claude Code plugin

Ten rounds of automated review ran against this branch before submission. The great majority
of what it surfaced was not in the new code — it was in files copied from
hindsight-integrations/claude-code, several byte-for-byte. Rather than carry them over,
they are fixed here.

claude-code is untouched by this PR, so the two plugins now differ in 26 places. Each
is a candidate for a follow-up PR against claude-code, which I am happy to send — I did not
want a new-integration PR to also be a behaviour change to an existing one. The full list
with per-item reasoning is in the commit message; the headline items:

  • Security. lib/daemon.py no longer passes HINDSIGHT_API_LLM_API_KEY as a
    profile create --env argument, where any local user could read it from the process
    listing. It reaches the daemon through the subprocess environment instead.
  • The daemon was shared but its lifecycle was not. The stop decision hung off a single
    global marker, so the first session to end stopped the daemon out from under the others.
    Sessions now register under an interprocess lock and it stops only when the last one
    deregisters; ownership is released in the same locked write that decides to stop, and
    handed back if the stop then fails. A failed stop is now detected at all — _run_embed()
    does not pass check=True, so "could not stop" came back as an ordinary
    CompletedProcess and read as success.
  • Retention silently dropped messages. Checkpoints were committed before the retain
    request rather than after, so a failed request skipped the messages it never sent.
  • _file_lock() yielded unlocked when it could not acquire the lock, so a
    read-modify-write ran with no interprocess synchronisation while looking exactly like it
    held the lock — the corruption the lock exists to prevent.
  • A malformed setting took a hook down with it. A string where a list belongs iterates
    one character at a time; a list where a dict belongs raises AttributeError; a string
    where an int belongs raises TypeError. All inside a hook, so one mistyped optional
    setting switched off the thing it was added to tune. load_config() now checks each value
    against the type of its DEFAULTS entry.
  • UnicodeDecodeError slipped through eight handlers — it subclasses ValueError, not
    OSError, so it escaped except (json.JSONDecodeError, OSError) at the settings reader,
    all four hooks' stdin readers, the installer's config read, the git worktree probe, and the
    session-db row parser. A single bad byte stopped every hook.
  • Neither hook can exit nonzero any more. Both document "0 — always" and both exited 2
    under debug, so turning debug on to investigate a recall failure escalated it from "no
    memories this turn" to a rejected prompt.

Also fixed: an unescaped SQLite URI, an unescaped page_id in MCP URL paths, a document_id
that stripped file extensions (so README.md and README.txt collided and the second
overwrote the first), a "name": null on a tool_use block that aborted a whole transcript,
LRU rather than lexicographic session eviction, and run_mcp.sh absolutising its data
directory before cd.

One inherited behaviour is deliberately left alone: _is_channel_message_tool() classifies
any mcp__* tool carrying a text-like input field as a channel reply, which catches
non-messaging tools such as mcp__email__send. That heuristic is a documented tradeoff — it
recognises any channel plugin without hardcoding tool names, and the obvious alternative
defeats the property it was built for. Worth changing deliberately alongside claude-code.

CI / release wiring

  • New test-devin-cli-integration job in .github/workflows/test.yml, gated on a new
    integrations-devin-cli path filter and wired into the aggregate status check.
  • devin-cli added to VALID_INTEGRATIONS in scripts/release-integration.sh.
  • Added to hindsight-integrations/README.md and the docs integrations index.
  • One fix outside the new plugin: scripts/release-integration.sh bumped only the first
    match of an if/elif chain, so releasing any integration with more than one version
    source updated pyproject.toml and silently left the plugin manifest stale. That already
    affects cursor and zcode. It now bumps every source present. (Related to Claude Code plugin.json version not bumped for v0.8.5 — plugin update is a no-op #2900; see the
    fix(release): guard Claude Code integration version #2913 note above.)

Verification

cd hindsight-integrations/devin-cli
uv run pytest tests -q
# 398 passed

uv run --no-project ruff check --config ../../ruff.toml .
uv run --no-project ruff format --check --config ../../ruff.toml .
# All checks passed! / 34 files already formatted

No live Hindsight server or Devin CLI is required for the suite — hooks are tested
end-to-end with a fake HTTP client, mocked stdin/stdout, and an isolated HOME. Every
regression test for the fixes above was confirmed to fail against the unpatched source; the
concurrency ones spawn real subprocesses, because the guarantee is an interprocess one. A
parity test asserts settings.json, DEFAULTS, and ENV_OVERRIDES stay identical to the
Claude Code plugin apart from the documented divergences, so adding a key to one plugin and
not the other fails CI. install.py / uninstall.py are verified idempotent against a
config file containing unrelated third-party hook entries.

@ichoosetoaccept
ichoosetoaccept marked this pull request as ready for review July 31, 2026 19:14
@ichoosetoaccept
ichoosetoaccept marked this pull request as draft July 31, 2026 19:27
@ichoosetoaccept
ichoosetoaccept force-pushed the feat/devin-cli-memory-plugin branch 2 times, most recently from a559ad5 to d0d93c1 Compare July 31, 2026 20:29
Port of the Claude Code plugin to Devin CLI: same hooks, config schema,
and MCP tools, adapted to Devin CLI's hook payloads and plugin system.

Two behaviours differ from the Claude Code plugin, both forced by gaps in
Devin CLI's extensibility surface:

- Setup is two steps. Devin CLI offers no plugin-root substitution for hook
  commands, and the only variable it exports to a hook process is
  DEVIN_PROJECT_DIR — the user's project, not the plugin. A relative command
  is handed to the shell verbatim and resolves against the directory the
  session was launched from, not the directory the hooks file lives in
  (verified with a probe hook on CLI 3000.3.22), so a plugin cannot name its
  own scripts. scripts/install.py writes absolute-path entries into the
  documented config locations instead; it is idempotent and replaces a
  previous version's entries on upgrade. Note that the binary does carry
  ${CLAUDE_PLUGIN_ROOT}, but in the MCP config importer rather than the hook
  path; if that resolves for a plugin's mcp_config.json the MCP half could be
  declared rather than installed. It is undocumented and unverified here, so
  the installer registers both and there is one uninstall path.
- Retain reads Devin CLI's local session SQLite database. Its Stop hook
  carries no transcript_path. The read path is defensive: a missing file,
  table, column, or unparseable payload degrades to an empty transcript, so a
  future CLI release disables auto-retain rather than breaking sessions.

Runs on its own daemon port (9078) so it does not collide with the Claude
Code plugin's local daemon.

The port also diverges from its source in a number of places where the
inherited behaviour was wrong. These are fixed here only; the Claude Code
plugin is deliberately untouched, and the same fixes are worth sending
upstream to it separately.

Concurrency and daemon lifecycle:

- The daemon is shared by every concurrent Devin CLI session, but its
  lifecycle hung off a single global marker, so the first session to end
  stopped it out from under the others, which had no way to notice.
  Sessions now register against the daemon state and it is stopped only
  once the last one deregisters. Registration and deregistration run under
  an interprocess lock; unlocked, two hooks racing would each drop the
  other's id. A crashed session never deregisters, so this can hold a
  daemon open past its last real user — bounded by the daemon's own idle
  timeout, which reaps it anyway. Both start paths claim ownership through
  that same lock, merging into the registry rather than writing over it:
  pre-start returns before the daemon listens, so the next session's health
  check races it and also starts one, and the synchronous path carried no
  session list at all. Either write would have dropped a live session's id
  and let the next SessionEnd stop the daemon out from under it — the very
  failure the registry exists to prevent.
- Ownership is released in the same locked write that decides to stop, so
  a session arriving during the teardown cannot attach to a daemon already
  condemned. The stop itself runs outside the lock on purpose: it is
  allowed 10s and SessionStart only 5s, so holding the lock across it
  would trade the race for a guaranteed hook timeout. If the stop then
  fails, ownership is handed back — after re-checking health, under the
  lock, and yielding to any newer daemon that claimed the port meanwhile.
  That health probe draws on what is left of SessionEnd's budget rather
  than a fixed 2s of its own, since the stop before it is allowed the whole
  10s and the sum would otherwise get the hook killed.
- A failed stop is now detected at all. _run_embed() does not pass
  check=True, so the common failure — the daemon reporting it could not
  stop — came back as an ordinary CompletedProcess and read as success.
- Retention checkpoints are committed by compare-and-swap after a
  successful retain, not before it, so a failed request no longer skips
  the messages it never sent. The swap compares the chunk as well as the
  message count: a compaction can hand the same count back, so the count
  alone is not a version, and a stale hook matching on it would roll the
  chunk backwards and mark messages retained that belong to a transcript
  that no longer exists.
- The incremental retain slice is no longer narrowed twice. The checkpoint
  already reduces it to exactly the unretained messages, but
  retain_full_window was set from `start_index == 0`, so every retain
  after the first also asked the formatter for "the last turn only"
  within that slice. Anything outside the last turn was dropped and the
  empty-transcript path then committed the full message count, so no later
  run retained it either. Two ways that bit: an assistant-only tail
  formatted to nothing, and a slice spanning several turns (after one
  failed retain) silently lost every turn but the last.
- _file_lock() no longer yields unlocked when it cannot acquire the lock.
  It swallowed the error and ran the body anyway, so the read-modify-write
  proceeded with no interprocess synchronisation while looking exactly
  like it held the lock — two hooks reading the same counter and one
  overwriting the other, which is the corruption the lock exists to
  prevent. "The state directory is unwritable anyway" is not a defence: a
  stale lock file with the wrong owner or mode blocks the lock while the
  state file stays writable.

State file handling:

- write_state() staged every write at a fixed <path>.tmp, so two
  concurrent hooks writing the same file could have one os.replace() the
  partial file of the other. Staging is now per-process — in install.py's
  config writer as well, which lands on files the user did not ask us to
  corrupt.
- read_state() did not catch UnicodeDecodeError, which is what a torn
  write looks like from text mode and is neither JSONDecodeError nor
  OSError. A single bad write crashed every subsequent hook. It also
  returns the caller's default when the parsed value is the wrong shape,
  which covers four call sites that reached .get() on a non-dict.
- A malformed or negative message_count read as a checkpoint would slice
  the transcript from a negative offset, sending only the final message
  and then committing the real total — permanently skipping everything
  before it. Such values now read as "no checkpoint", which re-retains the
  session and is recoverable. bool is rejected explicitly, since it
  subclasses int.
- Tracked sessions are evicted LRU rather than by lexicographic id, which
  for opaque ids is arbitrary; an actively used session could lose its
  checkpoint while stale ones survived. The current session is never
  evicted, which previously raised KeyError out of increment_turn_count().

Parsing and I/O:

- slice_last_turns_by_user_boundary() no longer counts a tool result as a
  turn. Tool results arrive as role:"user" messages carrying a tool_result
  block, so in an ordinary exchange — prompt, tool use, tool result, answer
  — asking for two turns started the window at the tool result and dropped
  the prompt that caused it, from both the recall query and chunked
  retention. The sibling openclaw integration already filters these; this
  is the same filter.
- Tool calls read from the session database carry their id onto the
  tool_use block they become. content.py suppresses an operational
  Hindsight tool's *result* by matching tool_use_id against the id of the
  tool_use it dropped, which is the half of the anti-feedback-loop guard
  that keeps recalled memories from being retained again — and synthesising
  a block without an id disabled it silently, while the id sat unused in
  the OpenAI-style call being converted.
- A negative retainOverlapTurns can no longer empty the retain window.
  window_turns = retain_every_n + overlap_turns feeds a slicer that returns
  nothing for turns <= 0, and an empty transcript is skipped, so retention
  switched off silently — including on the forced session-end retain. The
  central check guarantees the type, not the range; overlap is clamped to
  widen the window only.
- dynamicBankGranularity is checked element by element, not just as a list.
  Both the validity test and the value lookup are hashed, so an unhashable
  element raised TypeError out of derive_bank_id() rather than reporting an
  unknown field — and derive_bank_id() runs in recall and retain both.
- Non-list tool_calls degrade instead of raising, and a message_id of 0 or
  "" is still deduplicated. read_session_messages() copied tool_calls
  through on truthiness alone, so a scalar reached `for call in ...`; and
  the dedup key was truth-checked, so those two valid ids read as absent
  and every replayed row carrying one was emitted again.
- An explicit "name": null on a tool_use block reached .startswith() and
  aborted the whole transcript. dict.get(k, default) only fills in for a
  missing key. Both readers are fixed, along with the same idiom on a
  tool_result text block ("text": null) and on the recall hook's own
  `prompt`, and non-string text blocks are guarded on the recall path as
  well as the retain path.
- A session-database row whose `role` is not a string is skipped rather
  than passed on. An emptiness check let a dict or a number through, which
  kept this reader's "never raises" contract only by moving the crash into
  the formatter it hands the transcript to.
- The SQLite URI interpolated the database path unescaped, and SQLite
  parses that URI — a `?` starts the parameter list and a `%` begins an
  escape. A configured path containing either opened some other filename,
  or none, and degraded to an empty transcript while the real database sat
  there readable.
- run_mcp.sh absolutises its data directory before deriving anything from
  it, since the script cd's there before its final exec and POSIX exec
  resolves a path containing a slash against the current directory.
- _plugin_version() runs at module import and called .get() on the parsed
  manifest, so a plugin.json holding valid JSON that is not an object
  raised AttributeError — uncaught — and killed every hook before it could
  make a request.
- The recall hook persisted a diagnostic breadcrumb before printing its
  response, so an unwritable state directory aborted the hook after recall
  had already succeeded and the user got no memories at all.
- The API URL is rejected if it carries a query or fragment, which would
  otherwise be silently dropped when paths are appended to it.
- The API key is kept out of argv; it reaches the daemon through the
  subprocess environment instead of `profile create --env`, which puts its
  arguments in the process listing.

Tools and configuration:

- agent_knowledge_ingest_file() stripped the extension when deriving
  document_id, so README.md and README.txt both became "readme". retain
  replaces by document_id, so ingesting the second silently overwrote the
  first. The full basename is used now, which also matches what the tool's
  own docstring promises.
- In the plugin-config branch a configured llmModel beat
  HINDSIGHT_API_LLM_MODEL, so the daemon ran a different model than the
  environment asked for. lib/llm.py documents the HINDSIGHT_API_LLM_* vars
  as the highest-priority source, and base_url in that same return already
  honoured the override unconditionally — the model was the odd one out.
- ensure_bank_mission() ran the API call and the record of it under one
  `except Exception`, so an OSError from write_state surfaced as "Could
  not set bank mission" when the mission had in fact been set and only the
  record was lost. The two are reported separately now. The write stays
  non-fatal deliberately: this is a hook path, and an unwritable state
  directory must not abort a hook that has already done its work.
- A malformed setting no longer takes a hook down with it. A settings file
  is arbitrary user JSON, and most wrong shapes are not a different
  behaviour but a crash: a string where a list belongs iterates one
  character at a time, a list where a dict belongs raises AttributeError
  from .get(), a string where an int belongs raises TypeError from
  arithmetic. Every one of those happened inside a hook, so a single
  mistyped optional setting switched off the thing it was added to tune —
  recall emitting nothing, or derive_bank_id blocking both hooks.
  load_config() now checks each value against the type of its entry in
  DEFAULTS and reverts anything that does not match. Deriving the expected
  shape from DEFAULTS rather than a parallel table means a new setting is
  covered the moment it has a default. Types are compared exactly, since
  bool subclasses int and `true` would otherwise reach arithmetic as 1.
  Per-entry shapes are still guarded at their call sites — the central
  check validates the top-level value, not what is nested inside it — as
  is directoryBankMap, since derive_bank_id() takes a caller-supplied
  config rather than always one from load_config(). requestTimeoutSeconds
  is validated at the client instead: its default is None, which carries no
  type, so the central check skips it — yet it is used structurally, as
  urllib's timeout, where a string raises TypeError and a zero makes every
  request fail instantly.
- The dict coercion in _cast_env() accepts only a JSON object. It took an
  array too, handing a list to a caller that calls .get() on it — the
  AttributeError-inside-a-hook the coercion exists to prevent. The list
  branch beside it already required a list.
- install.py refuses to replace a pre-existing "hindsight" MCP server it
  did not write, and uninstall.py refuses to delete one. The key alone was
  treated as proof of ownership, so a user who had configured their own
  server under that name lost it silently on install and for good on
  uninstall. Both now recognise the plugin's own entry by its launcher, the
  way _OURS_RE recognises its own hooks; --force replaces it deliberately.
  A warning would not have been enough: it scrolls past, and by the time it
  is read the value it names is already gone.
- install.py survives every shape a user's config.json can legally hold.
  `{"hooks": null}` reached dict(None), a non-list event value reached a
  for-loop, and a non-dict entry reached .get() — each an unhandled
  traceback out of the installer, which leaves the plugin uninstallable for
  a configuration the user cannot diagnose from the output. Unrecognised
  entries are treated as not ours, so they survive. An mcp_config.json
  whose "mcpServers" is not an object is reported and left alone.
- Hook ownership is decided per command rather than per entry, in both
  install.py and uninstall.py. A hooks.json entry holds a *list* of commands
  under one matcher, and nothing stops a user from listing this plugin's hook
  beside their own; one matching command made the whole entry ours to delete,
  so reinstalling or uninstalling this plugin took those third-party hooks
  with it, silently and with nothing left to restore from. Only matching
  commands are removed now, and an entry emptied of them is dropped whole so
  no husks accumulate. uninstall.py also compares the result by value instead
  of by length — an entry that merely lost one of its commands kept the list
  the same length, so the removal was read as "nothing changed" and never
  written, leaving an uninstalled hook still registered and still running.
  Its malformed-shape guards now match install.py's, for the same reason:
  fixing one side and not the other is how a contract drifts apart.
- A session that starts while the daemon is being stopped stays in the
  registry. stop_daemon() releases ownership in the same locked write that
  decides to stop, then runs `daemon stop` outside the lock — so a
  SessionStart that health-checks the still-running daemon in that gap
  reached register_session() with an empty state and was dropped. That is
  only safe while the stop succeeds, since the session then finds nothing
  listening and starts its own. When the stop *fails*,
  _reclaim_after_failed_stop() hands ownership back, and it was doing so with
  a hardcoded empty session list: the live session was erased, and the next
  SessionEnd read no other sessions and killed the daemon out from under it —
  precisely the failure the registry was added to prevent. Registrations are
  now recorded whether or not a marker is present, the reclaim carries them
  forward, and a fresh ownership claim adopts them unless its marker names a
  different port. Recording an id can only ever delay a stop, never cause one.
- directoryBankMap's values are type-checked, not just the map. A non-string
  value was returned verbatim as the bank id — or, with bankIdPrefix set,
  f-string-formatted into one, making the literal bank `p-['wrong']`. Both
  hooks then read and wrote a bank the user never named. Unlike the other
  malformed-input fixes here this one is silent rather than loud: nothing
  raises, and nothing surfaces until memories are missing from the bank they
  were supposed to be in. A bad entry falls through to the next resolution
  branch, which lands on a real bank.
- An operational Hindsight tool_use with no id no longer lets its result
  through. Suppressing the call is only half of breaking the feedback loop —
  the *result* is what carries recalled memories back into the transcript —
  and the two are correlated by id, so a call carrying none recorded nothing
  and its result was retained. Every later retain then re-ingests those
  memories and the bank compounds its own output. The matching result cannot
  carry a tool_use_id either, both being decoded from the same payload, so its
  own missing id is what correlates them. An unhashable tool_use_id is
  rejected before the set lookup rather than raising out of the hook.
- The README's external-server command merges into ~/.hindsight/devin-cli.json
  instead of replacing it. A `>` redirect meant a user with a bankId, token, or
  debug flag already configured lost all of it by following the documented
  setup step.
- UnicodeDecodeError is caught wherever text is decoded. It subclasses
  ValueError rather than OSError, so it slipped through eight handlers
  that were otherwise careful: the settings-file reader (one bad byte
  stopped every hook, the exact opposite of its fall-back-to-defaults
  intent), all four hooks' stdin readers, the installer's config read, the
  git worktree probe, and the session-database row parser. Only the
  plugin-manifest read already covered it, by catching ValueError.
- requirements.txt caps mcp below 2.0. mcp 2.0 removed mcp.server.fastmcp,
  which mcp_server.py imports, so the inherited unbounded `mcp>=1.0.0`
  resolves on a fresh install to a release the server cannot start under —
  and run_mcp.sh only re-pips when `import mcp` fails, which succeeds under
  2.x, so the venv is never repaired. This is issue #3026 against the Claude
  Code plugin, whose fix is proposed in PR #3027; this plugin shipped the
  same spec. A second test asserts mcp_server.py still imports from that
  path, so the pin is lifted when the reason for it goes away rather than
  outliving it.
- strip_channel_envelope() matched anywhere in the message rather than
  end to end, so any message that merely *mentioned* a channel envelope
  was replaced by that envelope's inner text — "please explain <channel
  ...>hello</channel> thanks" was retained as "hello", discarding what the
  user actually wrote, from both recall and retention.
- Neither hook can exit nonzero any more. Both document "0 — always
  (graceful degradation on any error)" and both exited 2 under `debug`. A
  nonzero hook exit is a blocking error, so turning debug on to
  investigate a recall failure escalated it from "no memories this turn"
  to a rejected prompt — a diagnostic flag changing control flow, and
  changing the behaviour of the very failure being investigated. Only
  recall.py was reported; retain.py had the identical path and is fixed
  with it.
- A recallMinScores floor of "nan" was accepted as a working floor, since
  float() parses it and every `value < nan` comparison is False. The
  filtering outcome is unchanged either way — a NaN floor drops nothing and
  neither does rejecting it — but a broken floor now says so in the debug
  log instead of quietly doing nothing. `inf` is rejected alongside it, and
  that one did change behaviour: it rejected every result.

One inherited behaviour is deliberately left alone.
_is_channel_message_tool() classifies any mcp__* tool carrying a text-like
input field as a channel reply, which catches non-messaging tools such as
mcp__email__send. That heuristic is a documented tradeoff — it exists to
recognise any channel plugin without hardcoding tool names — and the
obvious alternative, a positive channel identifier, defeats the property
it was built for. A misclassified tool's input is retained as assistant
text rather than as structured tool data, which costs transcript fidelity
but no data. Changing shared classification behaviour is worth doing
deliberately with the Claude Code plugin rather than as a side effect of
this port.

Also fixes a latent bug in scripts/release-integration.sh, hit by any
integration that declares more than one version source. It bumped only the
first match of an if/elif chain, so releasing a dual-distributed plugin
(cursor, zcode, and now devin-cli ship both a pyproject.toml and a host
plugin manifest) updated pyproject.toml and silently left the plugin
manifest on the old version. It now bumps every source present.

The CI job pins Python 3.11 like every other integration job rather than
reading the repo-root .python-version, which its path filter does not
watch — a bump there would have changed what this plugin is tested on
without ever running the job that would find out.

492 tests, all passing. The regression tests for the fixes above were each
verified to fail against the unpatched source; the concurrency ones run
real subprocesses, because the guarantee is an interprocess one.
@ichoosetoaccept
ichoosetoaccept force-pushed the feat/devin-cli-memory-plugin branch from d0d93c1 to 6391574 Compare July 31, 2026 20:46
@ichoosetoaccept

Copy link
Copy Markdown
Author

Closing this — superseded by #2522, which landed Devin CLI support in hindsight-integrations/coding-agents/ a few hours before I pushed the last revision here.

That's the better home for it. Devin CLI there is three entrypoint files, one HOOK_HARNESSES declaration, transcript-devin.ts and one HarnessInstaller — roughly 150 lines against a lifecycle shared with nine other agents — where this PR is a standalone Python plugin reimplementing recall, retain, bank derivation, knowledge tools and installation for a single agent.

Worth recording that the two implementations independently reached the same two conclusions about Devin CLI, since neither is documented by Cognition:

  • Devin CLI's hook payloads carry no transcript_path, so the conversation has to be read out of ~/.local/share/devin/cli/sessions.db (message_nodes, de-duplicated by message_id).
  • A plugin-shipped hooks.json has no ${DEVIN_PLUGIN_ROOT}-equivalent, and its relative paths resolve against the project working directory rather than the plugin's install directory — so hook and MCP registration has to be written into ~/.config/devin/config.json and ~/.config/devin/mcp_config.json.

Separately: two findings from reviewing this branch apply to code that is still on main, independent of anything proposed here. Recording them so they aren't lost with the PR.

Tool results are counted as user turn boundaries. In hindsight-integrations/claude-code/scripts/lib/content.py, slice_last_turns_by_user_boundary treats every role: "user" message as a turn boundary (line 185), as does the last-turn retention path (line 336). Anthropic-style tool results are delivered as role: "user" messages, so a normal prompt → tool_use → tool_result → answer exchange puts the boundary on the tool result and drops the prompt that caused it. This affects both recall query composition and chunked retention. The openclaw integration already filters these out in its own sliceLastTurnsByUserBoundary.

strip_channel_envelope matches anywhere in the message. Same file, line 32, uses re.search rather than an anchored match, so a message that merely mentions a channel envelope is replaced by that envelope's inner text — please explain <channel source="x">hello</channel> thanks is retained as hello, discarding what the user actually wrote, from both recall and retention.

No action needed on this PR.

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.

Claude Code: deleted subagent worktrees create orphan agent-* banks

1 participant