-
Notifications
You must be signed in to change notification settings - Fork 65
fix(serve): dispatch /rpc off the event loop #590
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e0ad6e3
8a0c9df
d23d10e
78877d8
8b5a377
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -66,6 +66,21 @@ def resolve_console_dir( | |
| return None | ||
|
|
||
|
|
||
| # The SPA shell names the hashed bundle, so it must be revalidated on every | ||
| # load. Served with only Last-Modified a browser applies heuristic freshness | ||
| # and keeps replaying the shell it cached before — after an upgrade that means | ||
| # the previous console build running against the new backend, which fails in | ||
| # whatever way the response shapes changed. `no-cache` still allows a 304 off | ||
| # the ETag, so the cost is one conditional request. Everything vite emits into | ||
| # assets/ is content-hashed and can be kept forever. | ||
|
Comment on lines
+69
to
+75
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win keep the added comment prose lowercase. The new comment block uses sentence case and uppercase prose. Please lowercase it while preserving literal protocol names where necessary. As per path instructions, “use lowercase prose in comments and review notes.” 🤖 Prompt for AI AgentsSource: Path instructions |
||
| _SHELL_CACHE = {"cache-control": "no-cache"} | ||
| _ASSET_CACHE = {"cache-control": "public, max-age=31536000, immutable"} | ||
|
|
||
|
|
||
| def _cache_headers(rel: str) -> dict[str, str]: | ||
| return _ASSET_CACHE if rel.startswith("assets/") else _SHELL_CACHE | ||
|
Comment on lines
+80
to
+81
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | grep -E '(^|/)console\.py$|AGENTS\.md|CLAUDE\.md|pytest|requirements|pyproject|README' | sed -n '1,200p'
echo "== console outline =="
ast-grep outline src/vouch/web/console.py --view expanded || true
echo "== relevant console sections =="
cat -n src/vouch/web/console.py | sed -n '1,120p'
printf '\n--- 180-215 ---\n'
cat -n src/vouch/web/console.py | sed -n '180,215p'
echo "== tests mentioning console/static/assets/cache =="
rg -n "_cache_headers|assets/|static|FileResponse|index.html|dot|\\./|\\.\\." tests src/vouch -S || trueRepository: vouchdev/vouch Length of output: 50371 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== test_console relevant sections =="
cat -n tests/test_console.py | sed -n '88,155p'
printf '\n--- tests mentioning console path params or console app ---\n'
rg -n "build_console_app|full_path|assets/|index\\.html|dot|\\./|\\.\\." tests/test_console.py -S
echo "== behavioral probe: path normalization in current logic =="
python3 - <<'PY'
from pathlib import Path
root = Path("/tmp/webapp/dist")
index = root / "index.html"
for rel in ("assets/app.js", "../index.html", "assets/../index.html"):
candidate = (root / rel).resolve()
try:
resolved_rel = candidate.relative_to(root).as_posix()
except ValueError:
resolved_rel = "(rejected)"
print(f"{rel:25} candidate={candidate} file={candidate.is_file()} rel_prefix={rel.startswith('assets/')} resolved_rel={resolved_rel}")
PYRepository: vouchdev/vouch Length of output: 5415 classify the resolved path, not the raw request path. An in-root alias such as 🤖 Prompt for AI Agents |
||
|
|
||
|
|
||
| def _err(status: int, code: str, message: str) -> JSONResponse: | ||
| """The vouch-native error envelope the SPA already understands.""" | ||
| return JSONResponse( | ||
|
|
@@ -182,8 +197,8 @@ async def _spa(request: Request) -> Response: | |
| except ValueError: | ||
| candidate = index | ||
| if candidate.is_file(): | ||
| return FileResponse(candidate) | ||
| return FileResponse(index) | ||
| return FileResponse(candidate, headers=_cache_headers(rel)) | ||
| return FileResponse(index, headers=_SHELL_CACHE) | ||
|
|
||
| routes = [ | ||
| Route("/proxy", _proxy, methods=_PROXY_METHODS), | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: vouchdev/vouch
Length of output: 10649
🏁 Script executed:
Repository: vouchdev/vouch
Length of output: 20987
🏁 Script executed:
Repository: vouchdev/vouch
Length of output: 20500
🏁 Script executed:
Repository: vouchdev/vouch
Length of output: 31179
🏁 Script executed:
Repository: vouchdev/vouch
Length of output: 32128
Serialize or lock KB mutations before offloading
/rpcto threadpool workers.handle_requestnow calls synchronous MCP handlers from real worker threads, and/rpcdispatches the same handlers. The approval flow performs read-then-write checks aroundstore.put_*/update_*,move_proposal_to_decided(), and audit indexing without a cross-thread/global guard, so concurrent/mcpor/rpcwrites can interleave and leave artifacts/audit state inconsistent. The existing lock is only aroundlog_event; it should cover the full mutating approval write path or the workers should be serialized at the request boundary before the storage layer changes.🤖 Prompt for AI Agents
Source: Path instructions