Skip to content

fix: stale HookRef in TaskProcessor, and cmf vs. bare compatibility - #113

Open
tedhabeck wants to merge 23 commits into
0.1.xfrom
feat/python_plugin_compat_0.1.x
Open

fix: stale HookRef in TaskProcessor, and cmf vs. bare compatibility#113
tedhabeck wants to merge 23 commits into
0.1.xfrom
feat/python_plugin_compat_0.1.x

Conversation

@tedhabeck

@tedhabeck tedhabeck commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

On 0.1.x, declaring a plugin's manifest kind as a Python class path (e.g. package.module.ClassName) was how in-process Python plugins were declared. This PR makes such bare-FQN plugins run out-of-process by default: at install time the FQN kind is auto-converted to isolated_venv, so the plugin executes in a per-plugin virtual environment instead of in-process. The conversion requires no requirements.txt — the plugin's package is installed directly into the venv from its source.

What changed

Bare-FQN auto-conversion (default on)

  • New classify_plugin_kind() buckets a manifest kind into known / fqn / reject using purely shape-based detection (2+ dot-separated Python identifiers, final segment capitalized) — no import or venv needed.
  • convert_fqn_kind_in_place() moves the FQN into default_config.class_name and sets kind: isolated_venv. An explicit existing class_name wins (mismatches are logged).
  • Conversion runs on plugin install and plugin catalog update, and the converted form is persisted to plugin-manifest.yaml / plugins/config.yaml.
  • Unknown kinds recognized as neither a known kind nor a class-shaped FQN raise with a message naming the supported kinds.

--no-convert escape hatch

  • New flag on cpex plugin install (pypi/test-pypi/git/local) opts out of conversion, keeping the declared FQN kind so the plugin loads in-process — preserving 0.1.x behavior.
  • Also softens an unknown/unsupported kind from a hard error to a warning.

New framework surface

  • KNOWN_PLUGIN_KINDS and plugin-type constants (native, builtin, wasm, PDP, …) in constants.py.
  • PluginRegistry.get() to look up an installed plugin by name.
  • manifest_filename_for_class() util; manifests are now keyed on the full class name.

Fixes bundled in

  • Isolated worker: stale request_id — the worker reused a main()-local request_id across loop iterations, so an error raised before the next task parsed could be tagged with the previous request's id, misdelivering the error or hanging the real caller. The id is now reset per iteration. (Also hardened worker.py stdin reading: bad readline keyword, wrong Pydantic membership tests, oversized-line truncation, refactored into a tested read_task_line helper.)
  • install pkg@ wrongly skipped — the repeat-install check dropped the version constraint and compared against a possibly stale catalog entry; an explicit constraint now always proceeds.
  • Upgrade force-rebuilt every isolated_venv — the venv cache now treats a missing manifest version/hash signal as "no signal" rather than a mismatch, so pre-existing venvs aren't wiped on first run after upgrade.
  • Multi-plugin packages thrashing the venv cache — the persisted manifest is keyed on the plugin's full class name instead of the shared package root, so installing one plugin no longer invalidates a sibling's cache hash.
  • test-pypi isolated installs — now pass --extra-index-url https://pypi.org/simple/ so transitive deps (including cpex) resolve from real PyPI.

⚠️ Behavior change on upgrade

Because conversion is on by default, upgrading moves existing FQN-declared Python plugins from the in-process model to the out-of-process isolated_venv model. Install with --no-convert to keep the in-process behavior.### Checks

  • make lint passes
  • make test passes
  • CHANGELOG updated

@tedhabeck tedhabeck mentioned this pull request Jul 10, 2026
3 tasks
…s declared with a Python class path (e.g. pkg.module.ClassName) instead of a standard kind — by auto-converting them to isolated_venv plugins that install their package into the venv without requiring a requirements.txt. Work items are tagged U2/U4/U5/U6/R3 in the code comments.

Signed-off-by: habeck <habeck@us.ibm.com>
Signed-off-by: habeck <habeck@us.ibm.com>
Signed-off-by: habeck <habeck@us.ibm.com>
@tedhabeck
tedhabeck marked this pull request as ready for review July 21, 2026 01:52
tedhabeck added 13 commits July 22, 2026 17:32
Signed-off-by: habeck <habeck@us.ibm.com>
Signed-off-by: habeck <habeck@us.ibm.com>
…rong Pydantic membership tests, and added oversized-line truncation handling with prefix-length logging, refactored into a tested read_task_line helper

