Repository navigation
Re-vendor skill-lib at 32a8a9f (skill-lib#121: TypeScript reader fail-closed) - #82
erinepshovel-code wants to merge 3 commits into
Conversation
Exact copy of git archive The-Interdependency/skill-lib@32a8a9f (compare 1b1a947..32a8a9f): TypeScript reader failures and timeouts fail closed (exit 3), deep valid input is a per-file typescript_input_too_deep diagnostic, and #122's consumer-drift and wdll changes.
Moves every 1b1a947 citation to 32a8a9f, records the 1b1a947..32a8a9f compare range and generator-change notes, recomputes work_graph_sha256, and documents the new exit-3 and deep-input behaviour in VM_SETUP.
…MODULE_BUILD end marker Since skill-lib#121, collector exit 3 also means a failed or timed-out TypeScript reader or worker, so the refusal is renamed reader-runtime-failed and its operator action no longer points only at installing runtimes. worker_sandbox_run.sh had the END MODULE_BUILD marker fused with the Usage comment (Review P3); they are now separate lines. Tests cover both.
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 43096bd56a
ℹ️ 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".
| Since The-Interdependency/skill-lib#121 (in `32a8a9f`), any TypeScript reader | ||
| or worker failure (nonzero exit, signal, incomplete output, refused flag) and | ||
| any reader run past the 120-second default timeout makes the collector exit 3 | ||
| without writing, and the worker records it as a `reader-runtime-failed` |
There was a problem hiding this comment.
When the TypeScript identity probe hangs, the backend paths inspected in generator_identity() and generator_components() impose their own 120-second timeout (backend/msdmd.py:195,221), while the new collector timeout also defaults to 120 seconds and starts later, after the collector has launched and hashed its sources. The outer timeout therefore wins: it kills only the collector Python process, so the collector never exits 3 or records reader-runtime-failed, and the Node process—started in a separate session by run_node()—can survive as an orphan. Pass a shorter --typescript-timeout from the backend, increase the outer timeout, or make the outer timeout kill the entire process group.
Useful? React with 👍 / 👎.
Re-vendor skill-lib at 24029ca (skill-lib#123 vm-mcp auth; supersedes #82)
1 participant
Re-vendors the root
skill-lib/snapshot to The-Interdependency/skill-lib32a8a9f(the merge of skill-lib#121, TypeScript reader fail-closed), the same way #78 did for1b1a947. Erin merges.Warning
This PR does NOT fix the vm-mcp hole. The vendored
skill-lib/vm-mcpstill has the unauthenticated loopbackadmin_exec, exactly as at1b1a947. It stays until skill-lib#123 merges and a later stack pin picks it up. (0e38909, which carried that fix, is in this compare range, but2ecdc8freverted it, so it nets to nothing here.)Compare range
1b1a9473fcbaab16965af9907e4aafd12853a704..32a8a9f0fdb07e725c122bf45d58ed3169d409be: skill-lib#119 (3d42ca1), #122 (19b1988, consumer drift tools and wdll) and #121 (32a8a9f).What's included
skill-lib/re-vendored fromgit archive 32a8a9f. Stack treeHEAD:skill-lib=027ab8ca7e1a9de8949c3820cdee6529a8ccd8b7= skill-lib32a8a9f^{tree}. All 244 entries match in path, mode and blob.1b1a947to32a8a9f:stack-manifest.json,STACK_MANIFEST.md,README.md,backend/fresh-making-provenance.json(doctrine and generator snapshot commits, plusskill_lib_compare= the range above),ahbg/integration/work-graph.json(consumed_commitand the decision text), andbackend/deploy/VM_SETUP.md.work_graph_sha256is recomputed to7cdbe321…929b.fresh-making-provenance.json(msdmd_generator_changes).fresh_making_doctrine_changed_in_rangestaysfalse(fresh-making/SKILL.mdis unchanged in the range), andmsdmd_generator_changed_in_rangestaystrue.backend/msdmd.pyexit-3 hint. The collector's exit 3 now also means a failed or timed-out TypeScript reader or worker, so the refusal is renamedreader-runtime-missing→reader-runtime-failed. Its operator action now covers a missing runtime, reader or worker failure, and timeout, not just installing runtimes. New test:test_exit_3_hint_covers_reader_failures_and_timeouts.backend/ops/worker_sandbox_run.shhad# === END MODULE_BUILD ===fused with the# Usage:line. They are now split. New test:test_module_build_marker_stands_alone.VM_SETUP.mddocuments the new exit-3 cases and the deep-input behaviour.Collector flags are unchanged.
Expected behaviour: deep TypeScript publishes at exit 0
Stack does not pass
--strict. So a valid TypeScript file that is nested too deeply for the jitless reader publishes with a per-filetypescript_input_too_deepdiagnostic (error severity, no facts for that file) and the collector exits 0. That is expected. Other files are collected normally. I checked this locally with the vendored tree, Node 24 and TypeScript 5.8.3: a 4000-term chained+file gave exit 0, onetypescript_input_too_deep, and the sibling file's facts intact.Deploy notes
b98837…8abc).backend/ops/install_msdmd_runtime.sh, which probes throughworker_sandbox_run.shunder the worker unit'sMemoryDenyWriteExecute). The probe now loads the TypeScript compiler.Local checks
tools/check_stack_consistency.py: pass.portfolio_plan.pyvalidation and the ancestor check: OK.bash -n backend/ops/*.sh: OK.node --check board.js.hmmm
research/weave/CYCLE_WORK_GRAPH.jsonstill cites skill-lib9867ab3, andbackend/deploy/VM_CONTACT.mdcites222ba4d. These look like cycle-specific and producer records, not snapshot pins, so I left them..agents/skills/stack-update/PROVENANCE.json(a7b95f8) is an independent pin, and its blob is unchanged at32a8a9f.STACK_IDENTITY_TIMEOUT_SECONDS, default 120) equals skill-lib's new 120 s probe timeout. The probe takes about 0.4 s in practice.