Skip to content

Deferred review backlog rollup (PR #26) — Medium #34

@Anarchid

Description

@Anarchid

Two items from PR #26 (Assorted conhost fixes: MCPL policy, stderr forwarding, typing metadata, workspace watcher, merged 2026-04-23) were explicitly self-disclosed as out-of-scope at merge time and haven't been addressed since.

Background: agent-framework has no automated reviewer wired up and most review traffic in this repo happens out-of-band. The two items below are the only ones that left a recoverable GitHub trace — both as author self-disclosures.

Medium

  • Reconnect-path stderr listener leak in McplServerConnection. src/mcpl/server-connection.ts:564. The reconnect probe → persistent-swap path rebinds stderr from the throwaway fresh instance onto this, but fresh's own readline 'line' handler and process 'exit' handler stay registered on the transferred child (short-circuited only by fresh.closed = true). The post-merge follow-up commit 32cb10c explicitly added a TODO comment instead of fixing it; commit body reads "Not reworked — worth knowing for whoever cleans that reconnect shape up." TODO still present at line 564; no further commits to the file since.

  • Inode-replace race in workspace watcher. src/modules/workspace/watcher.ts. PR Assorted conhost fixes: MCPL policy, stderr forwarding, typing metadata, workspace watcher #26's body explicitly said: "Does not address inode-replace (dir rm'd + recreated post-subscribe); that needs re-attach logic / @parcel/watcher." No commits to the file since the merge. Manifests if a watched directory gets rm -rf'd and recreated while the subscription is live — chokidar reports nothing and the watcher silently stops emitting events on the new inode.

Origin

Audit run 2026-05-21. Both items are author self-disclosures in PR #26 — there was no external review on this PR. They have the same shape: lifecycle/race-correctness of integration code against external systems (subprocess lifecycle, filesystem watcher edge cases), which is the class of issue that's easiest to leave for later because reproducing the bug requires real load.

Audit-level meta-finding for this repo: the GitHub PR-thread surface is almost entirely empty. Across 27 merged PRs, only PR #11 had non-trivial line-level review activity (from cursor[bot], all addressed). Every other "Address review feedback" commit (PR #4, #19, #23, #25) refers to feedback that lives out-of-band — Discord/Zulip/elsewhere. If you want this kind of audit to be effective in the future, the cheapest single change is making /roast output post to the PR thread. As things stand, anything punted in a verbal review leaves no GitHub trace at all.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions