Skip to content

fix(codex): expose plugin apps after delayed inventory load#96872

Merged
kevinlin-openai merged 2 commits into
mainfrom
dev/kevinlin/refresh-missing-codex-app-inventory
Jun 26, 2026
Merged

fix(codex): expose plugin apps after delayed inventory load#96872
kevinlin-openai merged 2 commits into
mainfrom
dev/kevinlin/refresh-missing-codex-app-inventory

Conversation

@kevinslin

@kevinslin kevinslin commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Fixes an issue where native Codex plugin apps could be omitted from a newly started OpenClaw thread when the initial app inventory was missing and plugin activation state changed before thread configuration was built.

Why This Change Was Made

OpenClaw now remembers when the initial inventory refresh was deferred for plugin activation. If activation does not already require a post-install refresh, it force-refreshes app/list, rereads plugin ownership and readiness, and keeps failed activations excluded. The existing install-before-inventory ordering remains unchanged.

User Impact

Configured, accessible plugin apps remain available when a thread starts during this activation-state transition instead of silently disappearing from that thread's Codex app configuration.

Evidence

  • Added a regression that reproduces the transition from activation-required to already-active while inventory is missing; it failed before the fix and now verifies a forced refresh plus app exposure.
  • node scripts/run-vitest.mjs extensions/codex/src/app-server/plugin-thread-config.test.ts extensions/codex/src/app-server/plugin-activation.test.ts extensions/codex/src/app-server/thread-lifecycle.binding.test.ts — 3 files, 65 tests passed.
  • Targeted oxfmt and oxlint checks passed.
  • The local staged changed-gate was unable to complete dependency reconciliation because the internal npm mirror returned HTTP 403 without an authorization header; clean PR CI is the authoritative exact-commit gate.

@clawsweeper

clawsweeper Bot commented Jun 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 25, 2026, 9:06 PM ET / 01:06 UTC.

Summary
The PR force-refreshes missing Codex app inventory after plugin activation-state transitions, enables configured accessible plugin apps per thread even when app/list reports them disabled, and adds docs plus regression tests.

PR surface: Source +26, Tests +346, Docs +10. Total +382 across 5 files.

Reproducibility: yes. at source level. Current main suppresses missing app-inventory refresh while activation is required and also requires app.enabled, so the delayed-inventory and disabled-app omission paths are clear without a live repro run.

Review metrics: 1 noteworthy metric.

  • App enablement predicates: 2 narrowed. The branch removes Codex isEnabled from both app exposure and not-ready refresh decisions, which changes how existing Codex app-disabled settings interact with OpenClaw plugin config.

Stored data model
Persistent data-model change detected: vector/embedding metadata: docs/plugins/codex-harness-reference.md, vector/embedding metadata: docs/plugins/codex-harness.md. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Preserve explicit Codex per-app disables or get explicit maintainer acceptance that OpenClaw config overrides them per thread.
  • [P1] Add redacted real startup proof showing the configured plugin app present in the after-fix thread config after the activation-state transition.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body lists tests and lint only; before merge it needs redacted terminal output, logs, live output, or a linked artifact from a real OpenClaw/Codex startup showing the configured plugin app in thread/start.config.apps after the transition, with private details such as tokens, IPs, phone numbers, and non-public endpoints redacted. Updating the PR body should trigger a fresh ClawSweeper review; if it does not, a maintainer can comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] Merging can enable a configured plugin-owned app for a thread even when Codex reports isEnabled=false, and Codex does not expose whether that false value came from a base default or an explicit per-app disable.
  • [P1] The PR body still has test/lint proof only; it needs redacted real OpenClaw/Codex startup output or an artifact showing the configured app present in thread/start.config.apps after the fixed transition.
  • [P1] The disabled-base-default behavior overlaps fix(codex): allow configured apps with disabled base defaults #95970, so maintainers should decide whether this PR, that PR, or a split branch owns the policy.

Maintainer options:

  1. Preserve Explicit App Disables (recommended)
    Keep explicit Codex per-app enabled=false fail-closed, or add a verified signal/opt-in that separates disabled defaults from app-specific disables before merging the enablement override.
  2. Accept OpenClaw Config Precedence
    Maintainers may intentionally decide that OpenClaw codexPlugins configuration overrides Codex disabled app state for the thread, but that decision should be explicit and backed by real startup proof.
  3. Split Inventory From Policy
    Merge the delayed missing-inventory refresh separately and leave the disabled-base-default policy to fix(codex): allow configured apps with disabled base defaults #95970 or a narrower follow-up.

Next step before merge

  • [P1] Protected maintainer label, missing real proof, and the Codex disabled-app precedence decision require human maintainer handling rather than an automated repair lane.

Security
Needs attention: The diff can weaken a Codex app/tool access boundary by overriding app-disabled config state for configured plugin apps.

Review findings

  • [P1] Preserve explicit Codex app disables — extensions/codex/src/app-server/plugin-thread-config.ts:445
Review details

Best possible solution:

Land the delayed-inventory repair only with real startup proof and either preserve explicit Codex app disables by default or get maintainer acceptance that OpenClaw plugin config intentionally overrides Codex app-disabled state per thread.

Do we have a high-confidence way to reproduce the issue?

Yes, at source level. Current main suppresses missing app-inventory refresh while activation is required and also requires app.enabled, so the delayed-inventory and disabled-app omission paths are clear without a live repro run.

