docs(plans): add GOAP Wave 2-7 plan with 54 tasks, update audit with 12 new findings by d-oit · Pull Request #365 · d-oit/do-web-doc-resolver

d-oit · 2026-05-13T08:34:59Z

Summary

New: plans/16-GOAP-WAVE2-6.md — 7-wave GOAP plan with 54 tasks covering CI config, constants extraction, quality/safety fixes, Rust file splits (semantic_cache.rs 1056, config.rs 712, query.rs 527), test coverage, and cross-platform parity
Updated: plans/AUDIT.md — M7 resolved (Playwright runs 3 projects), P3 resolved (Rust --profile wired), Q4/Q5 added, 12 new findings (N1-N12)
Updated: plans/README.md — Wave 7 added, plan 16 active / 15 superseded
Condensed: 8 prior roadmap plans (01-08) into ~30-line summaries

Key Findings From Audit

ID	Issue	Severity
N1	semantic_cache.rs 1056 lines (2x limit)	P0
N2	config.rs 712 lines (over limit)	P0
N3	build_budget() duplicated in query.rs + url.rs	P1
N5	CircuitBreakerRegistry.is_open() TOCTOU	P1
N12	Raw requests.post() in synthesis — no SSRF, no retry	P1
N4	Dead Profile methods never called	P2
N6	_maybe_evict() not lock-protected	P2

Waves Defined

Wave	Focus	Est. PRs
2	CI/config fixes	~1
3	constants.py + state.py extraction	~1
4	Quality/safety fixes (TOCTOU, silent exceptions, synthesis)	~2-3
5	Rust file splits (semantic_cache, config, query)	~2
6	Test coverage (web lib, Rust resolver, skills evals)	~2
7	Web middleware + cross-platform parity	~2

Add threading locks to all 6 shared mutable state locations with threading.RLock for reentrant-safe cache access. SSRF hardening: reject ftp/ftps in is_url(), validate Mistral browser URLs, validate initial URL in web safeFetch(). Fix resolve_direct() dispatch for LLMS_TXT, SERPER, DOCLING, OCR. Fix Profile.max_hops() returning None for unknown profiles. Fix web/package.json non-existent version specifiers. Update conftest to use lock-safe clear methods. Update tests for new is_url() ftp rejection. All quality gate checks verified: 162 Python tests, 52 Rust tests, ruff, black, cargo fmt, clippy, web lint pass.

Updated next@15.3.0 and typescript@5.7.0 version specifiers in package.json need matching lockfile entries.

S3 SSRF fix added validateUrlForFetchAsync at safeFetch() entry, requiring one additional mock in the Jina proxy URL test.

- HIGH: SemanticCache SQLite thread safety — check_same_thread=False + _conn_lock - MEDIUM: CircuitBreakerRegistry Lock→RLock (recursive safe) - MEDIUM: RoutingMemory Lock→RLock + deduplicated get_domain_stats - MEDIUM: Add SSRF is_safe_url() checks to Jina and Firecrawl providers - LOW: Broad except handlers in Mistral browser -> logged exceptions

Capture self.open_until once at function entry to avoid a TOCTOU (time-of-check-time-of-use) race between the None guard on line 17 and the attribute read on line 20.

Use logger.warning('SSRF blocked: %%s', url) instead of f-string to avoid unnecessary formatting when logging disabled.

…12 new findings - Create 16-GOAP-WAVE2-6.md with 7-wave plan covering CI/config, constants/state extraction, quality/safety fixes, Rust file splits, test coverage, and cross-platform parity - Update AUDIT.md: M7 resolved (Playwright runs 3 projects), P3 resolved (Rust --profile wired), add Q4/Q5 (semantic_cache.rs 1056, config.rs 712), add 12 newly discovered issues (N1-N12), restructure priority actions - Update README.md: add Wave 7, mark plan 16 active/15 superseded - Condense 8 prior roadmap plans into summaries

vercel · 2026-05-13T08:35:04Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
do-web-doc-resolover	Ready	Preview, Comment	May 13, 2026 8:43am

