Repository navigation
chore(skill-lib): refresh root skill-lib snapshot to 1b1a947 (#120) - #78
erinepshovel-code wants to merge 16 commits into
Conversation
Mirror The-Interdependency/skill-lib@9867ab3 into skill-lib/ exactly (git archive; blob- and mode-identical, 66 added, 83 modified, 0 removed). Move every projection that named the old fb3b53a snapshot: - stack-manifest.json / STACK_MANIFEST.md skill-lib pin, with work_graph_sha256 recomputed (889a1234... -> 3d873d1c...) - backend/fresh-making-provenance.json: snapshot and doctrine pins at 9867ab3 (fresh-making/SKILL.md is byte-identical to 891dc0d); the separate-refresh hmmm is resolved - ahbg/integration/work-graph.json consumed_commit (test-enforced), with the decision/hmmm text no longer waiting on this refresh - README licensing row: only data-visualization/SKILL.md is Apache-2.0; provenance note now describes the exact 9867ab3 snapshot - README, AGENTS.md and backend/README.md: drop claims that the snapshot refresh is still separate The vendored .agents/skills/stack-update blob pin (a1e9148) still equals skill-lib@9867ab3 stack-update/SKILL.md. libs/ untouched.
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: f55bda8f2c
ℹ️ 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".
| PyYAML==6.0.3 | ||
| docstring-parser==0.18.0 | ||
| tree-sitter==0.26.0 |
There was a problem hiding this comment.
When the documented VM setup is used, it installs only backend/requirements.txt, so these Python dependencies—and the new npm TypeScript dependency—are absent. The collector converts missing dependencies into unsupported diagnostics, while backend/msdmd.py::_run_collector does not pass --strict; consequently both generation and independent rerender can agree on the same incomplete collection and record it as fresh. Install the native-reader runtimes in the backend deployment path and/or make these diagnostics fatal for fresh-making.
AGENTS.md reference: skill-lib/AGENTS.md:L176-L179
Useful? React with 👍 / 👎.
| */ | ||
| 'use strict'; | ||
| const fs = require('node:fs'); | ||
| const ts = require('typescript'); |
There was a problem hiding this comment.
For repositories containing TypeScript, collection output now depends on this worker and its locked TypeScript runtime, but backend/msdmd.py::generator_identity still calls tree_sha256 with the default (".py",) suffix. Changing typescript-reader.cjs, package-lock.json, or the schema assets therefore leaves the freshness key and verifier identity unchanged, allowing output produced by different generator code to retain the same accepted identity. Hash every implementation and lock file that can affect collection bytes.
AGENTS.md reference: AGENTS.md:L86-L89
Useful? React with 👍 / 👎.
|
|
||
| def render_typescript(collection: dict[str, Any], *, import_path: str) -> str: | ||
| """Render a schema-1 or schema-2 collection as a TypeScript module.""" | ||
| helper = "defineMsdmdCollectionV2" if collection.get("schema_version") == SCHEMA_VERSION else "defineMsdmdCollection" |
There was a problem hiding this comment.
When make-msdmd targets a checkout that still vendors the schema-1 helper—as all five current libs/*/.agents/skills/msdmd/collection.ts copies do—the new default output imports defineMsdmdCollectionV2, which those helpers do not export. The backend neither requests --legacy-blocks-only nor type-checks the artifact, so byte-identical verification can accept an unusable TypeScript module. Propagate the schema-2 helper before generation, select legacy output for old consumers, or add an explicit compatibility check.
AGENTS.md reference: skill-lib/AGENTS.md:L176-L179
Useful? React with 👍 / 👎.
| source_id = str(edge.get("source_id") or edge.get("from") or "") | ||
| target_id = str(edge.get("to") or "") | ||
| sources = id_files.get(source_id, set()) | ||
| targets = id_files.get(target_id, set()) |
There was a problem hiding this comment.
When the semantic domain consumes an actual schema-2 collection, _reconcile_block_edges has already rewritten source_id and resolved to values to fully qualified MSDMD addresses, but this function indexes declarations only by their short id. These lookups therefore miss every resolved edge; the semantic graph has no connections and analysis either fails the three-active-node requirement or reports meaningless results. Index declaration addresses (with an explicit short-ID fallback where needed) instead of assuming legacy edge IDs.
Useful? React with 👍 / 👎.
| if peer_uid != caller.pw_uid: | ||
| raise PermissionError( | ||
| f"broker peer uid {peer_uid} is not configured caller uid {caller.pw_uid}" | ||
| ) | ||
| response = execute_request(_recv_request(connection)) |
There was a problem hiding this comment.
In personal-console, shell_exec commands run as the same vmmcp UID that owns group access to the broker socket, and this UID check is the broker's only authorization. Any supposedly confined shell command can therefore connect to /run/vm-mcp/admin.sock, submit mode: "admin", and execute as root without using the visibly privileged admin_exec MCP tool or its audit/policy surface. Authenticate broker requests with a capability unavailable to shell children or isolate the broker client under a distinct service identity.
AGENTS.md reference: skill-lib/AGENTS.md:L182-L186
Useful? React with 👍 / 👎.
| VM_MCP_PORT=$PORT | ||
| VM_MCP_SHELL_ENABLED=0 | ||
| VM_MCP_ADMIN_SOCKET=$(systemd_quote "$ADMIN_SOCKET") |
There was a problem hiding this comment.
When installation sets a non-default VM_MCP_ADMIN_SOCKET, this value is written only to /etc/vm-mcp.env, which vm-mcp.service reads; vm-mcp-admin.service does not read that file and hardcodes /run/vm-mcp/admin.sock. The MCP client consequently connects to the configured path while the broker listens on the default path, leaving user_exec and admin_exec unusable. Generate a broker drop-in or load the same environment file in the admin unit.
Useful? React with 👍 / 👎.
| @@ -0,0 +1,303 @@ | |||
| # ratios: loc_comments=hmmm imports_exports=hmmm calls_definitions=hmmm | |||
There was a problem hiding this comment.
The new executable ai.sh starts with a ratios comment rather than a shebang. Interactive shells may fall back to interpreting an ENOEXEC file, but direct execution by execve-based callers—including subprocess.run(["ai.sh", ...])—fails with Exec format error, even though the installer exposes this file as a command on PATH. Put #!/usr/bin/env bash on the first line, as already done for install_ai.sh.
Useful? React with 👍 / 👎.
| source_commit=args.source_commit, | ||
| max_file_bytes=args.max_file_bytes, | ||
| snapshot_identity=args.snapshot_identity, | ||
| generated_outputs=[args.out.resolve().relative_to(args.root.resolve()).as_posix()] if args.out and args.out.resolve().is_relative_to(args.root.resolve()) else [], |
There was a problem hiding this comment.
The backend renders the candidate and verifier to distinct random paths inside the target root, and this new argument causes each path to be recorded as a generated-collection-output discovery entry. The verifier run also discovers the already-written candidate because it is not passed in generated_outputs. As a result, the two rendered files differ even for a deterministic repository, so run_job always fails its byte comparison and can never accept a fresh artifact. Normalize the recorded output to the final artifact path and exclude all temporary outputs from both scans.
AGENTS.md reference: AGENTS.md:L86-L89
Useful? React with 👍 / 👎.
| names = sorted(os.listdir(directory_fd)) | ||
| except OSError as exc: | ||
| failed(parent or ".", exc, "subtree") | ||
| return | ||
| for name in names: |
There was a problem hiding this comment.
For a clean Git checkout containing an ignored local file, this filesystem walk still reads that file and embeds its digest and parsed contents while the collection continues to claim the Git HEAD as its revision and dirty_worktree: false. Thus two machines can produce different artifacts for the same source identity, and ignored credentials such as a local private.json can be copied into a generated collection. Restrict commit-bound discovery to tracked files, or explicitly bind every additional local input through a content-snapshot identity.
AGENTS.md reference: AGENTS.md:L103-L104
Useful? React with 👍 / 👎.
…or-owned generator identity Mirror The-Interdependency/skill-lib@38c6433 (main after skill-lib #118) into skill-lib/ exactly (git archive; blob- and mode-identical, 3 added, 12 modified, 0 removed). Pins moved 9867ab3 -> 38c6433: - stack-manifest.json / STACK_MANIFEST.md, work_graph_sha256 recomputed (3d873d1c... -> 48ee1761...) - backend/fresh-making-provenance.json doctrine and snapshot pins; compare 891dc0d..38c6433 (fresh-making/SKILL.md unchanged) - ahbg/integration/work-graph.json consumed_commit and decision text - README provenance note Backend: - generator_identity delegates to `python -m msdmd.collect --print-generator-identity` under the worker's interpreter and collector environment (replaces tree_sha256); a collector that cannot report it fails closed. - Collector exits 3 (reader runtime missing), 4 (target schema helper older than the output) and 5 (git visibility / ignored root) fail the attempt closed in the ledger with the exit code and an operator action; nothing is published and the prior accepted artifact stays authoritative. - backend/ops/install_msdmd_runtime.sh plus README / VM_SETUP install steps: pip install -r $STACK_SKILL_LIB_ROOT/msdmd/requirements.txt and npm ci --ignore-scripts --prefix $STACK_SKILL_LIB_ROOT/msdmd. - Tests: collector-reported identity fails closed; exit 3/4/5 refusals.
erinepshovel-code
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 131497cf47
ℹ️ 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".
| def parse_yaml(data: bytes) -> Any: | ||
| import yaml # Optional reader dependency; missing installation is diagnostic. |
There was a problem hiding this comment.
When a collected repository contains a top-level yaml.py, this import executes that repository file because backend/msdmd.py::_run_collector launches Python with the target root as cwd, which precedes the trusted generator on sys.path; the same problem applies to docstring_parser and potentially msdmd itself. This violates the syntax-only boundary and lets inspected source run with the worker's PostgreSQL identity and write access to /srv/stack-repos, while the generator identity probe runs from the generator root and does not detect the shadowing. Invoke the collector with an isolated import path or otherwise ensure dependencies resolve only from the trusted runtime.
Useful? React with 👍 / 👎.
| def count_dirty_entries(status: str) -> int: | ||
| """Count dirty status entries while ignoring generated report files.""" | ||
| if status == "hmmm": | ||
| return 0 |
There was a problem hiding this comment.
When git status cannot run or its checkout is unreadable, git_value returns "hmmm", but this branch converts that unknown result to 0; the generated audit then presents the repository as clean and emits no dirty-state oddity. Preserve an explicit unknown state so a failed Git observation cannot be mistaken for evidence of a clean checkout.
AGENTS.md reference: skill-lib/AGENTS.md:L118-L119
Useful? React with 👍 / 👎.
| @mcp.tool(annotations=ADMIN_SHELL) | ||
| def admin_exec(command: str, cwd: str = "/", timeout_seconds: float = 60.0) -> dict[str, Any]: | ||
| """Personal-console only: execute an explicitly privileged command as root.""" | ||
| return _result( | ||
| lambda: policy_run_shell( | ||
| _config(), command, cwd=cwd, timeout_seconds=timeout_seconds | ||
| lambda: broker_exec( | ||
| _config(), mode="admin", user=None, command=command, cwd=cwd, | ||
| timeout_seconds=timeout_seconds, | ||
| ) |
There was a problem hiding this comment.
When VM_MCP_PROFILE=personal-console, any process belonging to any local account can connect to the unauthenticated Streamable HTTP listener on 127.0.0.1:8765 and invoke this root tool; the authenticated remote tunnel does not protect direct loopback callers. The broker's SO_PEERCRED check only sees the shared vmmcp server process, so it cannot distinguish the owner from a local attacker. Add caller authentication or expose the MCP endpoint through an OS-level transport that only the intended owner can access.
AGENTS.md reference: skill-lib/AGENTS.md:L182-L186
Useful? React with 👍 / 👎.
| if [[ ! -e "$WORK_ROOT" ]]; then | ||
| install -d -o "$SERVICE_USER" -g "$SERVICE_GROUP" -m 0750 "$WORK_ROOT" | ||
| elif [[ ! -d "$WORK_ROOT" ]]; then | ||
| echo "ERROR: VM_MCP_ROOT exists but is not a directory: $WORK_ROOT" >&2 | ||
| exit 5 |
There was a problem hiding this comment.
When VM_MCP_ROOT already exists with restrictive ownership—for example an application checkout mode 0700 owned by its service account—this branch deliberately leaves its ownership and permissions unchanged, but the MCP service still runs as vmmcp. Installation then completes and starts a service that cannot traverse or read its configured root. Preserve ownership while granting an explicit ACL/group permission, or fail installation with an actionable error when vmmcp lacks the required access.
Useful? React with 👍 / 👎.
…ython -P python -m puts the working directory ahead of PYTHONPATH, and the collector and identity probe ran with cwd set to the target checkout, so a target-level msdmd/collect.py (or yaml.py) shadowed the pinned collector; Review reproduced a forged artifact published as fresh (stack #78 P1). Both the executor/verifier runs and --print-generator-identity now use cwd=generator_root and -P (Python 3.11+). Test: a target carrying its own msdmd/ package and yaml.py never runs and its forged bytes never publish.
… and signals - collector_failure names the status of unmapped exits (exit N) and signal deaths (killed by SIGKILL/SIGSEGV, with an operator hmmm) instead of recording only stderr. - An OSError while spawning the executor or verifier (for example ENOMEM) is recorded with ledger.fail; the identity probe and git HEAD lookup turn OSError into fail-closed hmmm instead of raising; evaluate reports verifier-unavailable. - worker.run_once records any unexpected run_job exception on the job and keeps looping; if the ledger itself is unreachable it still crashes so systemd restarts it. Review P3s on stack #78; each with a test.
… closed - Before checking the worktree, run_job removes hidden .<out>.<rand>.candidate, .verify, .<out>.<job>.accepted-backup and .<out>.fresh-status-verify files older than the lease (regular files only, our naming only). A stale rollback copy that still holds the accepted bytes while the artifact does not is restored instead of deleted. - _unrelated_dirty raises ConstraintError (hold) when git status cannot run or exits nonzero, instead of treating the worktree as clean. Review P3s on stack #78; each with a test.
… executes The identity is environment-bound (interpreter, reader packages, Node, TypeScript, sandbox). The VM_SETUP flow queued from the operator shell with the shell's identity, the worker re-keyed it, and fresh status from the shell re-keyed it back (stack #78 P2). - build_spec/refresh_identities/evaluate/queue_make take observe_generator. stackctl --queue-only, fresh status and fresh explain never probe: they keep the identity the worker recorded, or mark a new derivation worker-pending. - run_job (the executing worker) observes the identity and records its components (--print-generator-identity --json) in runtime. A worker-pending job is superseded once under the observed key; later moves still fail closed for the operator. - A moved key or identity mismatch names the components that differ (e.g. node: v24.15.0 -> absent). - backend/ops/worker_sandbox_run.sh runs a command via systemd-run with the worker unit's user, environment file and sandbox properties; VM_SETUP and the backend README use it for worker once / status / explain. - New contracts stack_msdmd_target_cannot_shadow_collector and stack_msdmd_identity_observed_where_executed with checks.
… worker sandbox - umask 022 so the worker user can read the installed trees; the worker user (STACK_WORKER_USER, default stackorchestrator) must exist. - The identity probe (python -P, cwd at the generator root, like the backend) must succeed and report node and typescript, or the script exits 3. - A second probe runs inside the worker unit's sandbox through worker_sandbox_run.sh (systemd-run with the unit's User, EnvironmentFile and MemoryDenyWriteExecute/Protect* properties); without root or systemd-run it exits 4 unless STACK_SANDBOX_PROBE=skip. - pip hash-checking skipped and recorded as hmmm (no published hashes). - VM_SETUP: MemoryDenyWriteExecute=true stays; the sandboxed probe passes only once the pin includes skill-lib's node --jitless change (skill-lib #120). Review P2/P3 on stack #78; stubbed-tool tests in backend/tests/test_install_script.py.
None of stack's existing checks ran backend/tests. The new workflow runs bash -n on backend/ops and unittest discovery on Python 3.11 (the backend's documented minimum). PostgreSQL integration tests still skip without STACK_TEST_DATABASE_URL and remain VM deployment gates.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af58b11c18
ℹ️ 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".
| _collector_command("--root", runtime["root"], "--repo", runtime["repo"], | ||
| "--out", str(output), "--source-commit", _source_sha(spec)), |
There was a problem hiding this comment.
When a target contains TypeScript and the collector runs in the shipped worker unit, VM_SETUP.md now documents that Node dies with SIGTRAP under MemoryDenyWriteExecute; read_typescript records that as the error diagnostic typescript_reader_failed, which is not treated as a runtime-unavailable refusal. Because this invocation still omits --strict, the candidate and verifier can both omit the TypeScript facts, compare byte-for-byte, and be accepted as fresh. This is fresh evidence beyond the earlier missing-dependency comment: the current deployment guide explicitly identifies this non-missing-runtime crash path. Pass --strict, map worker crashes to a refusal, or pin the compatible --jitless collector before accepting output.
AGENTS.md reference: AGENTS.md:L86-L89
Useful? React with 👍 / 👎.
| if args.queue_only: | ||
| job, report = msdmd.queue_make(ledger, spec["target"], executor=args.executor) | ||
| job, report = msdmd.queue_make(ledger, spec["target"], executor=args.executor, | ||
| observe_generator=False) |
There was a problem hiding this comment.
When the current freshness key already has a terminal job—for example, a previous attempt failed, or a succeeded artifact was subsequently deleted or tampered with—enqueue deduplicates to that existing terminal row. Unlike the synchronous make path, this queue-only branch never calls ledger.retry, so it returns success with a failed/succeeded job while no queued work exists for the persistent worker to claim. Requeue terminal results before returning from --queue-only.
Useful? React with 👍 / 👎.
| def remove_path(config: VmMcpConfig, requested: str, *, recursive: bool = False) -> dict[str, Any]: | ||
| _require_workspace_write(config) | ||
| path = resolve_under_root(config.root, requested) |
There was a problem hiding this comment.
When requested names a symlink whose target is also inside VM_MCP_ROOT, resolve_under_root resolves the final component before this function receives it. Consequently remove_path("link") deletes the referent rather than the symlink, and leaves a broken link behind; move_path similarly moves or replaces the referent instead of the requested directory entry. Resolve and validate the parent path while retaining the final component for lstat/unlink/os.replace operations.
Useful? React with 👍 / 👎.
| WRITE_SHELL = ToolAnnotations( | ||
| readOnlyHint=False, destructiveHint=True, idempotentHint=False, openWorldHint=True | ||
| ) | ||
| WRITE_FS = ToolAnnotations(readOnlyHint=False, destructiveHint=False, idempotentHint=False, openWorldHint=False) |
There was a problem hiding this comment.
When write_text targets an existing file or move_path targets an existing destination, both implementations replace existing data via os.replace, yet both tools receive destructiveHint=False through this shared annotation. MCP clients and agents may therefore omit destructive-operation safeguards for calls that can irreversibly overwrite workspace data. Use a destructive annotation for the overwrite-capable tools, or make them refuse existing destinations unless overwrite is explicitly requested.
Useful? React with 👍 / 👎.
| def _read_yaml(path: Path, data: bytes, context: dict[str, Any]) -> tuple[list, list, list]: | ||
| reader_id = "yaml-core" | ||
| value, redacted = _redact_sensitive(parse_yaml(data)) |
There was a problem hiding this comment.
When a YAML document is a Kubernetes Secret, its payload fields are conventionally data and stringData, whose child keys are arbitrary names such as tls.key or config.json. The generic key-name heuristic does not recognize those containers or many valid child names, so this reader embeds their plaintext or base64 payloads verbatim in the structured-document fact and therefore in the generated collection. Detect kind: Secret and redact the complete data and stringData mappings before emitting facts.
Useful? React with 👍 / 👎.
| tmp = path.with_name(f".{path.name}.vm-mcp.tmp") | ||
| tmp.write_bytes(encoded) | ||
| os.replace(tmp, path) |
There was a problem hiding this comment.
When two stateless MCP requests write the same destination concurrently, both use the same predictable temporary pathname. One request can overwrite that temporary file before the other replaces it, causing a caller to report success even though the target contains the other request's bytes, while the losing request commonly fails because its temporary path has already been renamed. Create the temporary with a unique mkstemp/NamedTemporaryFile name in the destination directory before the atomic replace.
Useful? React with 👍 / 👎.
…than the lease
The pinned collector writes --out through tempfile.mkstemp(prefix='.msdmd-')
in the target root; a collector killed mid-write left that file behind and
the dirty-worktree check held every later attempt. _clean_stale_siblings now
also removes regular files whose whole name matches \.msdmd-[a-z0-9_]{8}
once they are older than the lease. Chosen over moving candidates to a
private directory, which would change the collector's --out/root handling.
Review P3 on stack #78 (af58b11); test covers stale, young and lookalike names.
…worker job evaluate rerenders into .<out>.fresh-status-verify; run_job's dirty checks did not ignore it, so a status run overlapping a worker attempt held the job. All three run_job worktree checks now ignore that sibling (the collector already excludes *_msdmd.ts.* siblings from its inputs). Review P3 on stack #78 (af58b11), with a test.
… relaxed hardening The wrapper parsed the unit file by hand: it ignored drop-ins, dropped 'Key = value' lines and broke on backslash continuations, so a probe could run without MemoryDenyWriteExecute and still pass. It now reads the loaded unit's properties with systemctl show -p ... (drop-ins merged), converts EnvironmentFiles entries (keeping ignore_errors as '-'), and exits 2 unless LoadState=loaded, User=STACK_WORKER_USER (default stackorchestrator) and MemoryDenyWriteExecute=yes. The install script passes its worker user. VM_SETUP: install the unit and daemon-reload before the runtime install. Review P3 on stack #78 (af58b11); stubbed systemctl/systemd-run tests.
…nding make-msdmd without --queue-only and stackctl make still registered the shell's generator identity (recovering only after one failed attempt). Now: - run_job/make/retry_job take observe_generator; stackctl make-msdmd, make, run and retry pass False, so they keep the worker-recorded identity. - A worker-pending target is only queued for the worker by a shell make. - A shell run holds (naming the differing components) unless its environment reproduces the recorded identity; it never re-keys the derivation. - --record-identity-here on make-msdmd / fresh make declares the shell the executor (dev hosts without a worker) and records its identity. - queue_make re-queues a same-key terminal job, so a held shell attempt or a repair can be picked up by the worker (moved here from make()). Review P3 on stack #78 (af58b11); tests for the library and CLI paths.
Mirror The-Interdependency/skill-lib@1b1a947 (main after skill-lib #120) into skill-lib/ exactly (git archive; blob- and mode-identical, 1 added, 7 modified, 0 removed versus 38c6433). Pins moved 38c6433 -> 1b1a947: - stack-manifest.json / STACK_MANIFEST.md; work_graph_sha256 recomputed (48ee1761... -> da8104f0...) - backend/fresh-making-provenance.json doctrine and snapshot pins; compare 38c6433..1b1a947 (fresh-making/SKILL.md unchanged; msdmd generator changed: Node --jitless and signal/probe exit 3) - ahbg/integration/work-graph.json consumed_commit and decision text - README provenance note; VM_SETUP.md MDWE note (pin now includes #120)
erinepshovel-code
changed the title
The merge kept the pre-#77 weave/stack research-participant relation, so test_native_edges_and_forge_ownership_match_root_manifest failed looking for "Stack-owned binary". Take main's weave participant records and recompute work_graph_sha256 (7e2aeaee…).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ae8677ec2b
ℹ️ 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".
| if not timed_out and now >= deadline: | ||
| timed_out = True | ||
| _kill_group(process) | ||
| descendants_cleaned = True | ||
| if process.poll() is not None and not descendants_cleaned: | ||
| _kill_group(process) | ||
| descendants_cleaned = True |
There was a problem hiding this comment.
When a user_exec or admin_exec command daemonizes with setsid (or otherwise creates a new process group), both cleanup branches kill only process.pid's original group. The detached child survives after the shell exits or the timeout expires, so a supposedly bounded command can leave an indefinite process—including a root process for admin_exec—while the broker reports completion. Track execution in a killable cgroup or otherwise terminate descendants outside the initial process group.
Useful? React with 👍 / 👎.
| while True: | ||
| chunk = connection.recv(64 * 1024) | ||
| if not chunk: | ||
| break |
There was a problem hiding this comment.
When an authorized peer connects but never closes its write side—for example, after sending a partial request—this blocking recv waits forever. Because serve() handles accepted connections serially, that one stalled client prevents every later user_exec and admin_exec request until the root broker is restarted. Add a receive deadline or a length-framed request protocol so incomplete clients cannot monopolize the service.
Useful? React with 👍 / 👎.
| tmp = path.with_name(f".{path.name}.vm-mcp.tmp") | ||
| tmp.write_bytes(encoded) | ||
| os.replace(tmp, path) |
There was a problem hiding this comment.
When write_text overwrites an existing shared workspace file under the shipped skill-lib/vm-mcp/systemd/vm-mcp.service, whose UMask is 0077, tmp.write_bytes() creates a new mode-0600 file and os.replace discards the destination's previous mode and ownership. A file previously readable by an application account or workspace group can therefore become accessible only to vmmcp after a successful MCP write. Preserve the existing metadata when replacing a file, or apply an explicit documented creation mode for new files.
Useful? React with 👍 / 👎.
1 participant
Summary
This PR refreshes the complete root
skill-lib/snapshot to The-Interdependency/skill-lib@1b1a9473fcbaab16965af9907e4aafd12853a704(main after skill-lib #120). It also moves stack's MSDMD fresh-making adapter onto the new collector. Erin approved the refresh on 2026-09-28 and the follow-ups on 2026-10-06.fb3b53a7629f7f03ecf255167d52c13abef1a979(plus licensing-only files frome6e3e5c)38c64332b840b2bbe1c07e53aeee8996644548e9(after skill-lib #118)1b1a9473fcbaab16965af9907e4aafd12853a704(after skill-lib #120: Node--jitless, signal/probe exit 3)f55bda8mirrored9867ab3.131497cre-mirrors38c6433(3 files added, 12 modified, 0 removed versus9867ab3). It also moves every pin and changes the backend.f318657re-pins to1b1a947(1 added, 7 modified, 0 removed versus38c6433). Blob- and mode-identical togit archive 1b1a947. Pins andwork_graph_sha256updated (48ee1761…→da8104f0…). Compare38c6433..1b1a947(fresh-making/SKILL.md unchanged; msdmd generator changed).8cb6126merges stack main (weave feat(weave): complete native sequence cycle and repair review findings #77 /ccf707f) into this branch. Keeps the1b1a947skill-lib pin; adds weave's ucns/uchc research participants; takes main's updated weave boundary hmmm; recomputeswork_graph_sha256(6340a86d…).ae8677erestores main's weave/stack research-participant relation text (merge had kept the pre-feat(weave): complete native sequence cycle and repair review findings #77 wording); recomputeswork_graph_sha256(7e2aeaee…).skill-lib/is blob- and mode-identical togit archive 1b1a947, with no stack-local residue.sync/skill-lib-9867ab3. I kept it on purpose so this PR stays the same; the content is pinned to1b1a947.Review fixes (Review on 131497c), as normal commits with tests
770857epython -mputs cwd ahead ofPYTHONPATH, and the collector ran with cwd at the target, so a target'smsdmd/collect.py(oryaml.py) shadowed the pinned collector. A forged artifact got published as fresh.--print-generator-identityall run withcwd=generator_rootandpython -P. The docstring is fixed. Test: a target with its ownmsdmd/package andyaml.pynever runs, and its bytes never publish. I checked the real collector on pceac24c14b: the output is byte-identical to cwd=target (3b3eefc6…).ad7de7afresh status.--queue-only,fresh statusandfresh explainnever probe it. They keep the identity the worker recorded, orworker-pendingfor a new derivation. The worker records its identity plus its components (--print-generator-identity --json). It supersedes aworker-pendingjob once, under the key it observed. Any later move still fails closed. Mismatches name the components that differ, for examplenode: v24.15.0 -> absent. Newbackend/ops/worker_sandbox_run.shruns a command throughsystemd-runwith the worker unit'sUser,EnvironmentFileand sandbox properties. VM_SETUP's queue/status flow uses it now. New contractsstack_msdmd_target_cannot_shadow_collectorandstack_msdmd_identity_observed_where_executed, each with a check.3c0dd5finstall_msdmd_runtime.shdidn't fail when node or typescript was absent. P3: umask and the worker user.absent. It also probes inside the worker unit's sandbox throughworker_sandbox_run.sh, and exits 4 without root orsystemd-rununlessSTACK_SANDBOX_PROBE=skip. It setsumask 022and requiresSTACK_WORKER_USER(defaultstackorchestrator) to exist. Tests stub pip, npm and the collector (backend/tests/test_install_script.py).5c85551OSErroron spawn crashed the worker loop.collector_failurenow namesexit Norkilled by SIGKILL/SIGSEGV, and signal deaths get an operator hmmm. A spawnOSError(for example ENOMEM) in the executor or verifier is recorded withledger.fail. The identity probe and the git HEAD lookup fail closed.evaluatereportsverifier-unavailable.worker.run_oncerecords any unexpected exception on the job and keeps looping, but still crashes so systemd restarts it if the ledger is unreachable.8a8e1c3git statustreated as clean.run_jobremoves our own hidden.candidate,.verify,.accepted-backupand.fresh-status-verifysiblings that are older than the lease. It only touches regular files with our names. A stale rollback that still holds the accepted bytes, while the artifact doesn't, is restored instead of deleted. Agit statusthat can't run or exits nonzero is now a hold.af58b11backend/tests..github/workflows/backend.ymlrunsbash -n backend/ops/*.shandunittest discover -s backend/testson Python 3.11 whenbackend/**,frontend/cli/**orskill-lib/msdmd/**change. The PostgreSQL tests still skip, because they're VM gates.Review P3s (Review on af58b11), as normal commits with tests
c4ebe18.msdmd-XXXXXXXXtemp files (from a collector killed mid-write) weren't cleaned, so they made the dirty check hold._clean_stale_siblingsalso removes regular files in the artifact's directory whose whole name matches the collector'stempfile.mkstemp(prefix=".msdmd-")pattern (.msdmd-+ 8 of[a-z0-9_]) and that are older than the lease. Test: a stale one is removed and the make succeeds. A young one, a too-long name and an uppercase lookalike are kept, and they still hold.b26fc2afresh statusrerender (.out.fresh-status-verify) held the worker job.run_job's three_verify_runtimecalls now ignore the artifact's.fresh-status-verifysibling (_status_verify_path). Test: with a status rerender file in place, the job succeeds and leaves the young file for the status run to remove.ee7d07eworker_sandbox_run.shparsed the unit file.systemctl show -p … -- $STACK_WORKER_UNIT.EnvironmentFiles=entries becomeEnvironmentFile=, with a-prefix whenignore_errors=yes. It exits 2 (fail closed) unlessLoadState=loaded,User=$STACK_WORKER_USER(defaultstackorchestrator) andMemoryDenyWriteExecute=yes. Empty values are dropped, and--chdiroverridesWorkingDirectory. Tests (backend/tests/test_worker_sandbox_run.py) use a stubbedsystemctlandsystemd-run: the exact property argv is forwarded, and a missing/wrongUser, missing/noMDWE, or an unloaded unit exits 2 without running anything.306367dmake-msdmdwithout--queue-only, andstackctl make, still registered the shell's identity. They only recovered after a failed attempt.make-msdmd,fresh make,fresh runandfresh retrynever observe the identity. They keep the worker-recorded identity, orworker-pending, the same way--queue-onlydoes. Aworker-pendingtarget is only queued for the worker. Otherwise the job runs in the shell and holds, naming the differing components (no re-key), unless the shell reproduces the worker's identity. New--record-identity-hereonmake-msdmd/fresh makemakes the shell the executor (for hosts with no worker).queue_makere-queues a terminal same-key job, so the worker can pick up a held shell attempt. Tests cover the library path (pending → queued only; mismatched env → hold with diff and no re-key; matching env → succeeds under the same key) and the CLI path (no probe while pending;--record-identity-hererecords).55690e7backend.ymlpinnedsetup-pythonby tag.actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0(a lightweight tag on that commit), like checkout. Testbackend/tests/test_backend_workflow.pyasserts everybackend.ymlaction is pinned to a 40-hex SHA with a version comment.Stack transaction (stack-update)
d0e76d5, skill-lib1b1a947(after intermediate38c6433)skill-lib/(the default MSDMD generator root)skill-lib/:stack-manifest.json,STACK_MANIFEST.md: skill-lib commit is38c6433.work_graph_sha256was recomputed with the checker's ownmanifest_digest(main889a1234…→48ee1761…).backend/fresh-making-provenance.json:doctrine.commitandlocal_generator_snapshot.commitare both38c6433. The evidence compare is891dc0d..38c6433;fresh-making/SKILL.mdis unchanged and the MSDMD generator changed. The separate-refreshhmmmis resolved.ahbg/integration/work-graph.json: skill-libconsumed_commitfollows the stack pin (enforced bytest_crossrepo_boundaries). Whether AHBG adopts the native readers remains a separate AHBG decision.README.md: the licensing row says onlydata-visualization/SKILL.mdis Apache-2.0. The provenance note now describes the exact38c6433snapshot.AGENTS.md,backend/README.md: removed the claims that the snapshot refresh is still separate..agents/skills/stack-updateblob pina1e9148still equalsstack-update/SKILL.mdat38c6433, so I left it unchanged.libs/*is untouched.Backend: fresh-making on the schema-2 collector
backend/msdmd.pygenerator_identityno longer hashesmsdmd/*.pywithtree_sha256. It runspython -m msdmd.collect --print-generator-identityusing the worker's own interpreter (sys.executable) and the samePYTHONPATHenvironment as the collector run.ValueError, which becomeshmmmin the ledger or aConstraintErrorhold).failed). The ledger records the error with the exit code and a short code, and an operator action inhmmm.reader-runtime-missing): a native reader runtime is missing.schema-helper-outdated): the target's.agents/skills/msdmd/collection.tshelper is older than the schema-2 output.git-visibility-unavailable): git can't list the target's visible files, or the root is git-ignored.evaluatereports the same classification when the verifier is refused.backend/ops/install_msdmd_runtime.shrunspip install -r $STACK_SKILL_LIB_ROOT/msdmd/requirements.txtinto the worker venv, thennpm ci --ignore-scripts --prefix $STACK_SKILL_LIB_ROOT/msdmd, then prints--print-generator-identity --json.backend/README.mdinstall block andbackend/deploy/VM_SETUP.md(install block plus a runtime section).test_generator_identity_comes_from_the_collector_and_fails_closedtest_collector_refusals_fail_closed_into_the_ledger: exits 3, 4 and 5. Even a collector that leaves a file behind is not published, the accepted receipt is unchanged, and no hidden temporaries remain.Validation (local, Node 24.15.0, Python 3.13)
backend/msdmd.py: default generator rootskill-lib/at38c6433, runtimes installed,MemoryLedger.c24c14b(current V1 helper):makesucceeded. The candidate (.<name>.<rand>.candidate) and the verifier (.<name>.<rand>.verify) were byte-identical, and the output was published.evaluatethen reran it a third time:fresh/verified.3b3eefc6…). No temporaries were left over.tsc --noEmit --strict --skipLibCheck --target ES2022 --module commonjs .agents/skills/msdmd/collection.ts pcea_msdmd.tswas clean.905e669(schema-1 helper): exit 4 became afailedjob withhmmm: msdmd exit 4 (schema-helper-outdated): …. Nothing was written, andevaluatereportsmaking-fresh/no-accepted-receipt.python tools/check_stack_consistency.py: pass (7 repositories, 36 research participant identities).skill-lib/interdependent-work-graph/portfolio_plan.py docs/work-graphs/repository-plan-report.json: pass.skill-lib/ratios/ratios_check.pyon the three english-full-view-evidence files: 0 drift, 0 gaps.pytest backend/tests: 17 passed, 2 skipped (PostgreSQL deployment gates), 3 subtests passed (at 131497c).af58b11):python -m unittest discover -s backend/tests -t .: 34 tests OK, 2 skipped, on both Python 3.13 and 3.11.tools/check_stack_consistency.py,portfolio_plan.py, theahbg/{runtime,benchmark,integration}suites andbash -n backend/ops/*.shall pass.ahbg/runtime,ahbg/benchmarkandahbg/integrationunittest suites passed.ahbg/groka0/tests,ahbg/testsandtestspassed.bash -n backend/ops/*.sh: OK.55690e7):python -m unittest discover -s backend/tests -t .ran 41 tests, OK (2 PostgreSQL skips), on both Python 3.13 and 3.11.tools/check_stack_consistency.pypassed (7 repositories, 36 research participant identities).portfolio_plan.pypassed, and its source commit is an ancestor. Theahbg/{runtime,benchmark,integration}suites passed, andbash -n backend/ops/*.shis OK. CI: 13/13 checks pass at55690e7.Deploy steps for Erin (not done here)
/etc/stack-orchestrator.envsourced:sudo -E env STACK_VENV=/srv/stack/.venv backend/ops/install_msdmd_runtime.shskill-lib/msdmd/requirements.txtinto/srv/stack/.venvand runsnpm ci --ignore-scriptswithumask 022. Then it probes the identity in the shell and inside the worker unit's sandbox. It fails if either probe reportsnodeortypescriptasabsent. Node and npm must already be installed, andnodemust be on the unit's default PATH.MemoryDenyWriteExecute=truestays). The pin now includes skill-lib #120 (1b1a947), so Node--jitlessis present. Remaining deploy acceptance: run one realsystemd-runsandboxed probe on the VM (viainstall_msdmd_runtime.shorworker_sandbox_run.sh) and confirm it reports node and typescript, notabsent. The two PostgreSQL backend tests still skip here; they are VM gates..agents/skills/msdmd/collection.tson main, so each exits 4 (a failed job, nothing published) until it is re-synced:metapat,ucns,edcmandptcnapcea(fixed when pcea Repair language construction and qualify complete source replay #43 merges)hmmm
backend/msdmd.pyruns the collector without--import-pathor--snapshot-identity. The skill-lib target (helper atmsdmd/collection.ts) would therefore get the default./.agents/skills/msdmd/collectionimport, which only warns "not found", and its output wouldn't match skill-lib's own committedskill-lib_msdmd.ts. This is pre-existing and not changed here.research/psfr/WORK_GRAPH.jsonstill has a research-local hmmm saying the snapshot is older than the doctrine it consulted. It is a historical research record, so I left it unchanged.Ready
skill-lib #120 has merged (
1b1a947). The DO-NOT-MERGE note is removed. This PR is ready for Erin's review and merge after CI is green.Remaining deploy acceptance (not blocking the merge):
systemd-runsandboxed probe on the VMErin merges this PR.
worker-pendingjob is superseded once, automatically. Any later generator move needs an operator to re-queue. When you run it by hand on first use,worker oncehas to run twice.fresh status/explainrun from a shell whose environment differs reportverifier-unavailable, with the differing components. Run them throughworker_sandbox_run.shfor a like-for-like check..msdmd-XXXXXXXXfiles are now cleaned after the lease (c4ebe18). A collector temp file younger than the lease still holds the job, by design.makeagainst a worker-recorded identity runs the collector in the shell and holds unless the environments match. Use--queue-only, or run throughworker_sandbox_run.sh. Use--record-identity-hereonly where no worker exists.requirements.txthas no hashes.worker_sandbox_run.shand the sandboxed probe were only dry-run here with a stubsystemd-run.