fix: enforce repository contracts and reduce query module debt - #143
Merged
Conversation
Three review-findings fixes, each with sidecar test coverage: - agent/ai_transcript: warn once per malformed Gemini content revision instead of on every poll cycle, and clear the suppression entry when the file parses again. The suppression map is deliberately not persisted. - command_log: resolve the hostname via the shared scanner::local_hostname() OS resolver rather than the HOSTNAME env var with a localhost fallback. - receiver/listener: drain oversized TCP frames up to a bounded multiple so the connection resumes at the next frame, and fix CR detection when the delimiter lands on a buffer boundary. Unterminated senders are cut off at 8x the max message size rather than pinning a TCP slot. Also carried in this commit (authored concurrently, outside this session's verification scope): - lab-auth git dependency moved to dinglebear-ai/labby at the same rev - removal of the no-mcp marketplace variant machinery (workflows, scripts, plugin hooks, and associated docs) - OTLP and plugin-option doc/comment corrections - session log for 2026-07-27 Verified for the three fixes above: cargo fmt --check clean, cargo clippy --all-targets --all-features clean, and targeted module tests pass (51 and 11 passed, 0 failed). The full cargo test --lib suite exceeded a ten-minute local timeout and was not run to completion; CI covers it. Co-authored-by: Claude <noreply@anthropic.com>
A four-agent review of the full PR diff surfaced one latent data-loss bug that three reviewers independently converged on, plus a CI blocker. Critical: - deny.toml still allowlisted github.com/jmagar/lab, so the required `deny` job would fail on the lab-auth source move. `cargo deny check sources` now reports `sources ok`. - receiver/listener: an at-limit payload terminated by CRLF was accepted or dropped purely as a function of where the TCP read boundary landed. The accumulate guard compared raw bytes (CR included) against max_size while the newline branch compared payload bytes, so the two disagreed by exactly one byte at the limit. Reachable in production: an 8 KiB BufReader with the default 8192-byte max_message_size. The accumulate limit now reserves one byte for a split CRLF terminator, which also makes the pos == 0 carriage return check load-bearing — it was previously unreachable and inert. Observability: - receiver/listener: the per-frame oversize warn! lost its only rate limiter when terminated oversize frames stopped tearing the connection down. Logging now backs off exponentially (1st, 10th, 100th) with per-connection totals in the closing summary; unterminated frames still always warn. - scanner: warn once when the hostname cannot be resolved instead of silently filing rows under the literal "localhost". - scripts/ci/changed_paths.py: emit ::warning:: at each fail-open point so a silent full-matrix fallback is explainable. CI contracts: - ci.yml: the docs-contract gate was skipped for exactly the files check-public-identity.sh scans, and the aggregate gate counts skipped as success. Broadened to release/skills/docker/workflow. - check-public-identity.sh: force text mode so the gate behaves identically with and without rg on binary files. - scripts/install.sh: align CORTEX_RMCP_REPO's default with platform.js. - workflow_shapes: read the workflows directory at test time so a new workflow with no permissions block cannot slip through unasserted. Tests and conventions: - Four new listener tests: at-limit CRLF split on the CR (fails without the fix above), the over-limit counterpart, EOF mid-drain, and both halves of the drain bound. Plus exponential-backoff coverage. - Replace the tautological command_log hostname test with one that sets $HOSTNAME to a sentinel and asserts it is ignored. - Move the dedupe_hosts tests into src/db/queries_hosts_tests.rs per the sidecar convention, dropping the #[cfg(test)] re-import that existed only to keep them compiling. Docs: correct the rmcp/xtask dependency claim, a surviving hooks/ row, two stale plugin skills, the incomplete lefthook tables, and the drain-multiplier comment (the cutoff is approximate, not an exact 8x). Deferred to syslog-mcp-ex8z2 (Gemini parse-failure observability) and syslog-mcp-3zevz (ghcr.io namespace migration is a deployment decision). Verified: fmt clean, clippy 0 warnings, 62 module tests, 12 CI-contract tests, 4 docs-contract tests, cargo deny sources ok, identity check OK. Co-authored-by: Claude <noreply@anthropic.com>
…findings # Conflicts: # deny.toml
The gitleaks job was retired on main, but tests/workflow_shapes.rs still required `require_success_or_skipped gitleaks` in the ci-gate block and still asserted the pinned Gitleaks image digest. Both assertions fail against the current ci.yml, so `ci_uses_changed_path_classifier_and_stable_gate` was already red on main independently of this branch — the merge just surfaced it. Replace them with a negative assertion so the retirement itself is the contract: re-adding gitleaks now requires a deliberate test update rather than silently satisfying a stale expectation. Co-authored-by: Claude <noreply@anthropic.com>
The `npm launcher` CI job requires packages/cortex-rmcp/README.md to be byte-identical to the repo README.md. Earlier commits on this branch and on main edited the repo README — adding the OTLP token paragraph and the Claude Code plugin section — without re-running the sync, so the packaged copy was 20 lines behind and `scripts/check-package.js` failed. Regenerated with packages/cortex-rmcp/scripts/sync-readme.js; `npm run check --prefix packages/cortex-rmcp` now reports ok. Co-authored-by: Claude <noreply@anthropic.com>
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
1 participant
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.
Summary
Docs ContractCI job for embedded documentation tests and public-identity checks, and require it in the CI gatedinglebear-ai/cortexwhile preserving the intentional legacyghcr.io/jmagar/corteximage identitydb/queries.rsintodb/queries_hosts.rsxtask pre-pushinstead of reloading a login shellBacklog cleanup
cortex statusand stale schema-version trackingVerification
cargo clippy --all-targets --all-features --locked -- -D warningscargo fmt --all -- --checkdinglebear-ai/cortexPowerShell is not installed on DOOKIE, so
install.ps1could not be executed locally; its source defaults and generated metadata were updated and will be covered by repository CI/platform validation.Tracks
syslog-mcp-iv50b.Security follow-up
quinn-protofrom 0.11.14 to patched 0.11.15 for GHSA-4w2j-m93h-cj5jcontentspermission by default, with explicit publish-job write scopes preservedsyslog-mcp-iv50b.4for the remaining unfixed Debian runtime-image CVEs