deepsource-io · 2026-05-13T08:35:45Z

DeepSource Code Review

We reviewed changes in b4dd390...378fbe9 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

Important

Some issues found as part of this review are outside of the diff in this pull request and aren't shown in the inline review comments due to GitHub's API limitations. You can see those issues on the DeepSource dashboard.

PR Report Card

Overall Grade	Security Reliability Complexity Hygiene

Code Review Summary

Analyzer	Updated (UTC)	Details
JavaScript	May 13, 2026 8:42a.m.	Review ↗
Python	May 13, 2026 8:42a.m.	Review ↗
Rust	May 13, 2026 8:42a.m.	Review ↗
Shell	May 13, 2026 8:42a.m.	Review ↗

Important

AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.

deepsource-io · 2026-05-13T08:36:23Z

+# These overwrites sync sub-module state but create race conditions under
+# concurrent imports. Both conftest.py and the cascade depend on this wiring.
 scripts._query_resolve._circuit_breakers = _circuit_breakers
 scripts._query_resolve._routing_memory = _routing_memory


Access to a protected member _query_resolve of a client class

Accessing a protected member (a member prefixed with _) of a class from outside that class is not recommended, since the creator of that class did not intend this member to be exposed. If accesing this attribute outside of the class is absolutely needed, refactor it such that it becomes part of the public interface of the class.

deepsource-io · 2026-05-13T08:36:23Z

+# concurrent imports. Both conftest.py and the cascade depend on this wiring.
 scripts._query_resolve._circuit_breakers = _circuit_breakers
 scripts._query_resolve._routing_memory = _routing_memory
 scripts._url_resolve._circuit_breakers = _circuit_breakers


Access to a protected member _circuit_breakers of a client class

Accessing a protected member (a member prefixed with _) of a class from outside that class is not recommended, since the creator of that class did not intend this member to be exposed. If accesing this attribute outside of the class is absolutely needed, refactor it such that it becomes part of the public interface of the class.

deepsource-io · 2026-05-13T08:36:23Z

+# concurrent imports. Both conftest.py and the cascade depend on this wiring.
 scripts._query_resolve._circuit_breakers = _circuit_breakers
 scripts._query_resolve._routing_memory = _routing_memory
 scripts._url_resolve._circuit_breakers = _circuit_breakers


Access to a protected member _url_resolve of a client class

Accessing a protected member (a member prefixed with _) of a class from outside that class is not recommended, since the creator of that class did not intend this member to be exposed. If accesing this attribute outside of the class is absolutely needed, refactor it such that it becomes part of the public interface of the class.

deepsource-io · 2026-05-13T08:36:23Z

+            """)
+            self._conn.commit()

    def _embedding_to_blob(self, embedding: list[float]) -> bytes:


Method doesn't use the class instance and could be converted into a static method

The method doesn't use its bound instance. Decorate this method with @staticmethod decorator, so that Python does not have to instantiate a bound method for every instance of this class thereby saving memory and computation. Read more about staticmethods here.

deepsource-io · 2026-05-13T08:36:23Z



 def _get_cache():
    global _cache


Using the global statement

It is recommended not to use global statement unless it is really necessary. Global variables are dangerous because they can be simultaneously accessed from multiple sections of a program. This frequently results in bugs. This also make code difficult to read, because they force you to search through multiple functions or even modules just to understand all the different locations where the global variable is used and modified. Read more about why it should be avoided here.

deepsource-io · 2026-05-13T08:36:24Z

+# TODO(ADR-014): Replace monkey-patching with scripts/state.py singleton.
+# These overwrites sync sub-module state but create race conditions under
+# concurrent imports. Both conftest.py and the cascade depend on this wiring.
 scripts._query_resolve._circuit_breakers = _circuit_breakers


Access to a protected member _circuit_breakers of a client class

Accessing a protected member (a member prefixed with _) of a class from outside that class is not recommended, since the creator of that class did not intend this member to be exposed. If accesing this attribute outside of the class is absolutely needed, refactor it such that it becomes part of the public interface of the class.

deepsource-io · 2026-05-13T08:36:24Z

+# These overwrites sync sub-module state but create race conditions under
+# concurrent imports. Both conftest.py and the cascade depend on this wiring.
 scripts._query_resolve._circuit_breakers = _circuit_breakers
 scripts._query_resolve._routing_memory = _routing_memory


Access to a protected member _routing_memory of a client class

Accessing a protected member (a member prefixed with _) of a class from outside that class is not recommended, since the creator of that class did not intend this member to be exposed. If accesing this attribute outside of the class is absolutely needed, refactor it such that it becomes part of the public interface of the class.

deepsource-io · 2026-05-13T08:36:24Z

-            self._conn = None
+            with self._conn_lock:
+                self._conn.close()
+                self._conn = None


Attribute '_conn' defined outside __init__

Defining an instance attribute outside __init__ affects the readability of code. It is expected to find all the attributes an instance may have by reading its __init__ method. If there is a need to initialize attribute via sub-initialization methods, it is recommended to assign attributes to None in the init then call the sub-initialization methods.

deepsource-io · 2026-05-13T08:36:24Z

    Returns:
        SemanticCache instance if enabled and initialized, None otherwise
    """
    global _semantic_cache_instance


