feat(MDS072): add external-link-check rule (issue #47)#707
Open
jeduden wants to merge 3 commits into
Open
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
jeduden
force-pushed
the
claude/youthful-knuth-r2pv0n
branch
from
db1565b to
ef978a7
Compare
jeduden
changed the title
jeduden
force-pushed
the
claude/youthful-knuth-r2pv0n
branch
from
ef978a7 to
f46eff6
Compare
Opt-in rule (off by default) that probes external http/https URLs with an HTTP HEAD request (falling back to GET on 405) and reports any URL that returns a transport error, a 4xx, or a 5xx response. Results are cached per URL for the run so a URL referenced in many files costs at most one request. Resolves issue #47. - AST walk collects inline links and autolinks; non-http(s) schemes and local destinations are skipped before any probe. - Config via the shared links: block (external-skip, external-timeout, external-rate-limit); keys owned by MDS027/MDS068 are tolerated. - WASM build split: probe_net.go (native HTTP) vs probe_wasm.go (no-op) keeps net/http out of the WebAssembly artifact. - RateLimit==0 sentinel keeps an unconfigured rule at zero allocs so the alloc-budget gate can run it; a perRuleAllocCeiling entry pins it. Numbered MDS072: MDS071 was taken by required-frontmatter on main. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WPyEKiuZwHDey65F8yd8jK
jeduden
force-pushed
the
claude/youthful-knuth-r2pv0n
branch
from
f46eff6 to
648a4e3
Compare
A three-angle code-review pass surfaced several defects in the first MDS072 implementation, all fixed here with regression tests: - Enable-with-`true` was a silent no-op: the bare `external-link-check: true` form leaves cfg.Settings nil, so ConfigureRule never calls ApplySettings and the RateLimit==0 "unconfigured" sentinel disabled the documented enable path. Defaults are now baked into the registered instance (newRule) so CloneInstance and the bare path inherit them, and the sentinel is gone. The network-bound alloc/timing gates skip the rule (isNetworkBound) instead of relying on the sentinel. - Autolink diagnostics anchored at (1,1): an AutoLink has no walkable Text child, so position() now locates the literal <url> in the enclosing block's source for autolinks. - external-rate-limit capped nothing: the semaphore was a per-Rule field, but the engine clones one Rule per worker; moved it (and the result cache) to package scope for a true global cap. - Concurrent double-probe: added a singleflight.Group keyed by URL so concurrent workers share one request, restoring one-request-per-run. - probe: shared client with per-request context timeouts, a bounded body drain for keep-alive, and defer-based slot release. - Images () are now probed alongside links and autolinks. Regression tests: bare-enable end-to-end via checker.ConfigureEnabledRules (internal/integration/externallink_enable_test.go), autolink position, image probing, and concurrent single-request. Package coverage 100%. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WPyEKiuZwHDey65F8yd8jK
Round 2 of the code-review pass found no functional defects; these are documentation/comment accuracy touch-ups: - Correct the isNetworkBound rationale: only the alloc-budget fixture carries an external URL; the per-rule bench doc does not, so the skip there is defensive. - Document autolinkPosition's first-match behavior for duplicate identical autolinks (matches linkstyle; minor column-only inaccuracy). - Expand the plan's known-limitations to cover the process-lifetime semaphore size freeze alongside the never-evicted cache (LSP). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WPyEKiuZwHDey65F8yd8jK
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
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
MDS072 external-link-check— an opt-in rule (off by default) that validates externalhttp://andhttps://URLs by making a HEAD request (fallback GET on 405), caching results per URL for the run, and reporting 4xx/5xx responses and transport errors as diagnostics. Resolves Add external URL link checking rule #47.plan/2606280208_external-link-check.mdtracking the implementation.externallinkininternal/rules/all/all.go.Changes
internal/rules/externallink/rule.go— MDS072 implementation:EnabledByDefault() false); only activates whenApplySettingsis called, so the alloc-budget gate sees the early-return path (0 allocs).links:block:external-skip(regex patterns),external-timeout(default 5s),external-rate-limit(default 10 concurrent). Tolerates keys owned by MDS027 (site-root,validate-images,validate-reference-style) and MDS068 (style) so one sharedlinks:block configures all three rules.sync.Mapcache (urlCache) ensures each unique URL is fetched at most once per process.r.semaphore) caps in-flight requests toexternal-rate-limit.sync.Onceinitializes thehttp.Clientand semaphore lazily on firstCheck.internal/rules/externallink/probe_net.go/probe_wasm.go— WASM build split: native HTTP probing lives behind a!(js && wasm)build tag; the wasm build carries a no-op prober sonet/httpnever inflates the WebAssembly artifact.internal/rules/externallink/rule_test.go/probe_net_test.go— unit tests usinghttptest.NewServer, covering the 200 / 404 / 405-then-GET / transport-error / cache-hit / autolink paths plus everyApplySettingsbranch (100% statement coverage ofrule.go).internal/rules/all/all.go— blank import to register MDS072.plan/2606280208_external-link-check.md— plan file, status ✅.Test plan
go build ./...— greengo test ./...— greengo tool golangci-lint run— 0 issuesgo run ./cmd/mdsmith check .— 0 failures (550 files)perRuleAllocCeiling🤖 Generated with Claude Code
https://claude.ai/code/session_01WPyEKiuZwHDey65F8yd8jK