mirror of
https://github.com/HKUDS/CLI-Anything.git
synced 2026-09-01 15:36:07 +08:00
fix(joplin):timeout and node
This commit is contained in:
+299
-299
File diff suppressed because it is too large
Load Diff
+270
-270
File diff suppressed because it is too large
Load Diff
@@ -125,8 +125,8 @@ For real backend runs, ensure `joplin` is installed and available in `PATH`.
|
||||
|
||||
Current validation baseline (Windows + Joplin CLI 3.6.2):
|
||||
|
||||
- `python -m pytest -q cli_anything/joplin/tests/test_core.py` �`85 passed, 1 skipped`
|
||||
- `python -m pytest -q cli_anything/joplin/tests` �`112 passed, 2 skipped`
|
||||
- `python -m pytest -q cli_anything/joplin/tests/test_core.py` �`91 passed`
|
||||
- `python -m pytest -q cli_anything/joplin/tests` �`118 passed, 1 skipped`
|
||||
|
||||
## Development notes
|
||||
|
||||
|
||||
@@ -65,9 +65,9 @@ cli-anything-joplin --dry-run --project ./demo.joplin-harness.json todos create
|
||||
|
||||
Current validation baseline (Windows + Joplin CLI 3.6.2):
|
||||
|
||||
- `python -m pytest -q cli_anything/joplin/tests/test_core.py` �`85 passed, 1 skipped`
|
||||
- `python -m pytest -q cli_anything/joplin/tests` �`112 passed, 2 skipped`
|
||||
- `python -m pytest -q cli_anything/joplin/tests/test_core.py` �`91 passed`
|
||||
- `python -m pytest -q cli_anything/joplin/tests` �`118 passed, 1 skipped`
|
||||
|
||||
See `cli_anything/joplin/tests/TEST.md` for the full plan (85 unit + 27 e2e,
|
||||
See `cli_anything/joplin/tests/TEST.md` for the full plan (91 unit + 27 e2e,
|
||||
2 skipped on Windows) and `cli_anything/joplin/WORKFLOWS.md` for the verified
|
||||
workflow inventory.
|
||||
|
||||
@@ -121,8 +121,8 @@ CLI_ANYTHING_FORCE_INSTALLED=1 python -m pytest -v -s cli_anything/joplin/tests/
|
||||
|
||||
Current validation baseline (Windows + Joplin CLI 3.6.2):
|
||||
|
||||
- `python -m pytest -q cli_anything/joplin/tests/test_core.py` �`85 passed, 1 skipped`
|
||||
- `python -m pytest -q cli_anything/joplin/tests` �`112 passed, 2 skipped`
|
||||
- `python -m pytest -q cli_anything/joplin/tests/test_core.py` �`91 passed`
|
||||
- `python -m pytest -q cli_anything/joplin/tests` �`118 passed, 1 skipped`
|
||||
|
||||
See `cli_anything/joplin/tests/TEST.md` for the full test plan and
|
||||
`cli_anything/joplin/WORKFLOWS.md` for the verified workflow inventory.
|
||||
|
||||
@@ -79,7 +79,7 @@ tag.add, tag.remove, attach.add, interop.export, note.remove, notebook.remove
|
||||
|
||||
| Layer | File / class | Backend needed | Workflows |
|
||||
|--------------------------|--------------------------------------|----------------|---------------------------------|
|
||||
| Unit + CLI contract | `test_core.py` | No | 85 tests (1 skipped on Windows) |
|
||||
| Unit + CLI contract | `test_core.py` | No | 91 tests |
|
||||
| CLI subprocess | `TestCLISubprocess` | No | 10 tests |
|
||||
| Real-backend commands | `TestBackendCommands` | Yes | 6 tests |
|
||||
| Real-backend workflows | `TestBackendWorkflows` | Yes | 11 tests (1 skipped on Windows) |
|
||||
@@ -105,7 +105,7 @@ tag.add, tag.remove, attach.add, interop.export, note.remove, notebook.remove
|
||||
non-zero exit code is a real failure; they are not stripped from the returned
|
||||
streams (so multi-line note bodies and exports stay intact).
|
||||
- JSON error envelopes use the same `command` identifier as success responses
|
||||
(`config.import_file`, `backend.export_sync_status`, `e2ee.decrypt_file`, …).
|
||||
(`config.import_file`, `backend.export_sync_status`, `e2ee.decrypt_file`, ?.
|
||||
|
||||
## 7. Known limitations
|
||||
|
||||
|
||||
@@ -155,7 +155,13 @@ def server_start(config: BackendConfig, exit_early: bool = True, quiet: bool = F
|
||||
args.append("--exit-early")
|
||||
if quiet:
|
||||
args.append("--quiet")
|
||||
return run_joplin_command(args, config, timeout=300)
|
||||
# When exit_early is True the CLI returns as soon as the server is up
|
||||
# (~5 s), so a 300 s safety cap is reasonable. When exit_early is False
|
||||
# the CLI intentionally blocks for the lifetime of the server process;
|
||||
# imposing any fixed timeout would terminate a legitimately long-running
|
||||
# server, so we pass None (no timeout) in that mode.
|
||||
timeout = 300 if exit_early else None
|
||||
return run_joplin_command(args, config, timeout=timeout)
|
||||
|
||||
|
||||
def server_stop(config: BackendConfig) -> dict:
|
||||
|
||||
@@ -82,7 +82,7 @@ When `--json` is enabled, commands return:
|
||||
- Backend `stdout`/`stderr` in command results are verbatim; do not assume
|
||||
warnings were stripped from note bodies or exports.
|
||||
- On failure, parse `error` using the same `command` field as success
|
||||
(`config.import_file`, `e2ee.decrypt_file`, …). Multi-word subcommands use
|
||||
(`config.import_file`, `e2ee.decrypt_file`, ?. Multi-word subcommands use
|
||||
a single dot between group and subcommand.
|
||||
|
||||
## Test workflow
|
||||
@@ -106,5 +106,5 @@ CLI_ANYTHING_FORCE_INSTALLED=1 python -m pytest -v -s cli_anything/joplin/tests/
|
||||
|
||||
Current validation baseline (Windows + Joplin CLI 3.6.2):
|
||||
|
||||
- `python -m pytest -q cli_anything/joplin/tests/test_core.py` �`85 passed, 1 skipped`
|
||||
- `python -m pytest -q cli_anything/joplin/tests` �`112 passed, 2 skipped`
|
||||
- `python -m pytest -q cli_anything/joplin/tests/test_core.py` �`91 passed`
|
||||
- `python -m pytest -q cli_anything/joplin/tests` �`118 passed, 1 skipped`
|
||||
|
||||
@@ -8,7 +8,7 @@ The suite has two files:
|
||||
|
||||
Real-backend classes are skipped automatically when `joplin` is not on `PATH`.
|
||||
|
||||
## test_core.py (85 tests, 1 skipped on Windows)
|
||||
## test_core.py (91 tests)
|
||||
|
||||
Pure Python tests covering the harness surface. Safe to run anywhere without
|
||||
a Joplin backend.
|
||||
@@ -24,6 +24,14 @@ Coverage areas:
|
||||
benign Node warning handling (scrubbed copy used only for the non-zero exit
|
||||
decision; returned `stdout`/`stderr` stay verbatim), mixed warning+real-error
|
||||
surfacing, timeout, JSON parse fallback, empty stdout
|
||||
- Node warning continuation line (`(Use \`node --trace-deprecation …\`)`):
|
||||
the hint line Node appends after each deprecation warning is now also dropped
|
||||
during scrubbing so warning-only stderr no longer triggers a false
|
||||
`RuntimeError`; `run_joplin_json` parses JSON even when warning + hint prefix
|
||||
the payload; real errors after warning+hint are still surfaced
|
||||
- `server_start` timeout: `exit_early=True` uses a finite cap (300 s);
|
||||
`exit_early=False` passes `timeout=None` so a long-lived `--wait` server
|
||||
process is never killed by a hard deadline
|
||||
- Backend stdout preservation: multi-paragraph note bodies keep internal blank
|
||||
lines; `run_joplin_json` parses JSON when a Node warning prefixes stdout
|
||||
- Error envelope command ID consistency: failures on `config import-file`,
|
||||
|
||||
@@ -316,6 +316,76 @@ def test_backend_run_command_node_warning_passes(monkeypatch):
|
||||
assert result["stderr"] == _WARNING
|
||||
|
||||
|
||||
def test_backend_run_command_node_warning_with_continuation_passes(monkeypatch):
|
||||
"""Regression (P1): Node always appends a hint line after each warning:
|
||||
|
||||
(node:…) [DEP0040] DeprecationWarning: …
|
||||
(Use `node --trace-deprecation …` to show where the warning was created)
|
||||
|
||||
Before the fix, the continuation line was kept after scrubbing, making
|
||||
scrubbed_stderr non-empty, which caused run_joplin_command to raise even
|
||||
though the exit was caused only by benign Node noise.
|
||||
"""
|
||||
_WARN = "(node:1234) [DEP0040] DeprecationWarning: The `punycode` module is deprecated."
|
||||
_HINT = "(Use `node --trace-deprecation ...` to show where the warning was created)"
|
||||
_STDERR = f"{_WARN}\n{_HINT}"
|
||||
|
||||
class Proc:
|
||||
returncode = 1
|
||||
stdout = ""
|
||||
stderr = _STDERR
|
||||
|
||||
monkeypatch.setattr(joplin_backend, "find_joplin", lambda _: "joplin")
|
||||
monkeypatch.setattr(joplin_backend.subprocess, "run", lambda *a, **k: Proc())
|
||||
cfg = joplin_backend.BackendConfig(binary="joplin", profile=None)
|
||||
# Must not raise — both lines are benign warning/hint noise.
|
||||
result = joplin_backend.run_joplin_command(["ls"], cfg)
|
||||
assert result["returncode"] == 1
|
||||
assert result["stderr"] == _STDERR # raw stream still intact
|
||||
|
||||
|
||||
def test_backend_run_json_parses_despite_warning_and_continuation_prefix(monkeypatch):
|
||||
"""run_joplin_json must succeed when both the warning AND its hint line
|
||||
are emitted to stdout ahead of the JSON payload."""
|
||||
_WARN = "(node:7) [DEP0040] DeprecationWarning: The `punycode` module is deprecated."
|
||||
_HINT = "(Use `node --trace-deprecation ...` to show where the warning was created)"
|
||||
_JSON = '[{"title":"NoteA"}]'
|
||||
|
||||
class Proc:
|
||||
returncode = 0
|
||||
stdout = f"{_WARN}\n{_HINT}\n{_JSON}"
|
||||
stderr = ""
|
||||
|
||||
monkeypatch.setattr(joplin_backend, "find_joplin", lambda _: "joplin")
|
||||
monkeypatch.setattr(joplin_backend.subprocess, "run", lambda *a, **k: Proc())
|
||||
cfg = joplin_backend.BackendConfig(binary="joplin", profile=None)
|
||||
result = joplin_backend.run_joplin_json(["ls", "--format", "json"], cfg)
|
||||
assert isinstance(result["data"], list)
|
||||
assert result["data"][0]["title"] == "NoteA"
|
||||
|
||||
|
||||
def test_backend_run_command_real_error_not_masked_by_warning_with_hint(monkeypatch):
|
||||
"""A real Joplin error must surface even when stderr contains both the
|
||||
benign warning AND its continuation hint line before the error text."""
|
||||
|
||||
class Proc:
|
||||
returncode = 1
|
||||
stdout = ""
|
||||
stderr = (
|
||||
"(node:1234) [DEP0040] DeprecationWarning: The `punycode` module is deprecated.\n"
|
||||
"(Use `node --trace-deprecation ...` to show where the warning was created)\n"
|
||||
"Error: Cannot find \"missing-note\"."
|
||||
)
|
||||
|
||||
monkeypatch.setattr(joplin_backend, "find_joplin", lambda _: "joplin")
|
||||
monkeypatch.setattr(joplin_backend.subprocess, "run", lambda *a, **k: Proc())
|
||||
cfg = joplin_backend.BackendConfig(binary="joplin", profile=None)
|
||||
with pytest.raises(RuntimeError) as excinfo:
|
||||
joplin_backend.run_joplin_command(["cat", "missing-note"], cfg)
|
||||
assert "Cannot find" in str(excinfo.value)
|
||||
assert "DEP0040" not in str(excinfo.value)
|
||||
|
||||
|
||||
def test_backend_run_command_real_error_alongside_warning_raises(monkeypatch):
|
||||
"""Regression: a real Joplin error must surface even when stderr also
|
||||
contains the benign DEP0040 punycode deprecation warning."""
|
||||
@@ -414,6 +484,44 @@ def test_backend_run_command_timeout(monkeypatch):
|
||||
assert "timed out" in str(excinfo.value)
|
||||
|
||||
|
||||
def test_backend_server_start_exit_early_uses_finite_timeout(monkeypatch):
|
||||
"""P2 regression: server_start(exit_early=True) must pass a finite timeout."""
|
||||
captured = {}
|
||||
|
||||
class Proc:
|
||||
returncode = 0
|
||||
stdout = "Server started"
|
||||
stderr = ""
|
||||
|
||||
def fake_run(cmd, *a, **k):
|
||||
captured["timeout"] = k.get("timeout")
|
||||
return Proc()
|
||||
|
||||
monkeypatch.setattr(backend_core, "find_joplin", lambda _: "joplin")
|
||||
monkeypatch.setattr(backend_core, "run_joplin_command",
|
||||
lambda args, cfg, timeout=120: captured.update({"timeout": timeout}) or
|
||||
{"command": args, "returncode": 0, "stdout": "", "stderr": ""})
|
||||
cfg = joplin_backend.BackendConfig()
|
||||
backend_core.server_start(cfg, exit_early=True)
|
||||
assert captured["timeout"] is not None
|
||||
assert captured["timeout"] > 0
|
||||
|
||||
|
||||
def test_backend_server_start_no_exit_early_uses_no_timeout(monkeypatch):
|
||||
"""P2 regression: server_start(exit_early=False) must pass timeout=None so
|
||||
a long-lived server process is never killed by a hard deadline."""
|
||||
captured = {}
|
||||
|
||||
monkeypatch.setattr(backend_core, "run_joplin_command",
|
||||
lambda args, cfg, timeout=120: captured.update({"timeout": timeout}) or
|
||||
{"command": args, "returncode": 0, "stdout": "", "stderr": ""})
|
||||
cfg = joplin_backend.BackendConfig()
|
||||
backend_core.server_start(cfg, exit_early=False)
|
||||
assert captured["timeout"] is None, (
|
||||
f"Expected timeout=None for --wait mode, got {captured['timeout']}"
|
||||
)
|
||||
|
||||
|
||||
def test_backend_run_json_parse(monkeypatch):
|
||||
monkeypatch.setattr(
|
||||
joplin_backend,
|
||||
|
||||
@@ -39,15 +39,37 @@ def _line_is_benign_node_warning(line: str) -> bool:
|
||||
return any(all(marker in lowered for marker in markers) for markers in _BENIGN_NODE_WARNING_MARKERS)
|
||||
|
||||
|
||||
def _line_is_node_warning_continuation(line: str) -> bool:
|
||||
"""Return True for the hint line that Node prints after each deprecation warning.
|
||||
|
||||
Node consistently follows each DeprecationWarning with a one-line hint of
|
||||
the form ``(Use `node --trace-deprecation ...` to show where the warning
|
||||
was created)``. That line is *not* a warning header, so
|
||||
``_line_is_benign_node_warning`` does not match it. Without this check,
|
||||
the continuation line is kept and makes the scrubbed text non-empty,
|
||||
causing ``run_joplin_command`` to raise on what are purely benign warnings.
|
||||
"""
|
||||
stripped = line.strip()
|
||||
lowered = stripped.lower()
|
||||
# The hint always starts with "(use" and references --trace-deprecation.
|
||||
return stripped.startswith("(Use ") or (
|
||||
lowered.startswith("(use") and "--trace-deprecation" in lowered
|
||||
)
|
||||
|
||||
|
||||
def _strip_benign_node_warnings(text: str) -> str:
|
||||
"""Return ``text`` with known-benign Node warning lines removed.
|
||||
|
||||
The filter is line-based and only drops the Node warning lines themselves
|
||||
(and any blank padding emitted around them). Real diagnostic content is
|
||||
preserved verbatim, including blank lines between paragraphs. Callers
|
||||
must NOT use the returned value to replace ``stdout`` payloads -- it is
|
||||
only meant for the success/failure decision, where collapsing trailing
|
||||
blank padding around dropped warning lines is desirable.
|
||||
The filter is line-based and drops:
|
||||
- Lines matching ``_line_is_benign_node_warning``
|
||||
- The immediately following hint/continuation line if it matches
|
||||
``_line_is_node_warning_continuation`` (e.g. the "Use node
|
||||
--trace-deprecation ..." line Node always emits after each warning)
|
||||
- Blank padding lines that hug a dropped line
|
||||
|
||||
Real diagnostic content is preserved verbatim, including blank lines
|
||||
between paragraphs. Callers must NOT use the returned value to replace
|
||||
``stdout`` payloads -- it is only meant for the success/failure decision.
|
||||
"""
|
||||
if not text:
|
||||
return ""
|
||||
@@ -55,19 +77,24 @@ def _strip_benign_node_warnings(text: str) -> str:
|
||||
benign_indices: set[int] = set()
|
||||
for idx, raw_line in enumerate(lines):
|
||||
stripped = raw_line.strip()
|
||||
if stripped and _line_is_benign_node_warning(stripped):
|
||||
if not stripped:
|
||||
continue
|
||||
if _line_is_benign_node_warning(stripped):
|
||||
benign_indices.add(idx)
|
||||
# Also drop the continuation/hint line that Node appends right
|
||||
# after each deprecation warning.
|
||||
next_idx = idx + 1
|
||||
if next_idx < len(lines) and _line_is_node_warning_continuation(lines[next_idx]):
|
||||
benign_indices.add(next_idx)
|
||||
if not benign_indices:
|
||||
return text.strip()
|
||||
# Drop the warning lines themselves and any blank padding that hugs them
|
||||
# (Node tends to emit a blank line before and after each warning), but
|
||||
# keep blank lines that are part of legitimate payload structure.
|
||||
# Drop the warning/continuation lines and blank padding around them,
|
||||
# but keep blank lines that are part of legitimate payload structure.
|
||||
kept: list[str] = []
|
||||
for idx, raw_line in enumerate(lines):
|
||||
if idx in benign_indices:
|
||||
continue
|
||||
if raw_line.strip() == "":
|
||||
# Drop the blank line only when it is sandwiched against a warning
|
||||
prev_is_warning = (idx - 1) in benign_indices
|
||||
next_is_warning = (idx + 1) in benign_indices
|
||||
if prev_is_warning or next_is_warning:
|
||||
@@ -76,7 +103,7 @@ def _strip_benign_node_warnings(text: str) -> str:
|
||||
return "\n".join(kept).strip()
|
||||
|
||||
|
||||
def run_joplin_command(args: list[str], config: BackendConfig, timeout: int = 120) -> dict:
|
||||
def run_joplin_command(args: list[str], config: BackendConfig, timeout: Optional[int] = 120) -> dict:
|
||||
binary = find_joplin(config.binary)
|
||||
cmd = [binary]
|
||||
if config.profile:
|
||||
@@ -92,7 +119,7 @@ def run_joplin_command(args: list[str], config: BackendConfig, timeout: int = 12
|
||||
check=False,
|
||||
)
|
||||
except subprocess.TimeoutExpired as e:
|
||||
raise RuntimeError(f"Joplin command timed out after {timeout}s") from e
|
||||
raise RuntimeError(f"Joplin command timed out after {timeout}s") from e # timeout is non-None here
|
||||
|
||||
# IMPORTANT: stdout is the caller's payload (note bodies, JSON dumps,
|
||||
# config exports, ...). Preserve it verbatim -- collapsing blank lines or
|
||||
@@ -122,7 +149,7 @@ def run_joplin_command(args: list[str], config: BackendConfig, timeout: int = 12
|
||||
return result
|
||||
|
||||
|
||||
def run_joplin_json(args: list[str], config: BackendConfig, timeout: int = 120) -> dict:
|
||||
def run_joplin_json(args: list[str], config: BackendConfig, timeout: Optional[int] = 120) -> dict:
|
||||
command_args = args if "--format" in args or "-f" in args else args + ["--format", "json"]
|
||||
raw = run_joplin_command(command_args, config, timeout=timeout)
|
||||
text = raw["stdout"]
|
||||
|
||||
@@ -83,7 +83,7 @@ When `--json` is enabled, commands return:
|
||||
- Backend `stdout`/`stderr` in command results are verbatim; do not assume
|
||||
warnings were stripped from note bodies or exports.
|
||||
- On failure, parse `error` using the same `command` field as success
|
||||
(`config.import_file`, `e2ee.decrypt_file`, …). Multi-word subcommands use
|
||||
(`config.import_file`, `e2ee.decrypt_file`, ?. Multi-word subcommands use
|
||||
a single dot between group and subcommand.
|
||||
|
||||
## Recommended workflow
|
||||
@@ -98,5 +98,5 @@ When `--json` is enabled, commands return:
|
||||
|
||||
Current validation baseline (Windows + Joplin CLI 3.6.2):
|
||||
|
||||
- `python -m pytest -q cli_anything/joplin/tests/test_core.py` �`85 passed, 1 skipped`
|
||||
- `python -m pytest -q cli_anything/joplin/tests` �`112 passed, 2 skipped`
|
||||
- `python -m pytest -q cli_anything/joplin/tests/test_core.py` �`91 passed`
|
||||
- `python -m pytest -q cli_anything/joplin/tests` �`118 passed, 1 skipped`
|
||||
|
||||
Reference in New Issue
Block a user