Is this the best way to solve the issue?

No, not yet. The delayed-inventory refresh is in the right owner layer, but the enablement override is not the safest complete fix until explicit Codex app disables remain protected or maintainers accept OpenClaw per-thread precedence.

Full review comments:

  • [P1] Preserve explicit Codex app disables — extensions/codex/src/app-server/plugin-thread-config.ts:445
    AppInfo.isEnabled is Codex config state, and Codex computes it from both [apps._default] and explicit [apps.<id>] overrides. Returning only app.accessible lets OpenClaw send thread/start.config.apps[appId].enabled = true even when an operator disabled that exact app, so the patch can bypass an existing app/tool access setting instead of only fixing disabled defaults.
    Confidence: 0.9

Overall correctness: patch is incorrect
Overall confidence: 0.9

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against a0b397748fa3.

Label changes

Label changes:

  • add merge-risk: 🚨 compatibility: Merging changes behavior for existing users who configured an OpenClaw Codex plugin while the corresponding Codex app reports disabled.
  • add merge-risk: 🚨 security-boundary: Ignoring isEnabled=false can expose app tools in a thread despite a Codex per-app disabled setting, affecting a tool-access boundary.

Label justifications:

  • P2: This is a focused Codex plugin runtime bug fix with limited blast radius, but it can omit or expose configured plugin apps for affected Codex threads.
  • merge-risk: 🚨 compatibility: Merging changes behavior for existing users who configured an OpenClaw Codex plugin while the corresponding Codex app reports disabled.
  • merge-risk: 🚨 security-boundary: Ignoring isEnabled=false can expose app tools in a thread despite a Codex per-app disabled setting, affecting a tool-access boundary.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body lists tests and lint only; before merge it needs redacted terminal output, logs, live output, or a linked artifact from a real OpenClaw/Codex startup showing the configured plugin app in thread/start.config.apps after the transition, with private details such as tokens, IPs, phone numbers, and non-public endpoints redacted. Updating the PR body should trigger a fresh ClawSweeper review; if it does not, a maintainer can comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +26, Tests +346, Docs +10. Total +382 across 5 files.

View PR surface stats
Area Files Added Removed Net
Source 1 38 12 +26
Tests 2 351 5 +346
Docs 2 14 4 +10
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 5 403 21 +382

Security concerns:

  • [medium] Explicit app disable can be bypassed — extensions/codex/src/app-server/plugin-thread-config.ts:445
    Because Codex reports both default-disabled and explicitly app-disabled state through isEnabled, the new readiness check can force-enable a plugin-owned app for the thread even when the operator disabled that app in Codex config.
    Confidence: 0.88

What I checked:

Likely related people:

  • kevinslin: Merged PR history shows this person authored native Codex plugin app support and the later remote app-server plugin/app inventory refresh work in the same files and contracts. (role: feature owner and recent area contributor; confidence: high; commits: 9792fa19539c, 181347a8e90a, eaf8c7e7503c; files: extensions/codex/src/app-server/plugin-thread-config.ts, extensions/codex/src/app-server/plugin-inventory.ts, extensions/codex/src/app-server/app-inventory-cache.ts)
  • obviyus: Current-main blame for the central Codex plugin app-thread files points at a broad merged commit committed by this person, though the associated PR title is not Codex-specific, so this is weaker routing evidence. (role: recent merger/committer signal; confidence: low; commits: ecd29fe57269; files: extensions/codex/src/app-server/plugin-thread-config.ts, extensions/codex/src/app-server/plugin-inventory.ts, extensions/codex/src/app-server/app-inventory-cache.ts)
  • sheyanmin: GitHub associated current-main blamed commit ecd29fe with a merged PR by this person, but the PR's changed-file metadata is gateway-focused, so the ownership signal is low confidence. (role: adjacent merged PR author signal; confidence: low; commits: ecd29fe57269; files: extensions/codex/src/app-server/plugin-thread-config.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. labels Jun 26, 2026
@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation size: M and removed size: S labels Jun 26, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jun 26, 2026
@kevinlin-openai
kevinlin-openai merged commit a6a4652 into main Jun 26, 2026
128 of 134 checks passed
@kevinlin-openai
kevinlin-openai deleted the dev/kevinlin/refresh-missing-codex-app-inventory branch June 26, 2026 04:10
@github-actions github-actions Bot mentioned this pull request Jun 26, 2026
Open
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jun 26, 2026
@kevinslin
fix(codex): expose plugin apps after delayed inventory load (openclaw…
7bf29f1 
…#96872)

* fix(codex): refresh missing plugin app inventory

* fix(codex): honor OpenClaw app enablement overrides
@github-actions github-actions Bot mentioned this pull request Jun 26, 2026
Open
@clawsweeper clawsweeper Bot mentioned this pull request Jun 26, 2026
Merged
1 task
QiuYuang pushed a commit to QiuYuang/openclaw that referenced this pull request Jul 1, 2026
@kevinslin @QiuYuang
fix(codex): expose plugin apps after delayed inventory load (openclaw…
fc4c526 
…#96872)

* fix(codex): refresh missing plugin app inventory

* fix(codex): honor OpenClaw app enablement overrides
@clawsweeper clawsweeper Bot mentioned this pull request Jul 2, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@kevinslin kevinslin

Labels

docs Improvements or additions to documentation extensions: codex maintainer Maintainer-authored PR merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: M status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@kevinslin @kevinlin-openai