Repository navigation
chore(skills): sync vendored skill-lib skills to 38c6433 - #111
Conversation
Propagated with skill-lib tools/propagate_skills.py from The-Interdependency/skill-lib@9867ab3 for exactly the canonical skills already vendored here: meta-module-build, msdmd, test-build. Source pin: c14ee9d -> 9867ab3. Verified with check_consumer_drift.py --strict-sha --require-vendored (clean). Local non-canonical skills are 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. |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| """ | ||
| from __future__ import annotations | ||
|
|
||
| import hashlib |
| from __future__ import annotations | ||
|
|
||
| import ast | ||
| import hashlib |
| import math | ||
| import re | ||
| import shlex | ||
| import sys |
| if not Path(changed_path).name.endswith("_msdmd.ts"): | ||
| input_changes.append(line) | ||
| dirty = bool(input_changes) | ||
| except (OSError, subprocess.CalledProcessError): |
![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: 6f1830a7d2
ℹ️ 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".
| 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.
When stdout is redirected to a file under --root (for example, python -m msdmd.collect --root . --repo x --json --strict > collection.json), the shell creates and truncates that file before collection starts, but this exclusion only handles paths supplied through --out. The collector consequently scans its own empty destination, emits an invalid_json diagnostic, and makes strict mode fail; non-strict output also incorrectly includes the destination in its discovery snapshot. Route in-root JSON generation through --out or otherwise exclude the stdout destination.
Useful? React with 👍 / 👎.
| fd, temporary = tempfile.mkstemp(prefix=".msdmd-", dir=args.out.parent) | ||
| try: | ||
| with os.fdopen(fd, "wb") as handle: | ||
| handle.write(rendered.encode("utf-8")) | ||
| os.replace(temporary, args.out) |
There was a problem hiding this comment.
When --out is used, tempfile.mkstemp creates the replacement with mode 0600, and os.replace carries that mode onto the final collection even when replacing an existing 0644 file. In shared workspaces or multi-user CI stages, downstream consumers running under another UID can no longer read the generated collection. Apply the prior target mode, or a normal umask-derived readable mode, to the temporary file before replacing the destination.
Useful? React with 👍 / 👎.
| if len(data) > max_file_bytes: | ||
| excluded(relative, "size-limit", size=len(data)) | ||
| continue | ||
| readable.append({"file": relative, "data": data, "content_sha256": _sha256(data), "size": len(data)}) |
There was a problem hiding this comment.
On repositories containing many files below the per-file limit, _discover retains every file's complete byte buffer in readable until the entire traversal finishes. The default 2 MiB limit therefore does not prevent a large monorepo or untrusted tree from consuming gigabytes of memory before extraction begins, including for unsupported files that emit no facts. Process readable files incrementally or enforce an aggregate byte/file budget so the advertised bounded scan cannot exhaust the collector process.
Useful? React with 👍 / 👎.
| CoreLoader.add_implicit_resolver('tag:yaml.org,2002:null', re.compile(r'^(?:~|null|Null|NULL|)$'), ['~', 'n', 'N', '']) | ||
| CoreLoader.add_implicit_resolver('tag:yaml.org,2002:bool', re.compile(r'^(?:true|True|TRUE|false|False|FALSE)$'), list('tTfF')) | ||
| CoreLoader.add_implicit_resolver('tag:yaml.org,2002:int', re.compile(r'^(?:[-+]?[0-9]+|0o[0-7]+|0x[0-9a-fA-F]+)$'), list('-+0123456789')) | ||
| CoreLoader.add_implicit_resolver('tag:yaml.org,2002:float', re.compile(r'^[-+]?(?:(?:[0-9]+\.[0-9]*|\.[0-9]+|[0-9]+[eE][-+]?[0-9]+)(?:[eE][-+]?[0-9]+)?|\.(?:inf|Inf|INF|nan|NaN|NAN))$'), list('-+.0123456789')) |
There was a problem hiding this comment.
For a valid plain YAML scalar such as version: 1e2e3, this resolver matches the value as a float because the exponent-bearing alternative is followed by a second optional exponent. The later scalar conversion rejects it, so an ordinary string makes the entire YAML source invalid instead of being preserved as text. Restructure the alternatives so the optional exponent is applied only to decimal mantissas and cannot follow the already exponent-bearing form.
Useful? React with 👍 / 👎.
| if _SENSITIVE_KEY_RE.search(str(key)): | ||
| result[str(key)] = {"$redacted": True, "reason": "sensitive-field-name"} | ||
| redacted.append(child_pointer) |
There was a problem hiding this comment.
When a structured map uses a secret-like word as a legitimate identifier, this context-free key check corrupts its metadata. For example, package.json with "dependencies": {"token": "^1.0.0"} replaces the version constraint with a redaction marker and emits an inaccurate dependency fact, even though token is the package name rather than a credential field. Redaction needs to distinguish schema fields from identifier-keyed maps such as dependency tables.
Useful? React with 👍 / 👎.
| for line_number, raw in enumerate(text.splitlines(), start=1): | ||
| stripped = raw.strip() |
There was a problem hiding this comment.
When a requirements entry ends in an unescaped backslash, this line-by-line loop emits each physical line as an independent fact even though pip removes that newline before parsing. A continued environment marker can therefore become a phantom dependency named python_version, while a hash continuation is detached from the requirement, and the file is still reported as fully supported. Normalize continuations before comments and fact extraction according to the pip requirements-file format.
Useful? React with 👍 / 👎.
| for requirement in required_facts: | ||
| if set(requirement) != {"file", "convention", "kind"} or not all(isinstance(v, str) and v for v in requirement.values()): | ||
| raise ValueError("fact requirements must contain nonempty file, convention and kind strings") | ||
| codeowners_source = next((name for name in (".github/CODEOWNERS", "CODEOWNERS", "docs/CODEOWNERS") if any(item["file"] == name for item in readable)), None) |
There was a problem hiding this comment.
When .github/CODEOWNERS exists but is excluded from readable—for example, a 2.5 MiB file that exceeds this collector's 2 MiB default while remaining below GitHub's 3 MiB limit—this selection falls through to a root or docs file and labels that lower-priority file active. GitHub instead searches .github, root, and docs in that order and uses the first existing file, so the emitted ownership authority is incorrect; retain the preferred path as unresolved rather than promoting a lower file. See GitHub's CODEOWNERS location and size rules.
Useful? React with 👍 / 👎.
| if (node.moduleSpecifier && ts.isStringLiteralLike(node.moduleSpecifier)) { | ||
| result.imports.push({module: node.moduleSpecifier.text, kind: ts.isImportDeclaration(node) ? 'import' : 'reexport', | ||
| declaration: text(node), owner, ...span(node)}); | ||
| } else if (ts.isExportDeclaration(node) && node.exportClause && ts.isNamedExports(node.exportClause)) { |
There was a problem hiding this comment.
For a declaration such as export { foo as bar } from "./dep" or export * as ns from "./dep", the presence of moduleSpecifier takes this first branch and prevents the named-export branch from running. The collector consequently records only a dependency/re-export import and emits no exported name, alias, type-only flag, or export edge for bar or ns, leaving the module's advertised static API incomplete. Handle named export clauses even when they also have a module specifier.
Useful? React with 👍 / 👎.
Re-propagated with skill-lib tools/propagate_skills.py from The-Interdependency/skill-lib@38c6433 (main after #118). Pin updated in .agents/skills/README.md; the vendored msdmd helper exports MSDMD_COLLECTION_HELPER_VERSION = 1.
erinepshovel-code
changed the title
| diagnostics.append(_diagnostic(context, reader_id=reader_id, code="invalid_systemd_directive", message="expected key=value", status="invalid", severity="error", location={"start_line": logical_start, "end_line": number})) | ||
| continue | ||
| key, value = (part.strip() for part in line.split("=", 1)) | ||
| published_value = value |
| probe = subprocess.run(["node", "-e", _NODE_PROBE], cwd=base, env=env, capture_output=True, text=True, check=False) | ||
| if probe.returncode == 0: | ||
| node = {key: str(value) for key, value in json.loads(probe.stdout).items()} | ||
| except (OSError, ValueError): |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9a0222eb1a
ℹ️ 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".
| envelope_fact['projection'] = {'mapping_version': 'dsse-envelope@1', 'supersedes': 'structured-document', | ||
| 'loss': 'raw base64 payload withheld; decoded payload published at /payload'} |
There was a problem hiding this comment.
When a DSSE envelope is collected using the default TypeScript output, this adds supersedes, but MsdmdNativeFact.projection in collection.ts does not permit that property. I generated a DSSE collection and verified that the documented tsc validation fails with TS2353, so these otherwise supported inputs produce unusable TypeScript artifacts; add the property to the helper contract or represent the supersession without emitting an undeclared field.
Useful? React with 👍 / 👎.
| snapshot_entries = [ | ||
| f"{item['file']}\0{item.get('content_sha256', 'hmmm')}\0{item['status']}" | ||
| for item in discovery | ||
| ] | ||
| snapshot_sha256 = _sha256("\n".join(snapshot_entries)) |
There was a problem hiding this comment.
On POSIX, a repository filename containing bytes that are not valid UTF-8 is returned by os.fsdecode with surrogate characters, and this _sha256 call then raises UnicodeEncodeError while encoding the joined snapshot entries. Even an unsupported regular file with such a name aborts the entire collection instead of producing a discovery or unreadable entry; hash filesystem paths using their original byte representation or reject and diagnose these paths before constructing the snapshot.
Useful? React with 👍 / 👎.
| else: | ||
| excluded_inodes = [] | ||
| if not args.out: | ||
| # A shell redirect into the scanned tree is the output, not an input. |
There was a problem hiding this comment.
When stdout is redirected inside --root while --legacy-blocks-only is used, the shell-created destination is scanned because redirected-output inode exclusion is initialized only in this schema-2 branch, after collect_legacy has already run. For example, --legacy-blocks-only --expected-block DOCS > generated.ts reports generated.ts itself as a missing-block gap. Fresh evidence after the earlier stdout-exclusion finding is this mode-specific branch in the current code; compute and pass the exclusion before selecting legacy versus schema-2 collection.
Useful? React with 👍 / 👎.
| for i, subject in enumerate(statement.get('subject', [])): | ||
| emit('in-toto.statement', version, 'attested-subject', statement_pointer + pointer('subject', i), subject, | ||
| scope='artifact', identity=subject.get('name', str(i)), native_id=subject.get('name'), standing='reported-evidence') |
There was a problem hiding this comment.
When a supported in-toto statement contains a malformed subject entry, such as null or a string, these unchecked .get calls raise and read_native discards every fact already produced by the standards reader. The generic JSON fact then survives without the supersedes marker and publishes the original reversible base64 payload; I reproduced this with a predicate containing a password, which remained recoverable in the collection alongside only a native_reader_error. Validate subject objects without aborting the reader, or suppress the raw envelope independently of later semantic extraction.
Useful? React with 👍 / 👎.
| if path.suffix == '.h': | ||
| return [], [], [_diagnostic(context, reader_id=rid, code='ambiguous_header_language', | ||
| message='.h can be C or C++; supply an unambiguous .c/.hpp source representation', status='ambiguous')] | ||
| language, version = GRAMMARS[path.suffix.lower()] |
There was a problem hiding this comment.
When a repository contains conventional uppercase C-family suffixes, lowercasing the lookup silently treats .C files as C even though toolchains commonly use .C for C++, while .H bypasses the exact .h ambiguity check and then indexes the absent .h key, producing a generic reader error once the grammar runtime is installed. Dispatch these case-sensitive suffixes explicitly or exclude unsupported uppercase variants instead of selecting the wrong grammar.
Useful? React with 👍 / 👎.
| text = data.decode("utf-8-sig").replace("\r\n", "\n") | ||
| if not text.startswith("---\n"): | ||
| return [], [], [] |
There was a problem hiding this comment.
When a valid Markdown or MDX document uses lone carriage returns as line endings, this normalization leaves them intact, so the opening delimiter never matches ---\n and the reader emits neither frontmatter nor a diagnostic. Because readers_for still reports markdown-frontmatter as applied, the discovery entry is marked supported even though all frontmatter was silently missed; normalize remaining \r terminators before testing and parsing the delimiter.
Useful? React with 👍 / 👎.
| elif path.name == "llms.txt": | ||
| selected.append(("llms-text", _read_llms_text)) |
There was a problem hiding this comment.
The reader manifest and module documentation define this convention as the root llms.txt, but this basename-only check also classifies files such as docs/llms.txt as authoritative llms.txt instruction metadata and marks them supported. Use the already-computed repository-relative path so nested files remain ordinary unsupported text rather than being promoted into the root publication convention.
Useful? React with 👍 / 👎.
| entries.append({'key': name, 'tag': key.tag, 'value': visit(value, depth + 1)}) | ||
| if string_keys: | ||
| return {entry['key']: entry['value'] for entry in entries} | ||
| # JSON object keys cannot encode typed YAML keys without loss. | ||
| return {'$type': 'yaml-mapping', 'entries': entries} |
There was a problem hiding this comment.
When a YAML mapping contains both a sensitive string key and any non-string scalar key, this branch converts the entire mapping into entries records before _redact_sensitive runs. A document such as password: TOPSECRET plus 1: filler is therefore published with TOPSECRET under generic key/value fields and produces no redaction diagnostic, because the redactor never sees password as a map key. Apply sensitive-key handling while visiting the native mapping or teach the redactor to interpret this typed mapping representation.
Useful? React with 👍 / 👎.
| if '\x00' in text or re.search(r'<!\s*(?:DOCTYPE|ENTITY)', text, re.I): | ||
| raise ValueError('DTD, entity declarations and non-UTF-8 XML are unsupported') | ||
| if re.search(r'encoding\s*=\s*[\'"](?!utf-8[\'"])[^\'"]+', text[:200], re.I): | ||
| raise ValueError('XML metadata reader requires UTF-8') |
There was a problem hiding this comment.
The raw-text regular expressions inspect every occurrence in the first part of the document rather than only the XML prolog or DTD. Consequently, valid UTF-8 XML such as <root encoding="base64"/> is rejected as non-UTF-8, and literal <!DOCTYPE text inside a comment or CDATA section is rejected as a DTD. Restrict these checks to syntactic declarations so ordinary attributes and character data do not invalidate otherwise safe documents.
Useful? React with 👍 / 👎.
| candidates = [ | ||
| symbol | ||
| for symbol in symbols | ||
| if symbol.header_line <= group.start_line | ||
| and group.end_line <= _lexical_end_line(symbol, lines) |
There was a problem hiding this comment.
For each non-special comment group, this comprehension scans every discovered symbol and repeatedly recomputes lexical end lines, making projection quadratic in the number of declarations and comments despite the per-file byte bound. On synthetic files with one inner comment per function, I measured roughly 0.14 seconds for 500 functions, 0.53 seconds for 1,000, and 2.0 seconds for 2,000; a sub-2-MiB generated or adversarial Python file can therefore occupy the collector for many minutes. Precompute lexical spans and use a position/containment index instead of a full symbol scan per comment.
Useful? React with 👍 / 👎.
1 participant
Summary
Syncs this repo's vendored skill-lib skills to The-Interdependency/skill-lib@
38c64332b840b2bbe1c07e53aeee8996644548e9(main after skill-lib #118).The-Interdependency/skill-lib@38c64332b840b2bbe1c07e53aeee8996644548e9c14ee9d500579a4b5d6821f62c9d82ca96e73608→38c64332b840b2bbe1c07e53aeee8996644548e9(.agents/skills/README.md)doctrine/msdmd-checks.mdre-copied (unchanged).Done with skill-lib's
tools/propagate_skills.py --skills <the above> --apply. No skills were added or removed. Local non-canonical skills are untouched; the tool's README rewrite now lists them under "Other installed skills (not refreshed by this propagation)".Re-sync to 38c6433
9867ab3. A second, normal commit re-runstools/propagate_skills.py --skills <the same list> --applyfrom a clean checkout of skill-lib38c64332b840b2bbe1c07e53aeee8996644548e9(main after #118 merged) and moves the README pin from9867ab3to38c6433.sync/skill-lib-9867ab3. I kept that name on purpose so this PR stays the same; the content is pinned to38c6433.Verification
python tools/check_consumer_drift.py <repo> --sha 38c64332b840b2bbe1c07e53aeee8996644548e9 --strict-sha --require-vendored→ clean.agents/skills/msdmd/collection.tshelper exportsMSDMD_COLLECTION_HELPER_VERSION = 1, which meets the new collector's exit-4 requirement (REQUIRED_HELPER_VERSION = 1).hmmm
msdmdskill ships the schema-2 native collector. This repo's<repo>_msdmd.tscollection point was generated with the previous collector, so it is stale relative to this collector until a later regeneration. That is intentionally left out of this PR and is Erin's call (CI does not enforce it here).Erin merges this PR.