fix(msdmd): integrate native standards readers and fail-closed coverage - #111
Conversation
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-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![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: a101f8ee46
ℹ️ 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".
erinepshovel-code
left a comment
erinepshovel-code
left a comment
There was a problem hiding this comment.
Repair disposition for exact head 2aac5fdcca22fe2ac4b9922e9927824904d578c0 (base e6e3e5cd2886eb2c4cf43744a9ed6b6f7ab3b535). This is implementation evidence, not independent approval.
Hosted run https://github.com/The-Interdependency/skill-lib/actions/runs/36419083060 reproduced the new regression suite's original failures, applied the fixes, then passed all 12 regression tests, all 364 repository unit tests, generated collection generation/drift, TypeScript compilation, gonol authority, library drift, skill compliance, Codex adapter drift, strict RATIOS, llms drift, and 7/7 RepoLOTO checks. The temporary executor has been removed; ordinary PR CI is replaying the final source head.
Review dispositions:
collect.pysupplemental blocks: fixed. Missing/ambiguous native comment extraction retains source-qualified, redacted block-shaped candidates with unverified standing in an error diagnostic. It does not certify raw strings as declarations, and strict mode cannot silently succeed. Regressions cover missing TypeScript runtime and ambiguous.h.- YAML
!!float 1: finding not reproduced. The existing regex already accepts integral lexemes and the previously shipped fixture already passed with PyYAML installed. Added a separate control asserting both float type and 1.0 value; no unnecessary parser substitution. - Systemd continuations: fixed separator replacement, intervening comments and source span. Regression checks resulting
/bin/echo one two. - Systemd Environment: every assignment is checked, safe assignments retained, sensitive values withheld. Unsupported quoting/escaping withholds the entire value and reports partial scope, rather than approximating shell syntax or leaking data.
- Requirements includes/constraints: preserved as declarations and explicitly dynamic-unresolved; required-source coverage now fails rather than claiming the included graph was read.
- JSON numeric fidelity: unsafe integers, inexact binary64 decimals and signed zero retain original lexemes in an explicit json-number tagged representation. Regression includes actual Node.js JSON round-trip. Finite overflow rejection remains explicit.
- TypeScript local exports: exported aliases, type-only flags, local declaration identities and relations are retained. Private unrelated declarations remain private.
- Maven XML: requires the POM namespace or unnamespaced project/modelVersion 4.0.0; Ant project XML stays generic.
Additional source-review repairs: malformed-code preprocessing is inside the reader diagnostic boundary (five code suffix regressions), and SVG now uses the shared bounded UTF-8, DTD/entity-rejecting XML parser. JSON Schema URI recognition uses exact authorities; both original CodeQL alerts were automatically resolved by GitHub Advanced Security after the source correction.
Scope remains extraction of the documented tested subsets, not complete standards certification, cross-source semantic reconciliation, independent evidence/signature verification, or proof of runtime behavior. No consumer repositories were changed.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2aac5fdcca
ℹ️ 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".
erinepshovel-code
left a comment
erinepshovel-code
left a comment
There was a problem hiding this comment.
Review-repair disposition at exact head 4f469ce11fcccc1077d18b5ed3458eb8f5160e8c, against base e6e3e5cd2886eb2c4cf43744a9ed6b6f7ab3b535. This is implementer verification, not independent approval.
Product commit: eed15bfbc7bf2eff9b61f0959ce9d564387a536c. Final tree 0e1067f0a878adf3677f2b2941a8b0fa642be447 exactly matches the fully validated hosted worktree after removal of the temporary executor. Hosted run: https://github.com/The-Interdependency/skill-lib/actions/runs/36445543708 . All 377 unit tests passed, including 13 new integrated regression methods; strict collection generation and replay, TypeScript compilation, gonol authority, skill/library/Codex adapter drift, strict ratios, llms drift, RepoLOTO audit and 7/7 RepoLOTO checks passed. Ordinary PR CI replays the clean final head separately.
Disposition of the seven findings from review of 2aac5fdcca:
- XML credentials (4122068806): scrub sensitive element subtrees and namespace-qualified attributes before generic or semantic projection. Tests verify no sentinel values escape and the original source digest remains unchanged; benign content survives.
- TypeScript defaults (4122068815): retain export assignments, identifier bindings, named/anonymous default declarations and export-equals. Anonymous collector identities are explicitly synthetic and do not masquerade as native IDs. Expression exports remain facts even without a named declaration.
- Python
__all__(4122068823): only a single simple literal declaration without mutations, additional assignments or escapes receives static standing. Other forms preserve declared information and report dynamic-unresolved; strict required-source coverage fails. Eight mutation/escape regressions plus literal and conditional controls. - Repeated SPDX lines (4122068831): occurrence-qualified IDs preserve both copyright declarations without fact-address collisions in Python and TypeScript.
- TypeScript comments (4122068838): compiler-token extraction includes empty-body/trailing trivia and attaches comments to the innermost lexical declaration; module comments and literal/regexp lookalikes have explicit controls.
- Reader identity (4122068847): the universal parser participates in implementation_digest; an executable-byte-change regression verifies invalidation.
- Gitignore whitespace (4122068852): remove unescaped trailing spaces using backslash parity, retain escaped spaces and marker escaping, and preserve ordering/directory/negation semantics.
The initial validation attempt passed the 13 regressions but correctly failed the repository snapshot replay before regeneration. The successful run regenerated first and passed the full suite; no gate was disabled or weakened. Generated collection SHA-256: 317e3dcc403b87d60e4580885188dee7bb780c41a53f69d7dbca351ec75f57e1.
hmmm: support remains the documented extraction subsets, not complete standards certification, executed program semantics, cross-source semantic reconciliation or cryptographic evidence verification. No downstream consumer repositories were modified.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f469ce11f
ℹ️ 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".
| value = {'attributes': dict(root.attrib), 'title': title, 'description': description} | ||
| return [_fact(context, reader_id=reader_id, kind='document-metadata', scope='document', identity=context['file'], | ||
| location={'pointer': '/svg'}, value=value, convention='svg.document-metadata')], [], [] |
There was a problem hiding this comment.
When an SVG root has a sensitive attribute such as api_token="secret", or a <title>/<desc> contains a sensitive descendant, this value is copied directly into the generated collection without a redaction diagnostic. The generic XML reader now scrubs sensitive element and attribute names, but .svg files bypass that reader, so apply equivalent redaction before publishing the SVG metadata.
Useful? React with 👍 / 👎.
| if value is None or isinstance(value, (str, bool, int)): | ||
| return value |
There was a problem hiding this comment.
For TOML or YAML integers outside JavaScript's safe range, such as n = 9007199254740993, this branch preserves the Python int and render_typescript emits it as a TypeScript number literal, which evaluates to 9007199254740992. The JSON-specific tagging path does not cover these inputs because TOML/YAML integers reach _json_safe directly, so unsafe integers need the same explicit lexeme/tag representation.
Useful? React with 👍 / 👎.
| match = _REQUIREMENT_NAME_RE.match(published) | ||
| name = match.group("name") if match else "hmmm" | ||
| facts.append(_fact(context, reader_id=reader_id, kind="dependency", scope="requirements-document", identity=f"requirement:{order}", location={"start_line": line_number, "end_line": line_number}, value={"requirement": published, "name": name, "class": "runtime-or-unresolved", "order": order}, native_id=name if name != "hmmm" else None, convention="python.requirements-file", standing="declared", projection={"mapping_version": "requirements-lines@1", "relation": "depends_on", "loss": "resolution and environment markers not evaluated"})) | ||
| if name != "hmmm": | ||
| edges.append(_edge(subject, f"python-package:{name}", "depends_on:runtime-or-unresolved")) |
There was a problem hiding this comment.
For valid pip requirement lines that are bare archive or VCS URLs, such as https://example.com/pkg.whl, this regex treats the URL scheme as the package name and emits a false python-package:https dependency edge. Because no unresolved diagnostic is produced, the file is marked supported and --require-source requirements.txt --strict succeeds with incorrect dependency metadata; recognize URL forms and retain their package identity as hmmm unless it is explicitly supplied.
AGENTS.md reference: AGENTS.md:L118-L124
Useful? React with 👍 / 👎.
| if (ts.isImportDeclaration(node) || ts.isExportDeclaration(node)) { | ||
| 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)}); |
There was a problem hiding this comment.
For the static TypeScript form import fs = require("fs"), the compiler produces an ImportEqualsDeclaration, so this branch emits neither an import fact nor a dependency edge; the later call-expression check also does not see the ExternalModuleReference. The source is consequently reported as fully supported while a declared dependency is silently absent, so handle this declaration kind or mark it unresolved.
AGENTS.md reference: AGENTS.md:L120-L124
Useful? React with 👍 / 👎.
| payload = json.dumps(collection, indent=2, sort_keys=True, ensure_ascii=False, allow_nan=False) | ||
| return f'import {{ {imports} }} from "{import_path}";\n\nexport default {helper}({payload});\n' |
There was a problem hiding this comment.
For JSON containing an escaped lone surrogate such as {"x":"\ud800"}, parse_json accepts the value and collection completes without a diagnostic, but this ensure_ascii=False serialization leaves the surrogate in the rendered string. The subsequent UTF-8 encode in --check or output writing then raises UnicodeEncodeError, aborting the collector instead of producing a controlled invalid-input result; either reject lone surrogates during parsing or keep them escaped in output.
Useful? React with 👍 / 👎.
| text = data.decode('utf-8-sig') | ||
| 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 scan rejects DOCTYPE, ENTITY, and non-UTF-8 encoding= text wherever it appears, including harmless XML comments or CDATA. Valid inputs such as <root><!-- <!DOCTYPE example> --></root> are therefore marked invalid, and the same false rejection affects SVG collection; detect actual XML declarations rather than matching their spelling inside character data.
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.
The implicit float resolver accepts 1e2e3 because the exponent-bearing alternative can be followed by a second optional exponent. That token is a valid YAML plain string, but it is tagged as a float and then rejected by scalar, causing the whole source to be reported invalid; constrain the resolver so it assigns the float tag only to numeric forms the scalar conversion accepts.
Useful? React with 👍 / 👎.
| for line_number, line in enumerate(text.splitlines(), start=1): | ||
| heading = _LLMS_HEADING_RE.match(line) | ||
| if heading: | ||
| title = heading.group("title") | ||
| facts.append(_fact(context, reader_id=reader_id, kind="instruction-section", scope="document-section", identity=f"heading:{line_number}", location={"start_line": line_number, "end_line": line_number}, value={"level": len(heading.group("marks")), "title": title, "generated": "hmmm"}, native_id=title, convention="llms.txt", standing="declared")) | ||
| continue | ||
| definition = _LLMS_DEFINITION_RE.match(line) |
There was a problem hiding this comment.
For an llms.txt containing a fenced Markdown example, lines such as # Example only or - **fake** = sample inside the fence are emitted as declared instruction sections and key definitions. These lines are code-block content rather than active Markdown structure, so downstream instruction inventories receive false facts without any diagnostic; track fenced-block state before applying the line-level patterns.
Useful? React with 👍 / 👎.
| if not re.fullmatch(r'https?://json-schema\.org/(?:draft/(?:2020-12|2019-09)|draft-0[467])/schema#?', version): | ||
| unsupported('unsupported_json_schema_draft', 'preserved schema; semantic extraction tested for drafts 4/6/7/2019-09/2020-12') |
There was a problem hiding this comment.
A schema URI such as https://JSON-SCHEMA.ORG/draft/2020-12/schema passes is_json_schema_uri, which correctly treats the host case-insensitively, but this case-sensitive draft regex then labels the same supported URI as an unsupported draft. The file becomes partial and fails required-source coverage even though it names the supported 2020-12 schema; normalize the parsed scheme/authority or apply case-insensitive matching to those URI components.
Useful? React with 👍 / 👎.
2 participants
Delivered repair
Replaces the block-only default collector with schema-2 native metadata collection. Existing declarations are consumed at their owning sources; no MSDMD redeclaration is required.
Final verification receipt
Base:
e6e3e5cd2886eb2c4cf43744a9ed6b6f7ab3b535.Final source head:
4f469ce11fcccc1077d18b5ed3458eb8f5160e8c.Final tree:
0e1067f0a878adf3677f2b2941a8b0fa642be447, exactly matching the fully validated hosted worktree.Latest product repair:
eed15bfbc7bf2eff9b61f0959ce9d564387a536c.Temporary repair executors have been removed.
Hosted verification: https://github.com/The-Interdependency/skill-lib/actions/runs/36445543708
Generated collection SHA-256:
317e3dcc403b87d60e4580885188dee7bb780c41a53f69d7dbca351ec75f57e1.Review repairs
Both Codex review rounds have recorded dispositions; all 17 inline threads are resolved. Final repairs cover XML credential redaction before projection, default TypeScript exports, lexical comment ownership, dynamic Python
__all__, repeated SPDX identities, universal-parser implementation hashing and Gitignore trailing-space semantics. Earlier repairs cover supplemental-block retention on parser failure, systemd directives/redaction, unresolved requirements includes, JSON numeric fidelity, local exports and Maven/XML discrimination. The YAML float finding was tested and not reproduced; no unnecessary parser replacement was made.The initial final-round run passed all 13 new regressions but correctly failed stale collection replay. Regeneration preceded the successful complete replay; no gate was disabled or weakened. Anchored implementer reviews document repair evidence and are not independent approval.
Usage
python -m pip install -r msdmd/requirements.txt npm ci --ignore-scripts --prefix msdmd python -m msdmd.collect --root . --repo The-Interdependency/skill-lib --snapshot-identity --import-path ./msdmd/collection --out skill-lib_msdmd.ts --strict --checkhmmm / scope limits
Reader support means the explicit tested extraction subsets in
msdmd/references/implemented-readers.md, not full standards certification. Ownership path applicability, cross-source semantic reconciliation, compiler/build context, arbitrary-language coverage and independent evidence/signature verification remain outside these readers. Consumer repositories have not been updated by this PR. Source extraction is not verified program behavior.