Using the global statement

It is recommended not to use global statement unless it is really necessary. Global variables are dangerous because they can be simultaneously accessed from multiple sections of a program. This frequently results in bugs. This also make code difficult to read, because they force you to search through multiple functions or even modules just to understand all the different locations where the global variable is used and modified. Read more about why it should be avoided here.

deepsource-io · 2026-05-13T08:36:24Z


 def reset_semantic_cache() -> None:
    """Reset the global semantic cache instance (mainly for testing)."""
    global _semantic_cache_instance


Using the global statement

It is recommended not to use global statement unless it is really necessary. Global variables are dangerous because they can be simultaneously accessed from multiple sections of a program. This frequently results in bugs. This also make code difficult to read, because they force you to search through multiple functions or even modules just to understand all the different locations where the global variable is used and modified. Read more about why it should be avoided here.

codacy-production · 2026-05-13T08:36:45Z

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 4 complexity · 7 duplication

Metric Results

Complexity 4

Duplication 7

View in Codacy

AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.

_{TIP This summary will be updated as you push new changes.}

codacy-production

Pull Request Overview

This PR establishes the GOAP Wave 2-7 implementation roadmap and addresses Wave 1 safety/correctness fixes. While the documentation updates are comprehensive, the implementation of thread-safety in scripts/circuit_breaker.py and scripts/utils.py contains critical gaps where locks are released prematurely, potentially leading to race conditions under high concurrency.

Codacy results indicate the project is 'Up to Standards', but substantial code duplication exists between core scripts and agent-specific directories that must be managed to prevent logic drift. Furthermore, while the plan claims to wire missing providers (LLMS_TXT, SERPER, etc.), the orchestration logic for these appears incomplete or untested in the current diff. High-risk files like circuit_breaker.py have identified logic issues combined with missing coverage, making them primary candidates for regression testing before merging.

About this PR

The documentation mentions wiring LLMS_TXT, SERPER, DOCLING, and OCR providers into resolve_direct, but this orchestration logic appears unaddressed in the current code changes. Ensure these are integrated and tested.
Finding N12 (raw requests.post) is flagged as a P1 security risk but deferred to Wave 4. Given that Wave 1 is intended as a safety and correctness push, consider moving the migration to the shared session and is_safe_url validation into this PR or the immediate next phase.

Test suggestions

Verify is_url rejects ftp and ftps schemes.
Verify safeFetch performs initial URL validation before the first fetch attempt to prevent SSRF.
Verify CircuitBreakerState.is_open captures temporal state once to prevent TOCTOU races.
Verify resolve_direct correctly handles LLMS_TXT, SERPER, DOCLING, and OCR providers.
Verify RoutingMemory ranking remains stable and thread-safe under concurrent access via RLock.
Increase unit test coverage for scripts/semantic_cache.py and scripts/routing_memory.py to address high-complexity uncovered paths.

