Skip to content

Commit 4eda0d4

Browse files
Copilothuberp
Copilot
and
huberp
authored
docs: add LangGraph log protocol assessment report (#113)
Agent-Logs-Url: https://github.com/huberp/agentloop/sessions/5b15500a-a238-4222-a127-c62fadaa9060 Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: huberp <4027454+huberp@users.noreply.github.com>
1 parent d083063 commit 4eda0d4

1 file changed

Lines changed: 341 additions & 0 deletions

File tree

‎docs/langgraph-log-assessment.md‎

Lines changed: 341 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,341 @@
1+
# Agentloop LangGraph Log Protocol Assessment
2+
3+
**Reference:** `huberp/agentloop` · run `24935457632` · job `73020142658` step 6
4+
**Sorted by:** criticality (Critical → High → Medium → Low)
5+
6+
---
7+
8+
## CRITICAL
9+
10+
---
11+
12+
### C-1 · Explorer Prompt Hardcodes Multi-Language Config File List → Blind Probing of Non-Existent Files
13+
14+
**Log evidence** (lines 193, 197-204):
15+
```
16+
iteration 2: toolCallCount=11, toolCalls=[
17+
{file-read, package.json},
18+
{file-read, jest.e2e.config.js},
19+
{file-read, tsconfig.json},
20+
{file-read, Cargo.toml}, ← ENOENT
21+
{file-read, pyproject.toml}, ← ENOENT
22+
{file-read, go.mod}, ← ENOENT
23+
{file-read, build.gradle}, ← ENOENT
24+
{file-read, build.gradle.kts}, ← ENOENT
25+
{file-read, pom.xml}, ← ENOENT
26+
{file-read, CMakeLists.txt}, ← ENOENT
27+
{file-read, CMakePresets.json} ← ENOENT
28+
]
29+
```
30+
31+
7 out of 11 tool calls in the second explorer iteration are ENOENT errors. The file-list call in iteration 1 already provided a complete directory tree containing no Rust/Python/Go/Java/C++ artifacts whatsoever.
32+
33+
**Root cause** — `src/agents/project-explorer.ts:34-36`:
34+
```ts
35+
`2. Identify key project files (package.json, Cargo.toml, CMakeLists.txt, CMakePresets.json, ` +
36+
`build.gradle, build.gradle.kts, pom.xml, go.mod, pyproject.toml, requirements.txt, Makefile, etc.).\n` +
37+
`3. Call file-read on each identified key file…`
38+
```
39+
40+
The instruction says "identify… then read each identified key file" — but the LLM interprets this as "the list in step 2 is the identification; now read ALL of them", bypassing the file-list output entirely. The system prompt enumeration of every possible language's build file is treated as a mandatory checklist rather than an example of what to look for.
41+
42+
**Connection to user's observation:** The user noticed "file-list seems to list the files of the ignore list". The confusion arises because `DEFAULT_EXCLUDE_DIRS` in `file-list.ts` contains language-specific build artifact directories (`target` for Rust/Maven, `.venv` for Python, etc.) and the explorer system prompt enumerates the same language ecosystem's config files (`Cargo.toml`, `pyproject.toml`, etc.). The two lists are mirror images of each other — one lists excluded output directories, the other lists the corresponding source config files — making it look like the file-list result drove the probe attempts.
43+
44+
**Structural problem:** The explorer's system prompt enumerates filenames, not a detection strategy. The LLM lacks the reasoning step "check file-list output FIRST; only read files that APPEARED in the listing."
45+
46+
**Actionable fixes:**
47+
1. **Change the prompt instruction** from listing specific filenames to specifying the heuristic: *"Scan the file-list output for build system configuration files (e.g., files named `package.json`, `Cargo.toml`, `pom.xml`, etc.). Only call `file-read` on files that actually appeared in the file-list result."*
48+
2. **Add an explicit guard in the prompt:** `"Do NOT attempt to read a file unless you saw it in the file-list output."`
49+
3. **Medium-term fix:** Pre-filter the file-list result in `exploreWorkspace()` code before giving it to the LLM — pass only the list of known build-system config files that actually appear in the listing to a simpler prompt.
50+
51+
---
52+
53+
### C-2 · LLM Hallucinated Wrong Repository URL (Step s2)
54+
55+
**Log evidence** (lines 217-224):
56+
```
57+
iteration 1: shell → git clone https://github.com/alfred-ai-research/alfred.git .
58+
→ SCHEMA ERROR (array instead of string, see H-3)
59+
iteration 2: shell → git clone https://github.com/alfred-ai-research/alfred.git .
60+
→ "fatal: destination path '.' already exists and is not an empty directory."
61+
iteration 3: shell → ls -la
62+
iteration 4: output "The workspace already contains a repository…"
63+
```
64+
65+
The agent cloned `alfred-ai-research/alfred` — a completely unrelated repository from the LLM's training data — instead of the user's actual repo `huberp/agentloop`. The request said "add Anthropic models to **github repo huberp/agentloop**" but the step context for s2 only got the description "Clone the forked repository locally to the workspace" with no URL grounding.
66+
67+
**Root cause** — `src/langgraph/step-runner.ts:103-111`: The step system prompt is:
68+
```ts
69+
`Step: ${node.description}\n`
70+
```
71+
72+
The plan node's `description` is "Clone the forked repository locally to the workspace" — no URL, no repo name. The step LLM hallucinates a plausible-sounding URL from training memory.
73+
74+
The execution was "saved" only because the workspace was already populated (making git clone fail). In a clean CI environment or if the workspace had been different, the wrong repo would have been cloned and all subsequent file edits would corrupt it.
75+
76+
**Structural problem:** There is no mechanism to pass the original request's named entities (repo URL, specific versions, etc.) down to individual plan steps. The planner LLM knows the full request but each step only gets its `description` string. Critical parameters are lost at compile time.
77+
78+
**Actionable fixes:**
79+
1. **Inject the original `state.request`** into every step's system prompt: `"Original user request (for context): ${state.request}"`.
80+
2. **Require the planner** to embed concrete values (URLs, package names) in step descriptions: add to `BLOCKS_PLANNER_SYSTEM` — *"Include all concrete values (URLs, file paths, version numbers) needed to execute the step in the step description itself."*
81+
3. **Tool-level fix for shell:** Validate git clone target URLs against an allowlist or at least log a warning when a clone target is not the workspace's own remote.
82+
83+
---
84+
85+
### C-3 · Semantic Step Failures Silently Recorded as "Success" (Step s1)
86+
87+
**Log evidence** (lines 214-215):
88+
```
89+
iteration 1: toolCallCount=0, toolCalls=[]
90+
output: "I cannot directly fork a repository or perform GitHub actions like forking.
91+
However, you can manually fork the repository by following these steps:…"
92+
→ recorded as "success", graph proceeds to s2
93+
```
94+
95+
Step s1 was "Fork the huberp/agentloop repository". The LLM explicitly states it cannot perform this action and outputs human-directed instructions instead. No exception was raised, so `runSubagent()` completed normally. `runPlannedStep()` in `step-runner.ts` returns `{ status: "success", output: "I cannot…" }`. The graph records s1 as succeeded and all s2–sN proceed on the false premise that forking was done.
96+
97+
**Root cause** — `src/langgraph/step-runner.ts:130-136`:
98+
```ts
99+
return {
100+
status: "success",
101+
output: result.output,
102+
…
103+
};
104+
```
105+
106+
Success is defined as "no exception thrown". There is no semantic validation that the step actually accomplished anything. The text "I cannot" is treated identically to "Done."
107+
108+
**Structural problem:** The LangGraph execution model delegates success/failure entirely to exception propagation. LLMs that gracefully decline or produce partial outputs appear indistinguishable from steps that succeeded.
109+
110+
**Actionable fixes:**
111+
1. **Marker-based failure detection** (symmetric with the existing `REPLAN_MARKERS`): Add a `STEP_FAILED_MARKERS` list (`["I cannot", "I am unable", "I don't have the ability", "cannot perform"]`) and check the output — if matched, return `status: "failed"`.
112+
2. **Structured step output format:** Instruct steps to respond with JSON `{ "status": "done" | "failed" | "blocked", "summary": "…" }`. This is more reliable than marker scanning.
113+
3. **Self-evaluation pass:** After each step, run a quick LLM call that asks "Did this output complete the task described? Yes/No" — cheap single-turn with no tools.
114+
115+
---
116+
117+
## HIGH
118+
119+
---
120+
121+
### H-1 · Parallel Branches s4 + s5 Both Mutate `package.json` Concurrently Without Resource Locks
122+
123+
**Log evidence** (lines 230-240):
124+
```
125+
runnable: ["p1.b0.s4", "p1.b1.s5"] ← dispatched simultaneously
126+
127+
s4 (npm install @anthropic-ai/sdk):
128+
→ npm error ETARGET No matching version found for @anthropic-ai/sdk@^0.25.3
129+
→ retry: npm install @anthropic-ai/sdk@latest → installed
130+
131+
s5 (file-edit package.json, adding @anthropic-ai/sdk ^0.25.3):
132+
→ success (wrote stale/conflicting version string)
133+
```
134+
135+
Both steps ran in parallel and both modified `package.json`. `npm install @latest` rewrote the lockfile and `package.json` at the same time as `file-edit` inserted `^0.25.3`. The result is a `package.json` with `^0.25.3` in `dependencies` but `@latest` in `node_modules` (and possibly the lockfile) — an inconsistent state.
136+
137+
**Root cause:** The scheduler in `src/langgraph/scheduler.ts` has a correct `file:write:` locking mechanism, but it is entirely inert here because the planner LLM never emitted `resources: ["file:WRITE:package.json"]` hints in the plan. The scheduler can only protect what is declared.
138+
139+
**Structural problem:** Resource hints are purely advisory and LLM-generated. The planner has no enforced obligation to declare them, and the schema hint in `BLOCKS_PLAN_SCHEMA_HINT` only shows them as an example. The consequence is that the entire write-conflict protection system is silently bypassed whenever the planner omits `resources`.
140+
141+
**Actionable fixes:**
142+
1. **Add npm-install to serial-only steps:** Instruct the planner explicitly — *"Steps that run `npm install`, `pip install`, `cargo build`, or any package manager command must never be parallelised with steps that edit the corresponding manifest file."* Add this as a hard constraint in `BLOCKS_PLANNER_SYSTEM`.
143+
2. **Infer file locks from `toolsNeeded`:** In `compileBlocksPlanToDag`, if `toolsNeeded` contains `file-edit` or `file-write`, mark the step as potentially writing to files and prevent its parallelisation with npm-install steps automatically.
144+
3. **Post-plan validation:** After the planner produces the plan, validate that no parallel branch pair contains one npm-install-like step and one file-edit step without a declared resource lock, and trigger refinement if so.
145+
146+
---
147+
148+
### H-2 · `isDeadlocked` Imported but Never Called → Infinite Loop on Deadlock
149+
150+
**Code evidence** — `src/langgraph/graph.ts:32`:
151+
```ts
152+
import { selectRunnable, getCancellableForRace, isAllDone, isDeadlocked } from "./scheduler";
153+
```
154+
155+
`isDeadlocked` is imported but never used in any node function or routing condition. The conditional router after `handle_outcomes` is:
156+
```ts
157+
if (state.done) return "finalize";
158+
if (state.replanRequested) return "maybe_replan";
159+
return "select_runnable"; // ← always falls through here
160+
```
161+
162+
If a deadlock occurs (e.g., all pending nodes have dependencies on failed nodes, `replanRequested` is false, and `done` is false), the graph enters an infinite loop: `select_runnable → execute_batch (empty) → handle_outcomes → select_runnable…`.
163+
164+
**Actionable fixes:**
165+
1. **Use the already-imported function** in the routing conditional:
166+
```ts
167+
if (state.done) return "finalize";
168+
if (state.replanRequested) return "maybe_replan";
169+
if (isDeadlocked(state.compiledPlan!, state.records, {
170+
maxConcurrency: state.maxConcurrency,
171+
networkConcurrency: state.networkConcurrency
172+
})) {
173+
// set fatalError or trigger replan
174+
return "finalize";
175+
}
176+
return "select_runnable";
177+
```
178+
2. **Add a deadlock guard node** or guard in `handle_outcomes` that sets `done: true, fatalError: "deadlock"` when no progress is possible.
179+
180+
---
181+
182+
### H-3 · Shell Tool Schema Mismatch — LLM Repeatedly Passes Arrays
183+
184+
**Log evidence** (lines 218, 244):
185+
```
186+
s2 iteration 1: shell({ command: ["git", "clone", "…", "."] })
187+
→ "Expected string, received array → at command"
188+
→ retry needed
189+
190+
s7 iteration 1: shell({ command: ["mkdir", "-p", "src/models/anthropic"] })
191+
→ same schema error
192+
→ retry needed
193+
```
194+
195+
The shell tool schema clearly requires a string, but the LLM was trained on OpenAI-style function-calling conventions where many shell tools accept `["cmd", "arg1", "arg2"]` array form. This hit twice in a single run.
196+
197+
**Root cause** — `src/tools/shell.ts:20`:
198+
```ts
199+
command: z.string().describe("Shell command to execute (split by whitespace; no shell expansion)")
200+
```
201+
202+
The description says "split by whitespace" which is about how the command is _executed_, not the _input format_. This may confuse the LLM into thinking an array is accepted.
203+
204+
**Actionable fixes:**
205+
1. **Improve the schema description:** Change to `"A single string shell command, e.g. 'git clone https://… dest'. Do NOT pass an array."` — the negative example prevents array attempts.
206+
2. **Add a Zod preprocessor** that coerces arrays to joined strings: `z.preprocess((v) => Array.isArray(v) ? v.join(" ") : v, z.string())` — eliminates the need for a retry entirely.
207+
208+
---
209+
210+
## MEDIUM
211+
212+
---
213+
214+
### M-1 · `MemorySaver` and Graph Rebuilt from Scratch on Every `invokeGraph` Call
215+
216+
**Code location** — `src/langgraph/graph.ts:550-555`:
217+
```ts
218+
export async function invokeGraph(request, deps, opts) {
219+
const graph = buildGraph(deps, opts); // ← new StateGraph + new MemorySaver every call
220+
…
221+
}
222+
```
223+
224+
Each invocation creates a brand-new `StateGraph`, compiles it, and instantiates a new `MemorySaver`. The in-memory checkpointer is discarded when the function returns. This means:
225+
- A mid-execution crash loses all completed step results.
226+
- Multi-turn conversations can't resume from where they left off.
227+
- The `MemorySaver` checkpointing machinery is entirely wasted in practice.
228+
229+
**Actionable fixes:**
230+
1. **Hoist MemorySaver** to module scope (or inject via `GraphDeps`), so the compiled graph and checkpointer persist for the process lifetime.
231+
2. **Reuse the compiled graph** across calls by memoizing in `getActiveExecutor()` in `src/index.ts` (which already memoizes the executor adapter).
232+
3. **Longer term:** Switch to `SqliteSaver` or a persistent store for production checkpointing.
233+
234+
---
235+
236+
### M-2 · Step Output Truncated to 500 Characters in `sharedContext`
237+
238+
**Code location** — `src/langgraph/graph.ts:329`:
239+
```ts
240+
stepOutputs[r.nodeId] = r.output.slice(0, 500);
241+
```
242+
243+
When later steps reference prior step output (e.g., step s8 uses the output of s4 to determine what SDK version was installed), only the first 500 characters are available. Complex step outputs (TypeScript code, npm install summaries, file read results) routinely exceed this. The truncation is silent — the receiving step LLM gets no indication the context is incomplete.
244+
245+
**Actionable fixes:**
246+
1. **Increase the limit** or make it configurable via `appConfig`.
247+
2. **Add a truncation marker:** Append `"… [truncated]"` so downstream steps know the context is incomplete.
248+
3. **Structured output:** Instead of raw text, have steps output a JSON summary object — the LLM can then produce a compact structured output more reliably within budget.
249+
250+
---
251+
252+
### M-3 · Planner Has No Mechanism to Prevent "Fork/Clone" Class of Impossible Steps
253+
254+
**Log evidence** (lines 210-215): The planner generated a step "Fork the huberp/agentloop repository to create a personal copy" as a plan node with `toolsNeeded: ["web_fetch"]`. The planning agent had no means of knowing the executing agent can't actually make GitHub API calls to fork repositories.
255+
256+
**Root cause** — `src/langgraph/graph.ts:157`: The planner is only told `availableTools` names, not their capabilities or limitations. `web_fetch` is a tool for reading pages; the planner extrapolated it could also trigger GitHub fork actions.
257+
258+
**Actionable fixes:**
259+
1. **Add tool descriptions to the planner context**: Instead of just tool names, pass each tool's `description` field so the planner knows what each tool actually does.
260+
2. **Add a "capabilities" section to the planner prompt**: Explicitly state what the agent ecosystem cannot do (e.g., "Cannot create GitHub PRs, forks, or OAuth flows").
261+
262+
---
263+
264+
### M-4 · Resource Hint Case Mismatch Between Schema Example and Code
265+
266+
**Code locations:**
267+
- `src/langgraph/graph.ts:96`: schema hint shows `"resources": ["file:WRITE:path"]` (uppercase)
268+
- `src/langgraph/compiler.ts:177`: normalises to lowercase: `.trim().toLowerCase()`
269+
- `src/langgraph/scheduler.ts:22`: checks `r.startsWith("file:write:")` (lowercase)
270+
271+
The round-trip works by accident (normalisation fixes it), but the planner LLM is shown the uppercase form in its schema hint. If the normalisation step is ever removed or the check is updated carelessly, this silently breaks.
272+
273+
**Actionable fix:** Standardise to lowercase in the schema hint: `"resources": ["file:write:path"]` and document the convention.
274+
275+
---
276+
277+
## LOW
278+
279+
---
280+
281+
### L-1 · `parsePlanOutput` Is Brittle Against Trailing Code Fences in the Middle
282+
283+
**Code location** — `src/langgraph/graph.ts:141`:
284+
```ts
285+
const stripped = text.replace(/^```(?:json)?\s*/i, "").replace(/\s*```$/, "").trim();
286+
```
287+
288+
This only strips a single leading and trailing fence. If the LLM emits text before or after the JSON block, or uses nested fences, the `JSON.parse` will throw and trigger the planner error path.
289+
290+
**Actionable fix:** Use a more robust extraction that finds the first `{` and the matching last `}`:
291+
```ts
292+
const jsonStart = text.indexOf("{");
293+
const jsonEnd = text.lastIndexOf("}");
294+
JSON.parse(text.slice(jsonStart, jsonEnd + 1));
295+
```
296+
297+
---
298+
299+
### L-2 · Plan Version Check Rejects Any Variation of "2.0"
300+
301+
**Code location** — `src/langgraph/compiler.ts:30`:
302+
```ts
303+
if (p.version !== "2.0") throw new Error(`Unsupported plan version: ${String(p.version)}`);
304+
```
305+
306+
If the LLM emits `"version": "2"`, `"version": "v2.0"`, or even `"version": 2` (number), the plan is rejected. Since the planner prompt instructs `"version must be \"2.0\""`, this usually works, but a single hallucination here forces a full replan cycle.
307+
308+
**Actionable fix:** Accept `String(p.version).startsWith("2")` or coerce with `String(p.version).replace(/^v/, "")`.
309+
310+
---
311+
312+
### L-3 · `file-list` `target` in `DEFAULT_EXCLUDE_DIRS` Hides Valid Project Subdirectories
313+
314+
**Code location** — `src/tools/file-list.ts:19`:
315+
```ts
316+
"target", // Rust / Maven
317+
```
318+
319+
`target` is an extremely common directory name used by non-Maven/Cargo projects (e.g., `target/` as a docs output folder, Bazel's `bazel-out/target/`, some npm setups). Silently skipping it without any log warning makes it impossible to debug missing entries.
320+
321+
**Actionable fix:** Log a debug message when a directory is excluded: `logger.debug({ excluded: entry.name }, "file-list: skipping excluded directory")`. Consider making it opt-in via a `defaultExclude: false` flag.
322+
323+
---
324+
325+
## Summary Table
326+
327+
| ID | Severity | Area | Description |
328+
|----|----------|------|-------------|
329+
| C-1 | **Critical** | `project-explorer.ts` | Explorer blindly probes all multi-language config files regardless of file-list output → 7 ENOENT errors per run |
330+
| C-2 | **Critical** | `step-runner.ts` / planner | LLM hallucinated wrong repo URL (alfred.git) due to missing request context in step prompts |
331+
| C-3 | **Critical** | `step-runner.ts` | Semantic failures ("I cannot…") recorded as success; graph continues on false premises |
332+
| H-1 | **High** | `scheduler.ts` / planner | Parallel branches write same file without resource locks; LLM rarely emits `resources` hints |
333+
| H-2 | **High** | `graph.ts` | `isDeadlocked` imported but never used → infinite loop possible on deadlock |
334+
| H-3 | **High** | `shell.ts` | LLM repeatedly calls shell with array schema; schema description misleads; wastes iterations |
335+
| M-1 | **Medium** | `graph.ts` | Graph and MemorySaver rebuilt each invocation; checkpointing wasted |
336+
| M-2 | **Medium** | `graph.ts` | Step outputs truncated to 500 chars in sharedContext without marker |
337+
| M-3 | **Medium** | `graph.ts` planner | Planner not told tool limitations; generates unexecutable steps (GitHub fork, clone) |
338+
| M-4 | **Medium** | `compiler.ts` / `graph.ts` | Resource hint uppercase in schema but normalised to lowercase in code |
339+
| L-1 | **Low** | `graph.ts` | `parsePlanOutput` brittle against non-standard fence placement |
340+
| L-2 | **Low** | `compiler.ts` | Plan version check strict-equals "2.0" |
341+
| L-3 | **Low** | `file-list.ts` | `target` directory silently excluded with no diagnostic log |

0 commit comments

Comments
 (0)