Add codex-harness-patterns plugin (v1.0.4, 23 Skills) - #33
Merged
hetaoBackend merged 3 commits intoSep 10, 2026
Merged
Conversation
… a fresh PR ## What This is a **replacement PR** for the abandoned PR MiniMax-AI#18 ("Add codex-harness-patterns plugin"). PR MiniMax-AI#18 was opened with `head = base = main` (head SHA `9600667e`, a merge commit on the `origin/main` branch that also accumulated 51 other commits spanning MiniMax-AI#5 tool-map, MiniMax-AI#17 mcode-island, and the proposal hooks). That state, plus a 51-commit / 168-file drift vs. `upstream/main`, made the PR effectively unreviewable: every reviewer comment touched a different change than the one the PR was nominally about, and `update-branch` returned `expected head sha didn't match current head ref` (HTTP 422). amszuidas' round-8 P2 (2026-09-07T03:17:13Z, head `9600667e`) captured this exactly: "Please rebase after those dependencies merge, or remove the unrelated changes so this PR can be reviewed and merged as the codex-harness-patterns contribution alone." This commit is the second half of that recommendation: it ships the codex-harness-patterns contribution as a single, self-contained commit on a fresh branch `add-codex-harness-patterns` based on `upstream/main`. The old PR MiniMax-AI#18 will be closed without merge; this PR is the one that should be reviewed. ## Why a single commit, not 30 The original 30-commit history (`fb0b87d` v0.3.0 origin through `c949789` v1.0.4 final, post-round-7) is available on the abandoned PR MiniMax-AI#18 branch. I attempted to cherry-pick those 30 commits onto a fresh `add-codex-harness-patterns` branch (preserving the review trail commit-by-commit), and got 30 modify/delete conflicts in a row: each commit is a "create + immediately modify" of the same files relative to the empty upstream, which git's 3-way merge does not handle in a non-interactive batch. Resolving 30 conflicts by hand does not preserve the review trail any better than a single squash, and is much noisier to audit. The end state of `c949789` (post-round-7) is what the reviewers ultimately agreed on; the 14 round-by-round commits are an artefact of the review process, not of the contribution itself. If a reviewer needs to see the round-by-round reasoning, the old PR MiniMax-AI#18's commit list, review comments, and round-by-round explanations remain at `https://github.com/MiniMax-AI/MiniMax-Code-Plugins/pull/18` after this PR replaces it. The abandoned PR is referenced explicitly in the new PR's body (see "Replacement of" below). ## What this PR contains Exactly two paths: - `plugins/antianqi/codex-harness-patterns/` — the plugin itself: `plugin.json` + `LICENSE` + `README.md` (4-section disclosure: no credentials / no network / no telemetry / no third-party services) + `OVERVIEW.md` + `PR-STATUS.md` + 23 Skills. - `test/codex-harness-patterns.test.mjs` — the static-check companion to the plugin (closes round-4 false-green holes, pins mcode 0.2.4 task contract, asserts minMcodeVersion). No other files change. The 30-commit PR MiniMax-AI#18 also touched `.github/workflows/`, `test-fixtures/`, `plugins/antianqi/tool-map/`, `plugins/antianqi/mcode-island/`, etc. — none of that is in this PR. ## Skill list (v1.0.4) background-task, completion-audit, context-pressure-compact, delegate-with-context, error-recovery-strategy, fork-context-decision, goal-persistence, goal-token-budgeting, long-term-memory, model-router, parallel-fanout, plan-stream-emit, plugin-author-helper, retry-with-backoff, review-mode, session-branch-fork, session-handoff, skill-auto-select, streaming-output-reader, subagent-family-tracking, tool-discovery-pattern, tool-output-budget, world-state-tracking. ## Test evidence ``` $ git checkout c949789 -- plugins/antianqi/codex-harness-patterns test/codex-harness-patterns.test.mjs $ git diff --cached --name-only | wc -l 29 $ ls plugins/antianqi/codex-harness-patterns/skills | wc -l 23 ``` `plugins/antianqi/codex-harness-patterns/scripts/smoke.mjs` (static-check that runs as part of `npm run check` on this plugin) will run under the `validate (windows-latest)` / `validate (ubuntu-latest)` jobs in this PR's CI matrix. ## Design compliance - **One Plugin, one commit, one branch, one PR.** The entire contribution is `plugins/antianqi/codex-harness-patterns/` (the plugin) plus `test/codex-harness-patterns.test.mjs` (its static-check companion). 29 files total. No workflow changes, no shared-test changes, no scope creep into other plugins. (The old PR MiniMax-AI#18 also touched `.github/workflows/`, `test-fixtures/`, and other plugins; those are explicitly NOT in this PR.) - **No credentials, no network, no telemetry, no third-party services.** All 23 Skills are local prompts and references; no HTTP, no token, no telemetry endpoint. - **README 4-section disclosure (v1.0.2, round-3):** `plugins/antianqi/codex-harness-patterns/README.md` contains a "What this Plugin does NOT do" section explicitly listing the four absences above, following the mcode plugin convention introduced by the round-2 / round-3 disclosure review. - **Replacement of** PR MiniMax-AI#18 (the 51-commit / 168-file / `head=base=main` / "rebase or remove the unrelated changes" one). The old PR's commit list and review history are preserved on the closed PR for audit.
hetaoBackend
requested changes
Sep 9, 2026
hetaoBackend
left a comment
Collaborator
There was a problem hiding this comment.
Current head 5a4e3fc fixes the earlier unsupported plugin.json requirements field and its CI/CodeQL are green, but the package still documents unverified host behavior as executable guidance. skills/background-task/SKILL.md:70-84,213-233 invents PID/job-control behavior for bash(run_in_background=true) without a public MiniMax Code contract or test proving the returned handle has a PID; goal-persistence, long-term-memory, session-handoff, session-branch-fork, and subagent-family-tracking similarly prescribe host paths/parameters such as ~/.minimax/memory, ephemeral/no_collab/no_network, and .minimax/agents/ without verified APIs. The package also ships no assets/agent manifests despite fork-context-decision claiming assets/agents//agent.md. Please label unsupported sections as conceptual pseudocode or rewrite them against verified public APIs, remove invented PID/path/parameter claims, and add artifact-completeness/API-vocabulary tests. Local focused tests pass 162/162, but they do not validate those host contracts. [code]smith is SKIPPED.
…tual pseudocode (PR MiniMax-AI#33 round-12) Round-12 review from hetaoBackend (2026-09-09T01:04:18Z on head 5a4e3fc) flagged that the package documents unverified host behavior as executable guidance across 7 Skills, and asked for either rewriting against verified public APIs, or labelling unsupported sections as conceptual pseudocode, plus artifact-completeness / API-vocabulary tests. This commit is a v1.0.5 amendment. ## Round-12 defects addressed 1. background-task SKILL.md:70-84, 213-233 previously treated the `bash(run_in_background=true)` handle as a raw integer PID and passed it directly to `Stop-Process -Id 12345` / `kill 12345` in two prose passages and the shell-background example. mcode 0.2.4 does not document the `bash` job-handle shape, does not document a kill action, and does not document that the handle exposes a process id. Replaced every literal integer PID with a `<process-id>` placeholder, added a CONCEPTUAL PSEUDOCODE banner to the example, and rewrote the `compatibility` frontmatter field to state that the host's job-control API is the source of truth for the underlying process id. 2. long-term-memory SKILL.md:45-46 pinned a `~/.minimax/memory/` path that is not part of the mcode 0.2.4 contract, and the Phase-2 spawn block presented Codex-internal flags (`ephemeral`, `no_collab`, `no_network`, `no_memory_tool`, `features.disable(Collab / MemoryTool / Apps / Plugins)`, `approval_policy = Never`, `network_access = false`, `memories.generate_memories = false`, `use_memories = false`, `redact_secrets`) as if they were mcode surface. Wrapped the on-disk layout in a `<host-memory-root>/...` placeholder with an `illustrative; actual path is host-internal` annotation, added a CONCEPTUAL PSEUDOCODE (Codex reference, codex-rs/memories/) banner to the example block, and labelled every fenced code block that mentions a Codex-internal flag as conceptual pseudocode. The `compatibility` frontmatter field now states the conceptual-reference nature of the Skill. 3. session-handoff SKILL.md pinned `.minimax/handoff/...`, `.minimax/goal/...`, `.minimax/state/...`, `.minimax/family/...` paths as if they were mcode 0.2.4 contract paths. Wrapped every host-side on-disk path in a `<host-X-root>/...` placeholder and rewrote the `compatibility` frontmatter field. 4. session-branch-fork SKILL.md is a literal mirror of `codex-rs/thread-store/`: Rust types (ThreadHistoryMode / RolloutLineageSegment / ForkBoundary / PreparedFork / StoredModelContext / ReverseJsonlScanner / ModelContextScan), SQLite CAS on `rollout_path`, `writer_lock_coordinator`, lease helpers, `Op::SuspendTurnAndShutdown` / `Op::RecoverTurn`, `JOB_LEASE_SECONDS` / `GRACEFULL_INTERRUPTION_TIMEOUT_MS` / `MAX_ROLLOUT_LINE_BYTES`, `TaskKind::Regular` / `Feature::Collab` / `PermissionProfile::External` — none of which are on the mcode 0.2.4 public surface. Added a banner at the top of the Skill explicitly labelling every block as Codex-internal pseudocode, and rewrote the `compatibility` frontmatter field to state the conceptual-reference nature. 5. subagent-family-tracking SKILL.md pinned `.minimax/agents/<thread-id>/subagents.md` as an mcode contract path. Wrapped the path in a `<host-agents-root>/<thread-id>/subagents.md` placeholder and rewrote the `compatibility` frontmatter field. 6. goal-persistence SKILL.md pinned `.minimax/goal/<YYYY-MM-DD>-<short-id>.md` as an mcode contract path. Wrapped the path in a `<host-goal-root>/...` placeholder. 7. fork-context-decision SKILL.md:47-65 said the on-disk path of each sub-agent's manifest is host-internal. The package itself ships NO sub-agent manifests, but the phrasing did not make that explicit. Rewrote the paragraph to make it explicit that this plugin package ships no sub-agent manifests, that the `assets/agents/<name>/agent.md` layout referenced in the table is the mcode 0.2.4 host install's layout (verified on the maintainer's install; may vary across installs and platforms), and that the Skills rely on the `agent_name` parameter for sub-agent selection, NOT on any on-disk manifest lookup from this plugin's package. ## artifact-completeness / API-vocabulary tests added Six new test cases in test/codex-harness-patterns.test.mjs, each with a negative fixture and a Skill-body assertion: - literal-integer PIDs in `bash(...)` code blocks are detected - background-task SKILL.md bash(...) examples use `<process-id>` placeholder, not literal PIDs - literal `.minimax/<X>/...` host paths in Skill bodies are detected (negative fixture) - 5 Skills (`goal-persistence` / `long-term-memory` / `session-handoff` / `session-branch-fork` / `subagent-family-tracking`) use `<host-X-root>/...` placeholders, not literal `.minimax/<X>/...` paths - long-term-memory labels Codex-internal flags as conceptual pseudocode (negative fixture) - long-term-memory SKILL.md labels Codex-internal flags as conceptual pseudocode ## Validation test/codex-harness-patterns.test.mjs: 40 pass / 0 fail, duration ~100ms (node --test). The new tests are fail-closed: any of the round-12 defects above will turn at least one test red. The parser also gains a UTF-8 BOM tolerance (some Skills, e.g. session-handoff, were originally authored with a BOM and the previous frontmatter uniqueness check refused to read them). ## Test evidence (negative-injection audit) Each of the three round-12 contract violations was injected into the working tree, the suite was re-run, and the file was restored from a backup. The restore step is wrapped in a finally block so a script crash cannot leave the working tree polluted. Step 0 (baseline): tests=40 pass=40 fail=0 Step 1 (literal PID injection): tests=40 pass=39 fail=1 Step 1 (restored): tests=40 pass=40 fail=0 Step 2 (literal .minimax/... inj): tests=40 pass=39 fail=1 Step 2 (restored): tests=40 pass=40 fail=0 Step 3 (no conceptual tag inj): tests=40 pass=39 fail=1 Step 3 (restored): tests=40 pass=40 fail=0 ## Design compliance - Skill-only plugin (no mcp.json, no package.json, 0 npm dependencies). The plugin itself ships no agent manifests (see fork-context-decision paragraph 7 above). - README / SKILL.md disclosure: the four-section no-credentials / no-network / no-telemetry / no-third-party-services disclosure is unchanged from v1.0.4. - Cross-platform path resolution: every host-side on-disk path is now wrapped in a `<host-X-root>/...` placeholder; no literal `D:\` / `C:\` / `/Users/` / `/home/` strings in the round-12 affected Skills. - Atomic write: no new code in this commit performs file writes; the change is documentation + test only. - One commit / one branch / one plugin. The 7 Skills and the test file are committed together because the round-12 contract spans all 7 Skills; splitting them would leave intermediate states in which the new tests fire on half-amended Skills. - No unrelated hardening. The UTF-8 BOM tolerance added to the existing parseFrontmatter helper is the minimum parser-side guard required to make the new artifact-completeness / API-vocabulary tests compatible with the existing frontmatter uniqueness check on Skills that were originally authored with a BOM (e.g. session-handoff); the helper otherwise behaves identically.
Contributor
Author
|
PR #33 round-12 (v1.0.5 amendment, head Thanks for the round-12 review. This is a v1.0.5 amendment that Per-Skill changes (7 files, +484/-51 lines):
Tests (
Each negative fixture is followed by a skill-body assertion that Validation:
Disclosure unchanged: the four-section Re-requesting review. |
…L.md (CI validate) The round-12 commit 918e2b2 left a UTF-8 BOM (U+FEFF) at the very start of `plugins/antianqi/codex-harness-patterns/skills/ session-handoff/SKILL.md`. CI `npm run check` (which calls `node scripts/validate.mjs`, which calls `validateSkillText` in `scripts/lib/validation.mjs:71`) asserts that the file starts with the literal byte sequence `---\n`. The BOM is byte 0, so `text.startsWith('---\n')` is false and the validator reports "YAML frontmatter is required". This is the only file in the repository that triggers this assertion; 126 other SKILL.md files do not have a BOM. This commit drops the 3 BOM bytes; the body content is unchanged. ## Why the BOM was there in the first place The repository-side `scripts/lib/validation.mjs:38` rejects BOM-prefixed JSON (`assert(!text.startsWith('\uFEFF'), ...)`), so the validator policy is consistent: no BOM, anywhere. The file was originally authored with a BOM (it predates the round-12 round-1 amendment) and the round-12 commit re-serialized it through PowerShell 5.1 `Set-Content -Encoding UTF8`, which adds a BOM (`[System.Text.Encoding]::UTF8` in .NET Framework 4.x is BOM-encoded). The round-12 commit inherited the BOM, and CI failed. ## Validation `node scripts/validate.mjs` now reports: Validated 4 hosted Plugins and all examples. (node --test still reports 40 pass / 0 fail for `test/codex-harness-patterns.test.mjs`; the new parseFrontmatter BOM-tolerance added in 918e2b2 means test-side reading is unaffected.) ## Design compliance - One commit / one fix. This commit is a doc-only file-hygiene change, isolated from the round-12 contract changes in 918e2b2. - No content drift: the diff is `±1 line` (the BOM line), the body is byte-identical to the version in 918e2b2 minus the 3-byte BOM prefix. - Cross-platform: removing the BOM means the file now passes the validator on Linux CI (where `git checkout` already normalises line endings to LF) and on Windows dev boxes (where `git checkout` produces CRLF and the validator expected `---\n` regardless).
hetaoBackend
approved these changes
Sep 10, 2026
hetaoBackend
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed against the exact current head a8bd7de.
Approval basis:
- plugin.json uses the supported manifest fields and declares Apache-2.0.
- The round-12 changes label the previously unsupported host-internal surfaces as conceptual/reference material and use the canonical mcode task/tool vocabulary.
- The focused suite passes (62/62) and the repository validator passes for the current head.
- No unresolved review threads remain, and the server reports MERGEABLE/CLEAN.
The workspace-relative .minimax/state and .minimax/snapshots conventions are not treated as host API claims; the host-internal paths called out in the prior review are now placeholder/conceptualized. This approval is for the exact head above only.
weekbin
added a commit
to weekbin/MiniMax-Code-Plugins
that referenced
this pull request
Sep 10, 2026
…audit This commit hardens the Plugin to the MiniMax Marketplace submission guide + the historical PR review pattern from MiniMax-Code-Plugins (PRs MiniMax-AI#4, MiniMax-AI#21, MiniMax-AI#33, MiniMax-AI#35, MiniMax-AI#37): - Add `.minimax-plugin/plugin.json` mirror: schemaVersion, displayName, string author, icon, category ("Other"), 3 exampleQueries, apps / mcpServers (empty arrays), skills (1 entry). The closed-schema `plugin.json` cannot carry these fields, so they live in a sibling Marketplace manifest. - Add `icon.png` (1080x1080 RGBA, 457 KB) per the user-provided art. - Extend `plugin.json` with `homepage` and `repository` (string fields, both pointing to the fork's plugin path), and `author.email`. - Rewrite the description in user-facing language ("Generate ... GIF memes from a scene description") instead of the previous internal-implementation trigger phrasing. - Add README "What this Plugin does NOT do" 4-section disclosure (no credentials / no network / no telemetry / no third-party services) and a `minMcodeVersion: 0.2.0` requirement line. - Add `license` and `metadata` keys to SKILL.md frontmatter. - Add `tests/plugins/octopus-meme-maker/smoke.test.mjs` with 20 static checks + 5 negative-injection tests. The negative tests cover the false-green holes called out in PR MiniMax-AI#21 round-4 / MiniMax-AI#33 round-4: every static check must detect its own broken input. The path sweep now covers all `.md` / `.py` / `.json` in the plugin, not just SKILL.md and the marketplace JSON, so future regressions cannot slip through. - Drop a duplicate `## Data and network` heading that was left behind by an earlier edit. - Replace hardcoded `04-lying-flat/` and `~/Works/octopus-worker-meme/` in published docs with `<scene-dir>/` and `<works>/octopus-worker-meme/` placeholders to honor the no-host-literal-paths rule. Verified: `npm run check` 221/221 pass; skill-review audit 100% PASS.
weekbin
added a commit
to weekbin/MiniMax-Code-Plugins
that referenced
this pull request
Sep 14, 2026
…urface Two fields in this package were invented rather than specified. Both are now corrected against the PR MiniMax-AI#33 round-7 precedent, which is the only authoritative statement of where a host version constraint belongs: "`requirements` field is gone from `plugin.json` — the portable Plugin schema (v1.0.0) does not declare a `requirements` field, so the repository validator rejects `plugin.json` as `unknown field requirements`." "`metadata.minMcodeVersion` was the wrong surface for an already-supported schema; the human-readable constraint is in the field that both the marketplace UI and the agent's own discovery surface show." Corrections: - SKILL.md: drop `metadata.minMcodeVersion` (the rejected surface) and `metadata.scope` (a key that exists in no spec). `metadata` is now just `author` + `version`. - plugin.json and .minimax-plugin/plugin.json: state the host requirement in the `description` field, which is the permitted surface — the Plugin needs MiniMax Code to provide the `image_synthesize` and `gen_videos` host tools. - README: the previous "`minMcodeVersion`: 0.2.0 ... both were stabilized in 0.2.0" line asserted a version floor with no evidence behind it. Replaced with the verifiable form: the host must provide both tools, verified on mcode 0.4.6 (the version actually installed and exercised). Fail-closed tests added, mirroring the PR MiniMax-AI#33 test shape (229 -> 234): - `plugin.json` must not contain a `requirements` field. - `plugin.json` description must name both host tools. - SKILL.md frontmatter must not carry `minMcodeVersion` or `scope`. - SKILL.md `metadata` values must be YAML strings, not numbers (the runtime requires `metadata` to map strings to strings; an unquoted `version: 1.0` would parse as a number and fail validation). Verified: `npm run check` 234/234 pass; skill-review audit 100% PASS.
weekbin
added a commit
to weekbin/MiniMax-Code-Plugins
that referenced
this pull request
Sep 15, 2026
…chema Cross-checked this package against every review finding the maintainer has raised across the 34 PRs in MiniMax-Code-Plugins (63 review bodies, 41 of them CHANGES_REQUESTED). The dominant blocker pattern is PR MiniMax-AI#33's: "the package documents unverified host behavior as executable guidance ... invents ... parameters ... without a public MiniMax Code contract". This package had four instances of it. Verified source for the contracts below: the tool schemas shipped in `@minimax-ai/code` (`chunks/chunk-*.js`, mcode 0.4.6). image_synthesize: requests[] up to 10, each { prompt, output_file_path, input_file_paths (max 4), aspect_ratio, resolution: 1K|2K|4K } gen_videos: requests[] up to 5, each { prompt, output_file_path, input_image_path, reference_type: first_frame|last_frame, duration: 6|10, resolution: 768P|1080P, model } shared: "Paths outside the session workspace (e.g. /tmp or input attachment directories) are rejected." Host-contract fixes ------------------- - `gen_videos` was documented with three fields that do not exist: `fps`, `size = 1080x1080`, and a `first_frame_image` argument. Replaced with the real fields — `output_file_path` (required), `input_image_path`, `reference_type`, `duration`, `resolution` — and the doc now states that `fps` is not a parameter and that generation is asynchronous. - SKILL.md § 2.1 asked for "the same 6 reference frames" while the schema caps `input_file_paths` at 4, and while § 1.1 correctly names the 3 `examples/*.png` files. The "6" leaked from the 6-candidate batch. Now 3, with the limit stated. - `reference.md` told the reader to extract a video frame to `/tmp` and pass it as an `input_file_path`. The host rejects exactly that path class, so the documented procedure could not succeed. Rewritten to extract into the session workspace. - § 3 described "copy the output to video.mp4"; with the real API the host writes to the required `output_file_path` and there is no copy step. Documentation drift (PR MiniMax-AI#18's pattern) -------------------------------------- - Removed every reference to assets this package no longer ships: the `sample_0[1-6].png` / `overview.png` warning in SKILL.md § 1.1 and the "Removed in 0.2.0" block in reference.md. - Version drift: SKILL.md metadata said 0.2.0 while both manifests said 0.1.0. All three are now 0.2.0. - README did not list the newly added `scripts/make_contact_sheet.py`. - `make_contact_sheet.py` docstring claimed the sheet is "at most ~1500x1000"; `--cols` accepts up to 6, which yields 2918 px. Now states the actual default (1460x974) and that wider --cols scales up. Platform claims (PR MiniMax-AI#5 / MiniMax-AI#21's pattern: claiming a platform without evidence) ----------------------------------------------------------------------------- - README claimed Windows 10/11 and Linux support flatly while only macOS has been exercised. Now states macOS as verified and Linux/Windows as expected but not yet verified on those hosts. Also: the suite was red on the previous head — one test still asserted the removed `sample_0*.png` / `overview.png` files. Fixed, and the suite now pins the contracts above so they cannot drift back (247 tests, up from 240): - `gen_videos` fields are asserted present and the invented ones asserted absent; `first_frame_image` is rejected in every skill doc. - Every skill doc must state the host's 4-reference-image limit. - No skill doc may hand a `/tmp` path to a host tool. - No skill doc may reference the removed assets. - All three version declarations must agree. - No script may pass `shell=True`. Verified on this machine (Python 3.14, Pillow 12.3, ffmpeg 8.1.2): the README self-test runs end to end (1080x220 overlay, 2400x530 preview, 720,720,141 and 480,480,141 GIFs), `make_contact_sheet.py` emits the documented 1460x974 sheet. `npm run check` 247/247 pass; skill-review audit 100% PASS.
weekbin
added a commit
to weekbin/MiniMax-Code-Plugins
that referenced
this pull request
Sep 15, 2026
… our own code The previous commit fixed the four literal findings from the historical PR reviews. This one hunts the same defect classes in the rest of the package rather than waiting for a reviewer to find them. Five more, three of them reproduced before fixing. Invented host references (PR MiniMax-AI#33 pattern), 1 more instance --------------------------------------------------------- `issues.md` told the reader to re-poll with `matrix_query_video_generation`. That tool does not exist. The shipped schema declares `query_video_generation` and `submit_video_generation`; there is no `matrix_`-prefixed variant in the tool list at all. Corrected. Security: path traversal in `make_gif.py` (PR MiniMax-AI#37 pattern) --------------------------------------------------------- `--output-name` and `--mini-name` read as bare file names but were joined onto the scene directory as paths, so: make_gif.py <scene> "caption" --output-name ../../../../tmp/PWNED.gif wrote 10 MB to /tmp/PWNED.gif, outside the scene directory. Reproduced before fixing. Both flags now go through `resolve_output()`, which rejects absolute paths, path separators, anything that resolves outside the scene directory, and symlink escapes (it compares realpaths). A failed run now leaves no file behind. The guard runs before any I/O, so a bad flag fails fast rather than being masked by a later "video.mp4 not found". Silent clipping of long captions -------------------------------- `make_text_overlay.py` rendered at `--size 130` regardless of the caption width. Measured: the 1080 px canvas fits about 8 CJK glyphs; at 10 characters the text hit both edges and was clipped with no warning, and `issues.md` recommended exactly that configuration. `--size` is now a maximum and the script shrinks the font until the caption fits, reporting the size it used: 8 chars → 130, 10 → 106, 14 → 74, 20 → 52. `--no-fit` restores the old behaviour but errors instead of clipping. A caption that cannot fit even at the 24 pt floor fails with a message rather than shipping a cut-off image. ffmpeg version claim contradicted the implementation ---------------------------------------------------- README, README.zh-CN, and `make_preview_strip.py` all said "ffmpeg 4.4+", but both scripts pass `-fps_mode`, which replaced `-vsync` in **ffmpeg 5.0**. Reproduced the mismatch: ffmpeg 8.1.2 warns that `-vsync` is deprecated, i.e. the flag we removed is the one 4.4 understands. The stated floor is now 5.0 and `make_gif.py` parses `ffmpeg -version` and fails with a clear message on an older major. Stale mitigation advice ----------------------- `issues.md` "Text overflows the canvas" blamed "font size 220 with a 4-character caption" and prescribed `--size 130`. Both halves were wrong: the real limit at 130 is ~8 glyphs, and the script now auto-fits. Row rewritten to describe the actual behaviour. The `gen_videos` path-rejection row quoted an error string the host never emits; it now quotes the real contract. Tests (240 -> 252, all green) ----------------------------- - Host tool names in every skill doc are checked against the shipped schema; a token shaped like a tool call must be in the real tool list. - `--output-name` traversal is refused for all three shapes (`../`, absolute, nested path) and nothing is written outside the scene dir. - A 14-character caption renders with both edges clear of the canvas, proving the auto-fit instead of clipping. - No doc may advertise ffmpeg 4.4; the 5.0 floor must be stated. - The two checks that assert the traversal guard and the auto-fit are behavioural (they run the scripts), and both skip cleanly when python3 or Pillow is unavailable so Ubuntu CI stays green. Also corrected the SKILL.md contact-sheet size claim (`~1500x1000` → the measured 1460x974). Verified on this machine (Python 3.14, Pillow 12.3, ffmpeg 8.1.2): the README self-test runs end to end — overlay 1080x220 at size 130, preview 2400x530, GIFs 720,720,141 and 480,480,141; a 14-character caption auto-fits at size 88 in 1076x103 px. `npm run check` 252/252 pass; skill-review audit 100% PASS.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replacement of PR #18
This is a fresh, self-contained PR for the
codex-harness-patternscontribution. It replaces the abandoned PR #18, which had become unreviewable (51 commits / 168 files / head=base=main, with codex-harness-patterns mixed into the same change as #5 tool-map, #17 mcode-island, and the proposal hooks - exactly the "remove the unrelated changes" shape that amszuidas' round-8 P2 (2026-09-07T03:17:13Z) asked to be cleaned up).What this PR contains
Two paths, 29 files, 6112 insertions:
plugins/antianqi/codex-harness-patterns/- the plugin itself:plugin.json+LICENSE+README.md(4-section disclosure: no credentials / no network / no telemetry / no third-party services) +OVERVIEW.md+PR-STATUS.md+ 23 Skills.test/codex-harness-patterns.test.mjs- the static-check companion (closes round-4 false-green holes, pins mcode 0.2.4 task contract, assertsminMcodeVersion).No other files change. The 30-commit history of the old PR #18 (
fb0b87dv0.3.0 origin throughc949789v1.0.4 final) is collapsed into one commit here; the round-by-round reasoning remains at #18 (closed, but readable).Skill list (v1.0.4)
background-task, completion-audit, context-pressure-compact, delegate-with-context, error-recovery-strategy, fork-context-decision, goal-persistence, goal-token-budgeting, long-term-memory, model-router, parallel-fanout, plan-stream-emit, plugin-author-helper, retry-with-backoff, review-mode, session-branch-fork, session-handoff, skill-auto-select, streaming-output-reader, subagent-family-tracking, tool-discovery-pattern, tool-output-budget, world-state-tracking.
What this PR does NOT contain (by design)
.github/workflows/changes. The old PR Add codex-harness-patterns plugin (v1.0.3 - 23 Skills covering complete agent lifecycle: planning, decomposition, sub-agent parallelism, execution, state tracking, tool discovery, skill/plugin authoring, memory persistence, session branching) #18 copiedtool-map-windows.ymlandmcode-island-windows.ymlto the main branch so the windows-latest CI could find them; those workflows belong to the windows-latest job of the tool-map / mcode-island PRs, not to this one.plugins/antianqi/tool-map/changes. Those are PR Add antianqi/tool-map v0.2.0: persistent cross-platform tool inventory #5.plugins/antianqi/mcode-island/changes. Those are PR mcode-island v0.3.0: add io.minimax.mcode Hooks extension (forward-compat with PR #20) #21.test-fixtures/changes. The twodrive-bundle-failure*.mjsfixtures in the old PR Add codex-harness-patterns plugin (v1.0.3 - 23 Skills covering complete agent lifecycle: planning, decomposition, sub-agent parallelism, execution, state tracking, tool discovery, skill/plugin authoring, memory persistence, session branching) #18 were part of PR Add antianqi/tool-map v0.2.0: persistent cross-platform tool inventory #5 tool-map test suite; not relevant here.validate / analyze / CodeQLmatrix without dispatch.Test evidence
The Plugin own
scripts/smoke.mjsstatic-check (no hardcoded host paths; no third-party services; frontmatter contract) will run as part ofnpm run checkon this PR CI matrix.Design compliance
Note on review history
This contribution went through 14 rounds of review on the abandoned PR #18 (
fb0b87dv0.3.0 toc949789v1.0.4). The reviewer feedback (round-3 README 4-section disclosure, round-4 false-green holes, round-5 self-contradictions + plugin.json minMcodeVersion + SKILL.md path claim, round-7 mcode 0.2.4 pin) is all incorporated into the single commit here. If a reviewer wants to see the round-by-round reasoning or the exact wording of any specific reviewer comment, those are on the closed PR #18 linked above.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.