Prompt proposal for missing tests

Consider implementing these tests if applicable:
1. Verify resolve_direct correctly handles LLMS_TXT, SERPER, DOCLING, and OCR providers.
2. Increase unit test coverage for `scripts/semantic_cache.py` and `scripts/routing_memory.py` to address high-complexity uncovered paths.

_{TIP Improve review quality by adding custom instructions}
_{TIP How was this review? Give us feedback}

codacy-production · 2026-05-13T08:39:15Z

+_cache_lock = threading.RLock()


 def create_session_with_retry() -> requests.Session:


_{🟡 MEDIUM RISK}

There is significant code duplication between the core scripts and the .agents/skills directory. This logic drift risk should be addressed by moving shared logic into a common internal package or using symbolic links.

See Clone in Codacy

codacy-production · 2026-05-13T08:39:15Z

+    with _cache_lock:
+        cache = _get_cache()
    if not cache:
        return

    if ttl is None:
-        # Use tiered TTL based on source (provider)
        ttl = get_ttl(source)

-    cache.set(_cache_key(input_str, source), result, expire=ttl)
+    with _cache_lock:
+        cache.set(_cache_key(input_str, source), result, expire=ttl)


_{🟡 MEDIUM RISK}

Extend the lock scope to cover the entire function to maintain operation atomicity and ensure the cache proxy remains valid during the set operation.

codacy-production · 2026-05-13T08:39:15Z

+    with _cache_lock:
+        cache = _get_cache()
    if not cache:
        return None
-    result = cache.get(_cache_key(input_str, source))
+    with _cache_lock:
+        result = cache.get(_cache_key(input_str, source))
    if result is None:
        return None
    return dict(result)



_{🟡 MEDIUM RISK}

The _cache_lock (RLock) should be held for the full duration of the read-and-retrieve sequence. Releasing it before accessing the cache instance allows for other threads to modify or clear the cache proxy in the gap.

codacy-production · 2026-05-13T08:39:15Z

    def is_open(self, provider: str) -> bool:
        return self.get_breaker(provider).is_open()


_{🟡 MEDIUM RISK}

The breaker status check should be wrapped within the registry lock to ensure atomicity between retrieval and state inspection. This fulfills the requirement identified in finding N5.

Suggested change

def is_open(self, provider: str) -> bool:

return self.get_breaker(provider).is_open()

def is_open(self, provider: str) -> bool:

with self._lock:

return self.get_breaker(provider).is_open()

codacy-production · 2026-05-13T08:39:15Z

    try:
        result = urlparse(input_str)
-        return all([result.scheme in ("http", "https", "ftp", "ftps"), result.netloc])
+        return all([result.scheme in {"http", "https"}, result.netloc])


_{⚪ LOW RISK}

Suggestion: Scheme validation correctly restricted to http and https to satisfy SSRF hardening requirement S2.

codacy-production · 2026-05-13T08:39:15Z


 def resolve_with_mistral_browser(url: str, max_chars: int = MAX_CHARS) -> ResolvedResult | None:
+    if not is_safe_url(url):
+        logger.warning(f"SSRF: blocked URL {url}")


_{⚪ LOW RISK}

Nitpick: Use lazy log formatting (e.g., logger.warning('msg %s', arg)) to remain consistent with other providers and avoid unnecessary string interpolation costs when the log level is not active.

- AUDIT.md I6: markdownlint.toml MD013=false ignored by pre-commit - 16-GOAP-WAVE2-6.md K7: Fix markdownlint.toml config format (TOML not recognized; consider JSON/YAML)

do-ops885 added 10 commits May 13, 2026 09:20

fix: regenerate package-lock.json after version specifier changes

06e433e

Updated next@15.3.0 and typescript@5.7.0 version specifiers in package.json need matching lockfile entries.

fix: add third validation mock for safeFetch() initial URL check

bdae029

S3 SSRF fix added validateUrlForFetchAsync at safeFetch() entry, requiring one additional mock in the Jina proxy URL test.