Signed-off-by: habeck <habeck@us.ibm.com>
Signed-off-by: habeck <habeck@us.ibm.com>
Signed-off-by: habeck <habeck@us.ibm.com>
Signed-off-by: habeck <habeck@us.ibm.com>
Signed-off-by: habeck <habeck@us.ibm.com>
Signed-off-by: habeck <habeck@us.ibm.com>
Signed-off-by: habeck <habeck@us.ibm.com>
Signed-off-by: habeck <habeck@us.ibm.com>
Signed-off-by: habeck <habeck@us.ibm.com>
Signed-off-by: habeck <habeck@us.ibm.com>
Signed-off-by: habeck <habeck@us.ibm.com>
Signed-off-by: habeck <habeck@us.ibm.com>
Signed-off-by: habeck <habeck@us.ibm.com>
Signed-off-by: habeck <habeck@us.ibm.com>
Signed-off-by: habeck <habeck@us.ibm.com>

@araujof araujof left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice work! The FQN conversion looks good. The PR also include several fixes to prior issues.

Minor Findings

  • Cache metadata is written before the package install. Order is persist manifest → initialize() (create venv → _save_cache_metadata) → catalog's _install_package_into_venv. Pre-PR, metadata was written only after install_requirements succeeded. Converted plugins have no requirements.txt, so that catalog-side pip install is the only thing making the venv usable — if it fails, the install errors but the cache reads valid, and at runtime initialize() skips everything and the worker can't import the class. Only an explicit reinstall heals it. Save metadata after the package install.
  • Changes in worker.py (commits 39a1f59, b6f9f7c, 919800f) add credential reconstruction, log/stdout/stderr/result scrubbing, Extensions reconstruction, and a capabilities handshake. None of it is in the PR description or CHANGELOG. It mutates process-global state (setLogRecordFactory, redirect_stdout/stderr) and is security-sensitive. Please document it in the PR and CHANGELOG.

Non-critical Findings

  • --no-convert silently no-ops for monorepo installs: _normalize_manifest_content converts unconditionally at catalog update, and the command refreshes the catalog before dispatch. Documented in the help text, but an accepted-and-ignored flag should warn at the call site.
  • _extract_credential_token accepts any non-empty token, so a 1–2 char token makes _result_contains_token's substring check fire spuriously and fail the task closed, and _scrub_token mangles every header; length guard it.
  • Credential present with payload absent is dropped silently — inconsistent with how loudly the rest of that path fails closed.
  • uv.lock bumps mcp 1.27.0 → 1.29.0 with no pyproject.toml change and no CHANGELOG entry. Two branch commits suggest a constraint edit that was reverted while the lock bump stayed. Legal under mcp>=1.26.0,<2, just unexplained.
  • CHANGELOG missing a blank line before ## [0.1.3].

Also, need to rebase from 0.1.x, and retarget this PR to 0.1.4.

@araujof araujof self-assigned this Aug 7, 2026
@araujof araujof added the enhancement New feature or request label Aug 7, 2026
@araujof araujof added the 0.1.x label Aug 7, 2026
@araujof araujof added this to CPEX Aug 7, 2026
@github-project-automation github-project-automation Bot moved this to Backlog in CPEX Aug 7, 2026
@araujof araujof moved this from Backlog to In review in CPEX Aug 7, 2026
@araujof araujof added this to the 0.1.4 milestone Aug 7, 2026
- Commit venv cache metadata only after the catalog-side package install
  succeeds. Converted bare-FQN plugins have no requirements.txt, so a failed
  package install previously left the cache reading valid while the venv was
  unusable, and only an explicit reinstall healed it.
- Guard credential-token substring use behind MIN_SCRUBBABLE_TOKEN_LENGTH (12)
  at the three consumers rather than at extraction, so a short token no longer
  fires _result_contains_token spuriously or mangles headers, while still being
  delivered to the plugin. _scrub_token now no-ops on an empty needle.
- Log a warning when a credential-bearing hook arrives with a credential but no
  payload, instead of dropping the credential silently.
- Warn when --no-convert is ignored on monorepo installs, naming the install
  types that honor it.
- Document the worker.py credential reconstruction, leak scrubbing, Extensions
  support, and capabilities handshake in the CHANGELOG, and note the uv.lock
  mcp 1.27.0 -> 1.29.0 resolution.
- Fix CHANGELOG blank lines around the 0.1.3/0.1.4 headings and add the 0.1.4
  compare link.
- Bump version to 0.1.4.

Signed-off-by: habeck <habeck@us.ibm.com>
@tedhabeck
tedhabeck requested a review from araujof August 7, 2026 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

0.1.x enhancement New feature or request

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

2 participants