Skip to content

context-engine: pass runtime context to ContextEngineFactory#67243

Open
jarimustonen wants to merge 15 commits intoopenclaw:mainfrom
jarimustonen:fix/context-engine-factory-context
Open

context-engine: pass runtime context to ContextEngineFactory#67243
jarimustonen wants to merge 15 commits intoopenclaw:mainfrom
jarimustonen:fix/context-engine-factory-context

Conversation

@jarimustonen
Copy link
Copy Markdown
Contributor

@jarimustonen jarimustonen commented Apr 15, 2026

Summary

  • Problem: ContextEngineFactory receives no parameters — plugins must use fragile workarounds (capture from first tool call, global mutable state, hardcoded fallback paths) to obtain runtime context like config, agentDir, and workspaceDir.
  • Why it matters: Context engine plugins need workspace and config information at construction time to initialize storage, resolve paths, etc.
  • What changed: ContextEngineFactory now receives a ContextEngineFactoryContext with config?, agentDir?, and workspaceDir?. resolveContextEngine() accepts optional ResolveContextEngineOptions and forwards them to the factory. All three resolution call sites (run, compact, subagent-registry) pass available context.
  • What did NOT change (scope boundary): The ContextEngine interface and its method signatures are unchanged. Existing no-arg factories continue to work because TypeScript permits assigning functions with fewer parameters to wider signatures.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor required for the fix
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

None.

Root Cause (if applicable)

N/A — this is a new feature.

Regression Test Plan (if applicable)

  • Coverage level that should have caught this:
    • Unit test
    • Seam / integration test
    • End-to-end test
    • Existing coverage already sufficient
  • Target test or file: src/context-engine/context-engine.test.ts
  • Scenario the test should lock in:
    • Factory receives ContextEngineFactoryContext with correct config, agentDir, workspaceDir
    • No-arg factories still work when context is passed
    • Undefined config is passed through honestly (no fake cast)
  • Why this is the smallest reliable guardrail: Direct unit tests on resolveContextEngine() cover the factory invocation contract without requiring full agent runtime.

User-visible / Behavior Changes

None — this is a plugin API addition. No user-facing behavior changes.

Diagram (if applicable)

Before:
resolveContextEngine(config) -> factory() -> ContextEngine

After:
resolveContextEngine(config, { agentDir, workspaceDir }) -> factory({ config, agentDir, workspaceDir }) -> ContextEngine

Security Impact (required)

  • New permissions/capabilities? No
  • Secrets/tokens handling changed? No
  • New/changed network calls? No
  • Command/tool execution surface changed? No
  • Data access scope changed? No

Repro + Verification

Environment

  • OS: macOS
  • Runtime/container: Node 22
  • Model/provider: N/A
  • Integration/channel (if any): N/A
  • Relevant config (redacted): N/A

Steps

  1. Register a context engine factory that accepts ContextEngineFactoryContext
  2. Call resolveContextEngine(config, { workspaceDir: "/some/path" })
  3. Verify factory receives the context with correct values

Expected

  • Factory receives { config, workspaceDir: "/some/path" }

Actual

  • Factory receives the expected context (verified via unit tests)

Evidence

  • Failing test/log before + passing after
  • Trace/log snippets
  • Screenshot/recording
  • Perf numbers (if relevant)

3 new tests in src/context-engine/context-engine.test.ts:

  • passes ContextEngineFactoryContext to factories that accept a parameter
  • no-arg factories still work when context is passed
  • passes undefined config when resolveContextEngine is called without config

All 39 tests pass.

Human Verification (required)

  • Verified scenarios: Tested with a plugin — workspaceDir is received correctly at factory construction time, no regression in existing behavior.
  • What you did not verify: Full end-to-end with third-party plugins, undefined config, no-arg factories, subagent-registry path

Review Conversations

  • I replied to or resolved every bot review conversation I addressed in this PR.
  • I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

  • Backward compatible? Yes — existing no-arg factories work unchanged
  • Config/env changes? No
  • Migration needed? No

Risks and Mitigations

  • Risk: Plugin SDK API surface change (new exported types ContextEngineFactoryContext, updated ContextEngineFactory signature)
    • Mitigation: Additive change only. Existing factories are assignable to the new type via TypeScript function parameter contravariance. Plugin SDK API baseline hash updated.

@openclaw-barnacle openclaw-barnacle bot added docs Improvements or additions to documentation agents Agent runtime and tooling size: S labels Apr 15, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 15, 2026

Greptile Summary

