Greptile Summary

This PR adds a backfillSessionKey() helper that resolves a sessionKey from the session store by sessionId at the top of runEmbeddedPiAgent(), ensuring hooks, LCM, and compaction receive a non-null key when callers omit it. The fix is narrow, correctly guarded, and has no behavior change for callers that already supply sessionKey.

Confidence Score: 5/5

Safe to merge — the fix is a well-scoped read-only lookup with a no-op fallback; no behavioral regression for existing callers.

All remaining findings are P2 style suggestions (silent catch, redundant fallback operand). No correctness or data-integrity issues were found.

No files require special attention.

This is a comment left during a code review.
Path: src/agents/pi-embedded-runner/run.ts
Line: 121-123

Comment:
**Silent error swallowing without any log**

If `resolveSessionKeyForRequest` throws unexpectedly (corrupt session store, permission error, etc.) the failure is invisible — the function returns `undefined` and the caller never knows the backfill was skipped. At minimum a `debug`/`warn` log would make future incidents diagnosable.

```suggestion
  } catch (err) {
    log.warn(
      `[backfillSessionKey] Failed to resolve sessionKey for sessionId=${params.sessionId}: ${describeUnknownError(err)}`,
    );
    return params.sessionKey;
  }
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/agents/pi-embedded-runner/run.ts
Line: 120

Comment:
**Redundant fallback operand**

`params.sessionKey` is guaranteed to be `undefined` or an empty string at this point (the early-return guard on line 112 already handles any truthy value). The `?? params.sessionKey` tail is therefore a no-op and adds a small amount of confusion — `resolved.sessionKey` alone is clearer.

```suggestion
    return resolved.sessionKey;
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix(agents): backfill sessionKey in runE..." | Re-trigger Greptile}

Pushed fixes for all review feedback:

P1 (agentId binding): Removed agentId from the resolveSessionKeyForRequest() call entirely. The resolver now looks up purely by sessionId, avoiding resolveExplicitAgentSessionKey() which would bind one-off callers (model probes, etc.) to the agent's main session. Good catch — this was a real correctness issue.

P2 (silent catch): Replaced bare catch with log.warn() using sanitizeForLog() and describeUnknownError() (both already imported in this file).

P2 (redundant fallback): Removed the ?? params.sessionKey tail — it was unreachable after the early-return guard.

P2 (whitespace normalization): Now normalizes with .trim() || undefined at entry and on the resolved value, so downstream consumers never see whitespace-only keys.

Docstring: Updated to reflect best-effort semantics.

On the test coverage request: happy to add a regression test if you can point me at the right mock setup for resolveSessionKeyForRequest() / session store in the embedded runner test suite. I'll submit a follow-up commit.

CI note: the checks-fast-extensions failure is unrelated to this PR — it's Mattermost reconnect test timeouts (extensions/mattermost/src/mattermost/reconnect.test.ts). There's already an upstream commit (1322aa2, test(ci): make mattermost reconnect deterministic) addressing that flaky suite.

This PR only modifies src/agents/pi-embedded-runner/run.ts.

@jalehman can you merge in. This quick fix enable LCM's to get the correct session key again - upstream fix of LCM patch already- need upstream to give LCM the data now to filter out subs and junk:
https://github.com/Martian-Engineering/lossless-claw/releases/tag/v0.6.0

Review feedback addressed in commit 8374c2ef:

Greptile P2: silent error swallowing → Fixed. Added log.warn in catch block so resolution failures are visible.

Greptile P2: redundant fallback operand → Fixed. Simplified to use resolved/trimmed key flow.

Copilot: whitespace-only sessionKey propagation → Fixed. Now returns trimmed/resolved value, not original.

Copilot: misleading docstring → Fixed. Updated to accurately describe resolver behavior.

Codex P1: avoid forcing agent main key during backfill → Fixed. Resolver now uses sessionId alone, not agentId, so it does not bias toward agent:<agent>:explicit:... when lookup misses.

Copilot: no regression test → Fixed. Added tests for:

• successful sessionId → sessionKey backfill
• warning log emission when backfill resolution throws

All 8 review items addressed.

Merged via squash.

• Prepared head SHA: 8081345f1c2191cb988072b565f6860e2d00047b
• Merge commit: 594ea6e1b970ec54875a6450bf2dbc5d2e4b5296

Thanks @100yenadmin!