fix: eliminate TOCTOU race in CircuitBreakerState.is_open()

dc26d92

Capture self.open_until once at function entry to avoid a TOCTOU (time-of-check-time-of-use) race between the None guard on line 17 and the attribute read on line 20.

docs: update GOAP_FOLLOWUP with TOCTOU race fix

f604ddf

style: black-format semantic_cache.py after SQLite changes

21db866

style: use lazy % formatting in SSRF logging (DeepSource)

b795c70

Use logger.warning('SSRF blocked: %%s', url) instead of f-string to avoid unnecessary formatting when logging disabled.

docs: mark ADR-012 Wave 1 as merged

dd9e92f

vercel Bot deployed to Preview May 13, 2026 08:35 View deployment

deepsource-io Bot reviewed May 13, 2026

View reviewed changes

codacy-production Bot reviewed May 13, 2026

View reviewed changes

docs(plans): add markdownlint config parsing issue to AUDIT and Wave 2

378fbe9

- AUDIT.md I6: markdownlint.toml MD013=false ignored by pre-commit - 16-GOAP-WAVE2-6.md K7: Fix markdownlint.toml config format (TOML not recognized; consider JSON/YAML)

vercel Bot deployed to Preview May 13, 2026 08:43 View deployment

		_cache_lock = threading.RLock()


		def create_session_with_retry() -> requests.Session:

		def is_open(self, provider: str) -> bool:
		return self.get_breaker(provider).is_open()

Conversation

d-oit commented May 13, 2026

Summary

Key Findings From Audit

Waves Defined

Uh oh!

vercel Bot commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

deepsource-io Bot commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

DeepSource Code Review

PR Report Card

Code Review Summary

Uh oh!

deepsource-io Bot May 13, 2026

Choose a reason for hiding this comment

Access to a protected member _query_resolve of a client class

Uh oh!

deepsource-io Bot May 13, 2026

Choose a reason for hiding this comment

Access to a protected member _circuit_breakers of a client class

Uh oh!

deepsource-io Bot May 13, 2026

Choose a reason for hiding this comment

Access to a protected member _url_resolve of a client class

Uh oh!

deepsource-io Bot May 13, 2026

Choose a reason for hiding this comment

Method doesn't use the class instance and could be converted into a static method

Uh oh!

deepsource-io Bot May 13, 2026

Choose a reason for hiding this comment

Using the global statement

Uh oh!

deepsource-io Bot May 13, 2026

Choose a reason for hiding this comment

Access to a protected member _circuit_breakers of a client class

Uh oh!

deepsource-io Bot May 13, 2026

Choose a reason for hiding this comment

Access to a protected member _routing_memory of a client class

Uh oh!

deepsource-io Bot May 13, 2026

Choose a reason for hiding this comment

Attribute '_conn' defined outside __init__

Uh oh!

deepsource-io Bot May 13, 2026

Choose a reason for hiding this comment

Using the global statement

Uh oh!

deepsource-io Bot May 13, 2026

Choose a reason for hiding this comment

Using the global statement

Uh oh!

codacy-production Bot commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Up to standards ✅

Uh oh!

codacy-production Bot left a comment

Choose a reason for hiding this comment

Pull Request Overview

About this PR

Test suggestions

Uh oh!

codacy-production Bot May 13, 2026

Choose a reason for hiding this comment

Uh oh!

codacy-production Bot May 13, 2026

Choose a reason for hiding this comment

Uh oh!

codacy-production Bot May 13, 2026

Choose a reason for hiding this comment

Uh oh!

codacy-production Bot May 13, 2026

Choose a reason for hiding this comment

Uh oh!

codacy-production Bot May 13, 2026

Choose a reason for hiding this comment

Uh oh!

vercel Bot commented May 13, 2026 •

edited

Loading

deepsource-io Bot commented May 13, 2026 •

edited

Loading

Attribute '_conn' defined outside init

codacy-production Bot commented May 13, 2026 •

edited

Loading