Passes a new ContextEngineFactoryContext (config, agentDir, workspaceDir) to context engine factories at resolution time, removing the need for plugins to use global state or first-call capture workarounds. The change is additive and backward-compatible — existing no-arg factories remain assignable to the updated ContextEngineFactory type via TypeScript's structural function subtyping. All three call sites (run, compact.queued, subagent-registry) forward available context; agentDir is absent on the subagent-ended path because it is not stored in SubagentRunRecord, which is expected given the agentDir? optional type.

Confidence Score: 5/5

Safe to merge — no P0/P1 issues; all findings are style and docs suggestions.

The implementation is clean, backward-compatible, and covered by three targeted unit tests. The two comments are both P2: a minor type inconsistency in an injected dep signature and missing doc examples for the new context parameter. Neither blocks correct runtime behavior.

No files require special attention; the minor dep-type inconsistency in src/agents/subagent-registry.ts (lines 80–83 and 109–112) is the only thing worth a second look.

Comments Outside Diff (1)

  1. docs/concepts/context-engine.md, line 121 (link)

    P2 Doc example doesn't show new factory context parameter

    The example factory still uses the no-arg form, so plugin authors won't discover ContextEngineFactoryContext without reading source. Now that ctx carries config, agentDir, and workspaceDir, a short snippet showing the new parameter would help new engine authors initialise storage at construction time — exactly the use-case driving this change. The same applies to the examples in docs/plugins/architecture.md (lines 1526 and 1558).

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: docs/concepts/context-engine.md
    Line: 121
    
    Comment:
    **Doc example doesn't show new factory context parameter**
    
    The example factory still uses the no-arg form, so plugin authors won't discover `ContextEngineFactoryContext` without reading source. Now that `ctx` carries `config`, `agentDir`, and `workspaceDir`, a short snippet showing the new parameter would help new engine authors initialise storage at construction time — exactly the use-case driving this change. The same applies to the examples in `docs/plugins/architecture.md` (lines 1526 and 1558).
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: src/agents/subagent-registry.ts
Line: 80-83

Comment:
**Type signature inconsistency with actual function**

The injectable `resolveContextEngine` dep is typed with a required `cfg: OpenClawConfig`, but the canonical `resolveContextEngine` function signature uses `config?: OpenClawConfig` (optional). While TypeScript accepts the assignment (optional satisfies required), the mismatch could confuse test authors injecting a mock and mean the type won't catch callers that pass `undefined` through this path.

```suggestion
  resolveContextEngine?: (
    cfg?: OpenClawConfig,
    options?: ResolveContextEngineOptions,
  ) => Promise<ContextEngine>;
```

The mirrored type declaration at line 109–112 (`ContextEngineRegistryModule`) has the same inconsistency and should be updated together.

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

---

This is a comment left during a code review.
Path: docs/concepts/context-engine.md
Line: 121

Comment:
**Doc example doesn't show new factory context parameter**

The example factory still uses the no-arg form, so plugin authors won't discover `ContextEngineFactoryContext` without reading source. Now that `ctx` carries `config`, `agentDir`, and `workspaceDir`, a short snippet showing the new parameter would help new engine authors initialise storage at construction time — exactly the use-case driving this change. The same applies to the examples in `docs/plugins/architecture.md` (lines 1526 and 1558).

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

Reviews (1): Last reviewed commit: "context-engine: regenerate plugin SDK AP..." | Re-trigger Greptile

Comment thread src/agents/subagent-registry.ts
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8dba5fb7cb

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/agents/pi-embedded-runner/run.ts
Comment thread src/agents/pi-embedded-runner/compact.queued.ts Outdated
@jarimustonen jarimustonen force-pushed the fix/context-engine-factory-context branch from 7728db5 to 07df597 Compare April 16, 2026 05:00
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 07df5971f6

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/agents/subagent-registry.ts
@jalehman jalehman self-assigned this Apr 20, 2026
@jalehman jalehman force-pushed the fix/context-engine-factory-context branch 2 times, most recently from f06a50f to 361c0f0 Compare April 20, 2026 22:31
@jalehman jalehman force-pushed the fix/context-engine-factory-context branch from 361c0f0 to 6ae3ec4 Compare April 20, 2026 22:32
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6ae3ec4500

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +513 to +517
const factoryCtx: ContextEngineFactoryContext = {
config,
agentDir: options?.agentDir,
workspaceDir: options?.workspaceDir,
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Pass an isolated factory context to fallback engine

resolveContextEngine constructs one factoryCtx object and reuses that same object when invoking both the configured engine factory and the default fallback factory. On fallback paths, a failing non-default factory can mutate ctx in place (for example while normalizing paths) before throwing/returning invalid data, and the default engine will then initialize from those mutated values instead of the original runtime context. This affects recovery scenarios directly, so each factory invocation should get its own immutable/independent context object.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling docs Improvements or additions to documentation size: M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants