feat(portable): complete Agent Plugins 1.0.0 adoption — manifest metadata, pinned byte lane, doctor findings (#307) - #373
Conversation
🦋 Changeset detectedLatest commit: 066ed4c The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
commit: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70e53441d6
ℹ️ 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".
| const reports = await Promise.all(validated.snapshot.manifest.targets | ||
| .filter((target) => | ||
| target.name === 'claude' || target.name === 'codex' || target.name === 'cursor' || target.name === 'plugin') | ||
| .map((target) => target.name === 'codex' | ||
| ? validateCodexPlugin({ | ||
| pluginDirectory: join(artifact, target.name), | ||
| strict: options.strict, | ||
| target: target.name, | ||
| }) | ||
| : target.name === 'cursor' | ||
| ? validateCursorPlugin({ | ||
| pluginDirectory: join(artifact, target.name), | ||
| target: target.name, | ||
| }) | ||
| : validateClaudePlugin({ | ||
| pluginDirectory: join(artifact, target.name), | ||
| strict: options.strict, | ||
| target: target.name, | ||
| }))); | ||
| .map((target) => target.name) | ||
| .filter(isHostValidatedTarget) | ||
| .map((target) => hostValidationReport(target, join(artifact, target), options.strict))); |
There was a problem hiding this comment.
Run the portable byte lane during ordinary builds
The new normative validator is invoked only inside the hostValidation === true path. Consequently, a normal build or validate --artifact still accepts and publishes portable documents containing values that the byte lane rejects, such as command: "bin/server", an escaping cwd, duplicate case-insensitive headers, or a non-loopback HTTP URL. Wire these checks into portable planning or ordinary artifact validation so standard-invalid output fails before publication rather than only when users opt into --host-validation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed — fixed in #406. The §7.2.1/§9.2 rules are extracted into packages/agent-bundle/src/adapters/portable-mcp-rules.ts and applied (1) at plan time in portable.ts as portable.mcp.{command,cwd,env,url,headers}.standard errors, so a standard-invalid server never reaches an artifact, and (2) as the full byte lane (AB6035–AB6037) over every emitted portable/ tree inside ordinary validateArtifact, which build and validate --artifact both run. Tests: portable-adapter.test.ts "fails closed at plan time on Agent Plugins 1.0.0 normative MCP rules the schema cannot express" (covers bin/server, bun run, escaping cwd, env-key placeholder, plain HTTP off loopback, duplicate case-insensitive headers, control-char header value) and artifact-validator.test.ts "fails ordinary artifact validation when an emitted portable tree breaks the Agent Plugins normative text" (validateArtifact without host validation returns four AB6036 errors).
There was a problem hiding this comment.
Fixed on main by #406 (98751df, the #307 lane): the §7.2.1/§9.2 rules are applied at plan time (portable.mcp.*.standard) and the full byte lane (AB6035–AB6037) runs over every emitted portable/ tree during ordinary artifact validation, so build/validate --artifact fail closed without --host-validation. #408 carried the same artifact-time wiring and dropped it in favour of #406 during rebase.
| target, | ||
| )); | ||
| } | ||
| if (typeof value === 'string' && /[\r\n\0]/u.test(value)) { |
There was a problem hiding this comment.
Reject all forbidden HTTP header control characters
For remote servers whose header value contains another forbidden control character, such as \u0001, vertical tab, or DEL, this condition reports no diagnostic even though such bytes are invalid HTTP field values and are rejected by HTTP clients (including Node's validateHeaderValue). Check the full forbidden control-character range while continuing to permit horizontal tab where applicable.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed — fixed in #406. Header values now reject every character outside VCHAR / SP / HTAB / obs-text (/[^\t\u0020-\u007E\u0080-\u00FF]/, RFC 9110 §5.5, matching Node's validateHeaderValue) rather than only CR/LF/NUL. Test: portable-plugin-validation.test.ts "rejects every forbidden control character in HTTP header values while permitting horizontal tab" (SOH, BEL, LF, VT, CR, DEL, NUL and U+2014 rejected; HTAB, obs-text and visible ASCII accepted), plus the same value rule at plan time in portable-adapter.test.ts.
There was a problem hiding this comment.
Fixed on main by #406 (98751df, the #307 lane): header values now reject every character outside VCHAR / SP / HTAB / obs-text (RFC 9110 §5.5, matching Node's validateHeaderValue), so \x01, VT, and DEL all produce the §7.2.1 field-value diagnostic. #408 carried an equivalent control-character check and dropped it in favour of #406 during rebase.
40242eb to
81f2538
Compare
…data, pinned byte lane, doctor findings (#307) Author the standard's §5.4 manifest metadata and §5.6 extensions under the portable config key and emit them into root plugin.json; add the pinned Agent Plugins byte lane (AB6035–AB6038) to validate --host-validation, to doctor for installed Cursor local plugins declaring the standard's $schema (AB7320), and to the portable host-install proof; record dated capability rows for every standard feature; re-verify schema pins; adapterRevision 1.5.0 → 1.6.0.
…t from an installed tarball
81f2538 to
066ed4c
Compare
#379/#385/#390 - dev: lease the adopted epoch in EpochAdoptionPolicy until replaced or closed; select the contract-matrix target from the server's own target list; apply the session timeout per matrix request; observe lifecycle progress through the session trace via a new ContractMatrixClient.observeProgress seam (#385) - playground: wait for a hard-link catalog publisher to release its staging link before adopting the sidecar; return to discovery when the publication is rolled back (#377) - build: run the Agent Plugins byte lane over portable/ during ordinary artifact validation; reject every forbidden control character in header values (#373) - events/hooks: Codex PostToolUse accepts any present JSON tool_response (#378) - api: project only contract fields of adapter capability rows in inspect (#390) - tests/support: digest the real Claude home in the live session guard; isolate USERPROFILE alongside HOME (#374) - docs: Claude plugin-root cwd exception, parked-pin trigger independence, provider typing contract, portable validation moments (#368/#379/#382/#373)
#379/#385/#390 - dev: lease the adopted epoch in EpochAdoptionPolicy until replaced or closed; select the contract-matrix target from the server's own target list; apply the session timeout per matrix request; observe lifecycle progress through the session trace via a new ContractMatrixClient.observeProgress seam (#385) - playground: wait for a hard-link catalog publisher to release its staging link before adopting the sidecar; return to discovery when the publication is rolled back (#377) - build: run the Agent Plugins byte lane over portable/ during ordinary artifact validation; reject every forbidden control character in header values (#373) - events/hooks: Codex PostToolUse accepts any present JSON tool_response (#378) - api: project only contract fields of adapter capability rows in inspect (#390) - tests/support: digest the real Claude home in the live session guard; isolate USERPROFILE alongside HOME (#374) - docs: Claude plugin-root cwd exception, parked-pin trigger independence, provider typing contract, portable validation moments (#368/#379/#382/#373)
#379/#385/#390 - dev: lease the adopted epoch in EpochAdoptionPolicy until replaced or closed; select the contract-matrix target from the server's own target list; apply the session timeout per matrix request; observe lifecycle progress through the session trace via a new ContractMatrixClient.observeProgress seam (#385) - playground: wait for a hard-link catalog publisher to release its staging link before adopting the sidecar; return to discovery when the publication is rolled back (#377) - build: run the Agent Plugins byte lane over portable/ during ordinary artifact validation; reject every forbidden control character in header values (#373) - events/hooks: Codex PostToolUse accepts any present JSON tool_response (#378) - api: project only contract fields of adapter capability rows in inspect (#390) - tests/support: digest the real Claude home in the live session guard; isolate USERPROFILE alongside HOME (#374) - docs: Claude plugin-root cwd exception, parked-pin trigger independence, provider typing contract, portable validation moments (#368/#379/#382/#373)
#379/#385/#390 - dev: lease the adopted epoch in EpochAdoptionPolicy until replaced or closed; select the contract-matrix target from the server's own target list; apply the session timeout per matrix request; observe lifecycle progress through the session trace via a new ContractMatrixClient.observeProgress seam (#385) - playground: wait for a hard-link catalog publisher to release its staging link before adopting the sidecar; return to discovery when the publication is rolled back (#377) - build: run the Agent Plugins byte lane over portable/ during ordinary artifact validation; reject every forbidden control character in header values (#373) - events/hooks: Codex PostToolUse accepts any present JSON tool_response (#378) - api: project only contract fields of adapter capability rows in inspect (#390) - tests/support: digest the real Claude home in the live session guard; isolate USERPROFILE alongside HOME (#374) - docs: Claude plugin-root cwd exception, parked-pin trigger independence, provider typing contract, portable validation moments (#368/#379/#382/#373)
#379/#385/#390 - dev: lease the adopted epoch in EpochAdoptionPolicy until replaced or closed; select the contract-matrix target from the server's own target list; apply the session timeout per matrix request; observe lifecycle progress through the session trace via a new ContractMatrixClient.observeProgress seam (#385) - playground: wait for a hard-link catalog publisher to release its staging link before adopting the sidecar; return to discovery when the publication is rolled back (#377) - build: run the Agent Plugins byte lane over portable/ during ordinary artifact validation; reject every forbidden control character in header values (#373) - events/hooks: Codex PostToolUse accepts any present JSON tool_response (#378) - api: project only contract fields of adapter capability rows in inspect (#390) - tests/support: digest the real Claude home in the live session guard; isolate USERPROFILE alongside HOME (#374) - docs: Claude plugin-root cwd exception, parked-pin trigger independence, provider typing contract, portable validation moments (#368/#379/#382/#373)
…dinary builds (#307 review follow-up) Address the two Codex review threads on #373: - P1: the normative byte lane only ran under `--host-validation`, so `build` and `validate --artifact` still published standard-invalid portable output. Extract the pure §7.2.1/§9.2 rules into `adapters/portable-mcp-rules.ts`, apply them at plan time (`portable.mcp.{command,cwd,env,url,headers}.standard`), and run the full byte lane (AB6035–AB6037) over every emitted `portable/` tree inside ordinary artifact validation. - P2: header values only rejected CR/LF/NUL; now reject every character outside VCHAR / SP / HTAB / obs-text (RFC 9110 §5.5, matching Node's validateHeaderValue). The mcp-session-service remote fixture carried a header placeholder the standard forbids; it now uses a literal token and asserts pass-through.
…dinary builds (#307 review follow-up) (#406) * fix(portable): fail closed on Agent Plugins normative MCP rules in ordinary builds (#307 review follow-up) Address the two Codex review threads on #373: - P1: the normative byte lane only ran under `--host-validation`, so `build` and `validate --artifact` still published standard-invalid portable output. Extract the pure §7.2.1/§9.2 rules into `adapters/portable-mcp-rules.ts`, apply them at plan time (`portable.mcp.{command,cwd,env,url,headers}.standard`), and run the full byte lane (AB6035–AB6037) over every emitted `portable/` tree inside ordinary artifact validation. - P2: header values only rejected CR/LF/NUL; now reject every character outside VCHAR / SP / HTAB / obs-text (RFC 9110 §5.5, matching Node's validateHeaderValue). The mcp-session-service remote fixture carried a header placeholder the standard forbids; it now uses a literal token and asserts pass-through. * test(packed): use a bare node command in the hand-written portable MCP fixture The packed consumer fixture wrote command: process.execPath into portable mcp.json; Agent Plugins §7.2.1 forbids absolute command paths and the build now fails closed on it (AB6036). Match the compiler's bare 'node' emission. * fix(validate-artifact): key the portable byte lane on the built-in adapter identity, not the target name An advanced TargetRegistry may bind 'portable' to its own adapter and artifactValidation contract; that output is not Agent Plugins 1.0.0 and must not be reported as AB6035. Compare registry.get(name) against portableAdapter. * fix(portable): make command/cwd containment platform-independent and correct the release note Refuse backslashes and NUL in ./ commands and cwd values and normalize with POSIX semantics only, so a bundle built on POSIX cannot resolve outside the plugin root on Windows. The changeset wrongly said publication was blocked only without --host-validation; the byte lane runs in every ordinary build. * test(artifact): pin that --host-validation never duplicates a failing portable byte lane Host reports only run over an artifact the ordinary lane accepted, so an AB6035-AB6037 failure is reported once and no portable report is produced. * fix(portable): check containment on the normalized relative path and skip the byte lane over refused filesystem entries - ./../anchor/server and ${PLUGIN_ROOT}/../anchor collided with the synthetic anchor and passed; containment now rejects a normalized path that starts with '..' (shared by planner and byte lane). - validate-artifact skips the byte lane for a portable tree whose inspection already recorded a symlink or other unsupported entry (AB6013), so tampered documents are never stat'ed or read before their containment is reported. * fix(portable): reject drive-relative Windows commands such as C:server win32.isAbsolute reports them relative, but they resolve against a per-drive working directory on a Windows consumer; treat any nonempty win32 root as rooted.
#379/#385/#390 - dev: lease the adopted epoch in EpochAdoptionPolicy until replaced or closed; select the contract-matrix target from the server's own target list; apply the session timeout per matrix request; observe lifecycle progress through the session trace via a new ContractMatrixClient.observeProgress seam (#385) - playground: wait for a hard-link catalog publisher to release its staging link before adopting the sidecar; return to discovery when the publication is rolled back (#377) - build: run the Agent Plugins byte lane over portable/ during ordinary artifact validation; reject every forbidden control character in header values (#373) - events/hooks: Codex PostToolUse accepts any present JSON tool_response (#378) - api: project only contract fields of adapter capability rows in inspect (#390) - tests/support: digest the real Claude home in the live session guard; isolate USERPROFILE alongside HOME (#374) - docs: Claude plugin-root cwd exception, parked-pin trigger independence, provider typing contract, portable validation moments (#368/#379/#382/#373)
#379/#385/#390 - dev: lease the adopted epoch in EpochAdoptionPolicy until replaced or closed; select the contract-matrix target from the server's own target list; apply the session timeout per matrix request; observe lifecycle progress through the session trace via a new ContractMatrixClient.observeProgress seam (#385) - playground: wait for a hard-link catalog publisher to release its staging link before adopting the sidecar; return to discovery when the publication is rolled back (#377) - build: run the Agent Plugins byte lane over portable/ during ordinary artifact validation; reject every forbidden control character in header values (#373) - events/hooks: Codex PostToolUse accepts any present JSON tool_response (#378) - api: project only contract fields of adapter capability rows in inspect (#390) - tests/support: digest the real Claude home in the live session guard; isolate USERPROFILE alongside HOME (#374) - docs: Claude plugin-root cwd exception, parked-pin trigger independence, provider typing contract, portable validation moments (#368/#379/#382/#373)
#379/#385/#390 - dev: lease the adopted epoch in EpochAdoptionPolicy until replaced or closed; select the contract-matrix target from the server's own target list; apply the session timeout per matrix request; observe lifecycle progress through the session trace via a new ContractMatrixClient.observeProgress seam (#385) - playground: wait for a hard-link catalog publisher to release its staging link before adopting the sidecar; return to discovery when the publication is rolled back (#377) - build: run the Agent Plugins byte lane over portable/ during ordinary artifact validation; reject every forbidden control character in header values (#373) - events/hooks: Codex PostToolUse accepts any present JSON tool_response (#378) - api: project only contract fields of adapter capability rows in inspect (#390) - tests/support: digest the real Claude home in the live session guard; isolate USERPROFILE alongside HOME (#374) - docs: Claude plugin-root cwd exception, parked-pin trigger independence, provider typing contract, portable validation moments (#368/#379/#382/#373)
#379/#385/#390 - dev: lease the adopted epoch in EpochAdoptionPolicy until replaced or closed; select the contract-matrix target from the server's own target list; apply the session timeout per matrix request; observe lifecycle progress through the session trace via a new ContractMatrixClient.observeProgress seam (#385) - playground: wait for a hard-link catalog publisher to release its staging link before adopting the sidecar; return to discovery when the publication is rolled back (#377) - build: run the Agent Plugins byte lane over portable/ during ordinary artifact validation; reject every forbidden control character in header values (#373) - events/hooks: Codex PostToolUse accepts any present JSON tool_response (#378) - api: project only contract fields of adapter capability rows in inspect (#390) - tests/support: digest the real Claude home in the live session guard; isolate USERPROFILE alongside HOME (#374) - docs: Claude plugin-root cwd exception, parked-pin trigger independence, provider typing contract, portable validation moments (#368/#379/#382/#373)
#379/#385/#390 - dev: lease the adopted epoch in EpochAdoptionPolicy until replaced or closed; select the contract-matrix target from the server's own target list; apply the session timeout per matrix request; observe lifecycle progress through the session trace via a new ContractMatrixClient.observeProgress seam (#385) - playground: wait for a hard-link catalog publisher to release its staging link before adopting the sidecar; return to discovery when the publication is rolled back (#377) - build: run the Agent Plugins byte lane over portable/ during ordinary artifact validation; reject every forbidden control character in header values (#373) - events/hooks: Codex PostToolUse accepts any present JSON tool_response (#378) - api: project only contract fields of adapter capability rows in inspect (#390) - tests/support: digest the real Claude home in the live session guard; isolate USERPROFILE alongside HOME (#374) - docs: Claude plugin-root cwd exception, parked-pin trigger independence, provider typing contract, portable validation moments (#368/#379/#382/#373)
/#379/#385/#390) (#408) * fix: address late review threads on merged PRs #368/#373/#374/#377/#378/#379/#385/#390 - dev: lease the adopted epoch in EpochAdoptionPolicy until replaced or closed; select the contract-matrix target from the server's own target list; apply the session timeout per matrix request; observe lifecycle progress through the session trace via a new ContractMatrixClient.observeProgress seam (#385) - playground: wait for a hard-link catalog publisher to release its staging link before adopting the sidecar; return to discovery when the publication is rolled back (#377) - build: run the Agent Plugins byte lane over portable/ during ordinary artifact validation; reject every forbidden control character in header values (#373) - events/hooks: Codex PostToolUse accepts any present JSON tool_response (#378) - api: project only contract fields of adapter capability rows in inspect (#390) - tests/support: digest the real Claude home in the live session guard; isolate USERPROFILE alongside HOME (#374) - docs: Claude plugin-root cwd exception, parked-pin trigger independence, provider typing contract, portable validation moments (#368/#379/#382/#373) * fix(dev): lease before publishing contract status; align fixtures with the portable byte lane - EpochAdoptionPolicy acquires the epoch lease before publishing a passed dev.contract.status and announces adoption synchronously with it, so a status reader never sees "passed" for an epoch that is not yet adopted - hooks.test: Codex PostToolUse accepts a string tool_response and rejects a missing one; Claude keeps the object check - mcp-session-service/public-api-packed fixtures: Agent Plugins forbids placeholders in headers and non-bare/non-./ commands, and ordinary artifact validation now enforces the standard, so the fixtures carry a literal header and a bare `node` command * chore(changeset): drop the Codex tool_response bullet already released by #404 * fix(playground,test): withdraw a failed catalog publication before releasing its staging link; bind custom observeProgress - #persistSnapshot rolls the sidecar back while the staging link still exists when the post-link directory fsync fails, so a concurrent reader keeps seeing an in-progress publication until the path is withdrawn instead of adopting a briefly singly linked file - contractProgressObserver invokes a client's observeProgress method with the client as receiver * chore(changeset): one-paragraph summary ending with the PR reference * fix(playground): recover a catalog staging link abandoned by an exited publisher After the settle deadline, a matching .stage-<pid>-* link whose publisher pid no longer exists is an abandoned publication of an already fsynced sidecar: withdraw the orphan and adopt the sidecar instead of rejecting the epoch forever. A live publisher's staging link is still never yanked. Adds the @internal catalogStagingSettleDeadlineMs seam for deterministic tests. * fix(playground): fsync the catalog directory after withdrawing an abandoned staging link * fix(dev): forward request _meta (progress token) through McpSession and the dev matrix client Lifecycle fixtures pass their generated progressToken as params._meta; the session adapter and McpSession.callTool dropped it, so generated routes never enabled sendProgress and every progress-gated lifecycle fixture failed the dev matrix. McpSessionToolCallOptions and McpClient.callTool now carry _meta. * fix(dev,playground): restart the adoption drain after a handoff race; keep the staging link when a sidecar rollback fails - EpochAdoptionPolicy reschedules its drain from the completion handler when a candidate arrived between the loop's last empty check and #processing being cleared, and settled() waits through restarts - #persistSnapshot releases the staging link after a failed publication only once the owned sidecar is confirmed withdrawn, so a rollback failure never leaves a singly linked sidecar for readers to adopt * ci: retrigger checks for the rebased head * chore: drop the portable byte-lane changes superseded by #406; keep the _meta assertions * fix(playground): restore the staging guard when a recovery fsync fails * fix(playground): accept a concurrently restored staging guard (EEXIST aliasing the sidecar) during recovery * fix(dev): recheck supersession after the adoption lease settles * fix(playground): keep a fresh pid-owned guard when recovery can neither re-link nor withdraw the sidecar * fix(playground): fsync every compensating recovery guard before trusting it
Summary
Closes the remaining #307 deliverables for the
portabletarget as the pinned Agent Plugins 1.0.0 adapter (PR #306 landed the pin, the installer proof, and the Cursor IDE dogfood):author,homepage,repository,license,keywords, and reverse-domainextensionsare authored under theportableconfig key and emitted into the rootplugin.json. Omitted fields leave the manifest byte-identical to the previous contract (test-pinned), so no previously emitted path or field changes; malformed values fail closed withportable.manifest.<field>.invalid.AB6035–AB6038): newhost-contracts/portable-plugin-validation.tsvalidates bytes at rest against the pinned schemas and the normative text the schemas cannot express (§10.1 version agreement, §7.2.1 command/cwd/URL/header forms, §9.2 placeholder scope, §6.2 fixed-location kinds, §7.1 skill layout, §4.1 symlink containment). Wired intovalidate --artifact --host-validation(newportablereport inValidateResult.hostValidation) and intodoctor: installed Cursor local plugins whose rootplugin.jsondeclares an Agent Plugins$schemaget the contract underAB7320(error →corrupt), replacing the previous "no pinned contract for that flavor" info for that case (feat(doctor): surface static bytes-at-rest validation findings (#133) #340 pattern).portable-1.0.0.json:manifestMetadata/extensionssupported,extensionDirectoriesandlegacySseunavailable with dated reasons; adapter capabilities expose the same rows.agent-plugins.org/schemas/1.0.0/*bytes rehash to the pinned sha256s; spec repo HEAD unchanged atff8ab5e39; a 1.1.0 working draft exists upstream without published schemas (recorded).docs/diagnostics.md(new AB6035–AB6038 section, AB7320 row), README portable section,docs/framework-mode.md; changeset (minor); portable adapterRevision 1.5.0 → 1.6.0 with pins updated.Evidence
contract: 'agent-plugins-1.0.0 byte lane clean (AB6035–AB6037)').packages/agent-bundle/tests/portable-plugin-validation.test.ts: conformant bundle passes; schema rejections; 15 normative-text rules; wrong filesystem kinds; skill dirs without SKILL.md; escaping and dangling symlinks.doctor.test.ts: Agent Plugins-flavored install →installed+ info; schema/normative violations →corrupt+AB7320retainingAB6035/AB6036.Test plan
pnpm build(publint passed)pnpm typecheckpnpm lint— 0 errors, 0 warningspnpm test:unit— 2676 passed; 2 load-induced timing failures (mcp-probe-service150 ms guard,native-claude-contract5 s timeout) pass on rerun / withAGENT_BUNDLE_TEST_TIME_SCALEunder load avg 170; neither touches this changehost-install-proof.test.tsportable proof +cli.test.tspass; the Codex host-install proof fails locally on a pre-existinginterfaceFieldsdrift (logoadded by fix(adapters): third-wave review findings (#346/#350/#352/#354/#357/#358) #364; CI skips it without a Codex CLI) — out of this lane's scope, reported on the issue