[Bug] Six smaller correctness defects found during the code-waste audit

[kyegomez]

What

Six secondary correctness issues surfaced during the code-waste audit. Each is small and independent; grouped here to keep them from being lost. Split into separate issues if any needs its own discussion.

---

1. conversation.py:1300-1319 — return_all_except_first skips two messages, not one

Both return_all_except_first (:1300-1306) and return_all_except_first_string (:1308-1319) slice [2:], not [1:]. The name, the docstring, and the obvious reading all say "except the first" — callers silently lose the second message too.

Needs a decision: fix the slice (behavior change for anyone depending on the current output) or rename the methods to match reality. Recommend fixing the slice and adding a test.

---

2. agent.py:4109-4115 — an exception tuple that collapses to except Exception

except (AgentRunError, AgentLLMError, BadRequestError,
        InternalServerError, AuthenticationError, Exception):

All five named classes are Exception subclasses, so the tuple is exactly equivalent to except Exception. The enumeration reads as if specific errors are being handled distinctly, but nothing distinguishes them. Either give each its own handler, or collapse to except Exception (6 lines → 1).

---

3. agent.py:2901-2903 — awaiting a sync method that always raises

arun does await self._handle_run_error(error), but _handle_run_error (:1788) is a sync method whose last statement is raise error. The await therefore never actually runs. It is harmless only by accident: if _handle_run_error is ever changed not to raise, this becomes TypeError: object NoneType can't be used in 'await' expression. Drop the await, or make the method genuinely async.

---

4. agent.py:3413-3415 — _reinitialize_after_load stores an already-shut-down executor

# if not hasattr(self, "executor") or self.executor is None:
with ContextThreadPoolExecutor(...) as executor:
    self.executor = executor

The with block shuts the executor down on exit, so self.executor is assigned a dead executor. There is also a commented-out guard left above it. Related to #1793 (executor never assigned in __init__) but a distinct site — worth fixing together.

---

5. graph_workflow.py:2557 / :2739 — visualize has a dead fallback and skips filename sanitization

output_path is assigned unconditionally at :2557, so the if output_path is None: branch at :2739-2744 is unreachable (6 dead lines). Worse, the dead branch is the one that builds a sanitized safe_name — the live path at :2557 uses the raw self.name, so a workflow whose name contains / produces a broken path and fails to render. Move the sanitization into the live path and delete the dead branch.

---

6. aop.py:2582-2597 — over-broad network-error classification

network_keywords lists "timeout" twice, and the list ("connection", "network", "socket", "reset", "aborted", …) is broad enough that _is_network_error returns True for most exception messages — routing genuinely non-network failures into _handle_network_error's retry loop, where they are retried pointlessly and reported misleadingly. The comment at :2566-2572 shows the isinstance check was already narrowed for exactly this reason, but the keyword list was never revisited. See also #1818.

---

Scope

• 1. Fix the [2:] slice (or rename); add a test
• 2. Collapse the redundant exception tuple
• 3. Remove the bogus await (or make _handle_run_error async)
• 4. Fix the executor assignment in _reinitialize_after_load; remove the commented-out guard
• 5. Move filename sanitization into the live visualize path; delete the dead branch
• 6. Tighten network_keywords and de-duplicate "timeout"

Note

A seventh item from the audit — file tools in autonomous_loop_utils.py not confining resolved paths to the workspace root — is already tracked as #1791.

Context

From the code-waste audit (experimental/CODE_WASTE_AUDIT.md, "Secondary correctness notes").

🤖 Generated with Claude Code

[nightcityblade]

Hi, I'd like to work on item 1 (fixing the return_all_except_first slice behavior and adding a regression test). I'll submit a focused PR shortly.

[kyegomez]

Status check against d59a2da9 — 3 of 6 done, keeping this open for the rest.

#	Item	Status
1	return_all_except_first slices [2:]	open — still [2:] in conversation.py
2	Exception tuple collapses to except Exception	open — tuple still ends in Exception,
3	await on a sync _handle_run_error	fixed by #1871
4	_reinitialize_after_load stores a dead executor	open — self.executor = executor still inside the with
5	visualize dead fallback / missing sanitization	fixed by #1863
6	aop.py over-broad network keywords	fixed by #1877 — the duplicate "timeout" is gone

Notes on the three that remain:

• Item 1 has an open PR: [Bug] ConcurrentWorkflow silently drops one agent's answer, and which one changes every run #1884 fixes the slice conditionally (deriving the offset from whether a System message is actually present), which handles both the Agent shape [System, User, ...] and the swarm shape [User, agent, agent, ...]. [Bug] Return all conversation messages except the first #1857 proposed a flat [1:] and was closed — that would make every agent's default output echo the task back.
• Item 4 is the same defect as [Bug][Agent][self.executor is never assigned in __init__: run(imgs=[...]) raises AttributeError] #1793 at a second site. PR fix(agent): build a call-scoped pool for concurrent execution (#1793) #1909 removes the attribute entirely in favour of call-scoped pools; [Bug] self.executor is never assigned in __init__, so run(imgs=[...]) raises AttributeError (#1793) #1801 keeps it as a lazy property. Either closes both sites.
• Item 2 has no PR and is a one-line change.