Skip to content

Feat/core loop#21

Open
wangxingjun778 wants to merge 23 commits into
mainfrom
feat/core_loop
Open

Feat/core loop#21
wangxingjun778 wants to merge 23 commits into
mainfrom
feat/core_loop

Conversation

@wangxingjun778

Copy link
Copy Markdown
Member

No description provided.

…DUI)

Runtime: domain-neutral Watch->Finding contract, FindingStore, MonitorManager; leapd-hosted scheduler with NotificationBus push; client-coupled watches excluded from idle keep-alive.

RPC: watch.* and session.* on protocol/service/client; session transcript + LLM analysis facade.

Dashboard: SDUI (ViewSpec catalog + validation + YAML templates + bidirectional action protocol + Custom escape hatch), aiohttp view server (optional dep, graceful degrade), ViewHub WS fan-out, launcher + 'leap dashboard', /dashboard slash namespace, TUI finding cards + status-bar counts, webbrowser auto-open.

Session-analysis dashboard (domain=session) with batch/model-salience/manual refresh (salience opt-in), session-scoped finding dedup.

Domain templates (finance/sentiment/research) + overview; config dashboard.* and monitor.session_* via ConfigService.

Tests: 7 hermetic suites; full suite 811 passed, 1 skipped.
… README

Task 1: rename the slash command /dashboard* -> /board* (12 subcommands) and CLI 'leap dashboard' -> 'leap board'; launcher spawn argv updated to match; robust split-based subcommand parsing; all usage/hint strings updated; no back-compat alias.

Task 2: LeapBoard brand in the shared web shell -- Fraunces serif wordmark (Leap+Board), muted italic slogan 'Signals into insight.', hairline divider; consulting-grade, monochrome, non-intrusive; tab title branded. Config keys dashboard.* kept (server settings, not the command).

Docs: README LeapBoard section (intro, how-it-works, install, config, /board + NL + CLI usage, showcase) + Key Modules rows for monitor/ and dashboard/.

Full suite: 811 passed, 1 skipped.
Redesign built-in LeapBoard YAML templates into analyst-report layouts with sections, charts, timelines, capped evidence cards, entity maps, and structured session analysis.

Add browser-side language switching for English, Chinese, French, Spanish, Arabic, and Russian with persisted locale and RTL handling for Arabic.

Update README LeapBoard usage docs with start/open, manage, stop/mute/pause, and continuous observation/refresh mechanics.

Validation: node --check, py_compile, scoped ruff, full pytest: 811 passed, 1 skipped.
In-process REPL hardcoded per-command dispatch and never wired /board, so /board session fell through to the chat stream (LLM called skills_list) instead of opening the web board. Route any registered non-client-local command through the shared command_execute contract so recognized slash commands never leak to the LLM, and let web-view actions (open/home/session) open without a local monitor runtime. Extract a shared _open_dashboard_view helper for both REPLs.
A session-analysis (client-coupled) watch persisted in DuckDB from a prior run surfaced as '1 watch' in the status bar on a fresh leap start, before any /board. Client-coupled watches only make sense while a live interactive client owns them, so a fresh daemon lifetime must own none: drop persisted client-coupled watches (and their findings) in _start_monitors. Standalone watches stay durable.
A misspelled subcommand like 'leap deamon status' fell through the pre-parse 'unknown first token -> chat prompt' path, silently spawning a daemon and asking the LLM. Add a conservative did-you-mean guard: a short, command-like invocation whose first word is a near-miss of a known command now prints a suggestion and exits 2 instead of routing to chat. Genuine free-text prompts (longer sentences, non-command words) still chat.
/board session awaited an LLM-backed run_watch_once inside the command RPC, so the analysis (tens of seconds) blocked the response past the 30s client timeout; the bridge then reconnected and retried, producing the ~60s 'Timed out waiting for leapd response' failure. Decouple: arm the session watch and schedule the analysis as a tracked background task (MonitorManager.schedule_watch_once), returning the open payload with the watch id immediately. The board receives the finding over WebSocket when the cycle completes. Surface the watch id in the TUI, which also unblocks the browser-open flow (previously the timeout prevented the open payload from ever returning).
TUI exit reported 'leapd did not stop within the exit window' even though SIGKILL was sent. Root cause: a daemon spawned by the exiting process lingers as an unreaped zombie, and os.kill(pid, 0) still succeeds for zombies, so a successful kill looked like a failure; short timeouts made a slow graceful shutdown worse. Fix _process_alive to reap our exited children (WNOHANG) so a terminated daemon is correctly seen as gone, and distinguish ProcessLookupError/PermissionError. Per the user's directive, the confirmed exit stop now escalates to SIGKILL with generous windows and narrates every step (graceful wait -> SIGTERM -> SIGKILL) via a stop_daemon on_progress callback, with an honest final message and exact remediation if the OS still blocks it.
Add a Review Requirements rule: any change to slash-command paths (registry, router, in-process/daemon REPL, command_execute, completion, rendering), especially UX-facing behavior, must get a second human confirmation before it is considered ready.
/board new only returned an 'armed' text payload, so it never opened the web board. Add an open_web directive (decoupled from mode) so the TUI both renders the armed confirmation and launches the browser, focused on the freshly created watch. Also fix a latent bug where URL assembly dropped the target for non-session views (so /board open <id> / watch <id> landed on the overview instead of the watch detail): a single launcher.build_view_url now owns action+target query assembly, shared by the daemon-running and not-yet-running paths and both REPLs. Web-opening commands: open/home/session/new; terminal-feedback commands: list/status/findings/pause/resume/stop/mute/refresh. Confirmed by the user per the AGENTS.md slash-command human-confirmation rule.
One analysis target (the current session); a template is the sole view lens. /board opens the session board with the default generic template; /board <template> reframes the same analysis; unknown names fall back to generic with a note. Add /board templates [list|add|remove|show] with profile-scoped custom-template onboarding (validate -> copy into profiles/<profile>/dashboard/templates via new DashboardLayout; TemplateLibrary override_dir wired). Control verbs refresh/pause/resume/stop/status act on the single session observation. Route the dashboard discovery state through ProfileLayout.dashboard_state_path (no ad-hoc join); build_view_url carries ?template=. Remove watch-management commands (new/list/open/session/findings/mute), the overview/watch view builders, DashboardIntent action/domain/target, and the overview/session.analysis/*.market/.topic/.paper templates; generic now holds the rich session view and finance/sentiment/research are session lenses. Status bar unchanged. Confirmed by the user per the AGENTS.md slash-command human-confirmation rule.
Deep review of the session-only/template refactor found three stale paths and fixed them: (1) leap board CLI built old ?action=/&target= URLs and exposed action=home|session|open -> now takes a template positional and uses build_view_url(template=); (2) the web front-end nav still showed Overview/Session (both rendered generic) with the old action model -> replaced with a dynamic template switcher driven by spec.meta.templates/active_template (builder now exposes them), current={template}, always-refresh WebSocket, dropped dead openWatch nav; (3) the TUI text renderer _render_dashboard_view handled removed modes so /board templates printed nothing, /board status showed wrong counts, and /board refresh printed garbage -> rewritten for open/control/status/templates with readable output. Removed dead _render_findings_lines and unused DEFAULT_TEMPLATE. Added a render-coverage regression test and a switcher-meta contract test. Full suite 832 passed. Confirmed by the user per the AGENTS.md slash-command human-confirmation rule.
…id-targeted control, and clean exit

Command dispatch: a non-verb token that is not a known template lens is now rejected with a clear message instead of being silently coerced to generic; bare /board still opens the default lens, /board <lens> opens a known one.

Status: /board status now returns and renders every watch (id, state, run/finding counts, muted, last-run age) plus the recent findings (severity, title, summary, age), not just a template list.

Control: /board refresh|pause|resume|stop accept an optional watch id (exact or unique prefix as /board status shows). Without an id the verb targets the current controllable session watch; /board stop <id> now stops that specific watch instead of ignoring the id. Unknown/ambiguous ids and no-active-session are reported clearly.

Exit: leaving the TUI now stops every active board watch (not just client-coupled ones) and prints a one-line summary; removed the standalone-keepalive messaging and dead helpers.

Web/CLI leftovers: leap board takes a template positional (build_view_url) instead of the old action= URL; the web nav is a dynamic template switcher fed by spec.meta.templates/active_template; removed dead note handling and _render_findings_lines. Updated registry hints, README, and tests. Full suite 836 passed.
…h /config

Completion: the TUI /config completer now guides the llm and secret sub-actions (llm show|set with value-aware --model/--base-url/--api-key/... flags; secret list|set|get|delete), closing the gap where those subcommands had no suggestions.

Secrets: ConfigService.get now renders secret fields as a masked hint (***abc for long values, *** for short, missing when unset) instead of the opaque set/missing, applied at the single source feeding /config show|get|list in both TUI and CLI; the full value still requires secret get --reveal.

README: the Installation step now leads with the TUI /config llm set ... slash command (api key + model + base url) and collapses the leap config CLI under a More block. Adds completion and mask tests.
Locally-handled slash commands (e.g. /board, /config) ended with a boxed done card while agent-backed commands ended with the flat ' |--  LEAP  #N status  elapsed' response label. This unifies them: a new LeapConsole.command_footer renders that same flat line, and every terminal state (done/failed/cancelled/blocked/skipped/dropped) now routes to it instead of a boxed card. Queued/running keep the box as start markers, so only the ending changed. Extracted the shared _COMMAND_STATUS_STYLES so the footer and response label stay identical. Adds a command_footer render test.
server_running/ensure_server only checked that the port was open, never that the server there accepts the stored token. A stale or mismatched discovery-state token (server respawned with a new token, port reuse, a leftover server, or the old port-only readiness check falsely marking a stale server ready) yielded a URL the server rejected, so every /board launch (bare, /board <template>, and leap board [template]) could land on 'missing or invalid token'.

launcher now: probe_token verifies a server actually accepts a token; server_running requires port-open AND token-accepted; ensure_server retires the stale leftover (SIGTERM recorded pid), picks a free port when the preferred one is held by a foreign/stale server, and uses token-aware readiness before publishing state.

The client is now the single authority on the dashboard server: the daemon no longer builds/hands out a URL (_board_web_directive removed), which also drops a blocking urllib probe from the async loop; _open_dashboard_view always ensures+validates via ensure_server (run in a worker thread). Tests updated; full suite 840 passed.
Rewrite the shared SDUI design system (styles.css) to a light paper ground with near-black ink, a single accent, hairline rules, flat surfaces, a tight modular grid, tabular figures, and CSS-counter section numbering; switch fonts to Inter + Source Serif 4. Render the storyline as an academic abstract (bold lead-in), object lists as definition lists, and figure/table captions with Fig./Table numbering (app.js). Restructure generic.yaml for higher information density (KPI metric strip, compact captions, numbered sections). Fix bind_value so multi-placeholder strings interpolate instead of resolving to None, with a regression test.
Switch the shared grid to auto-fit and add config-driven cols/span so sections fill the row instead of leaving phantom empty columns; Section 01 now pairs a wide abstract (span 2) with the severity figure. Render component-level empty states for tables/lists (No entries.) instead of header-only skeletons, and show a single artifact count instead of 0/0. Add the missing i18n keys (titles, labels, captions, subtitles, page title) across all five locales so localized chrome no longer falls back to English. Make the KPI row a hairline-divided metric band, drop the redundant coverage bar, top-align grid cards, and unclamp insight summaries. Update the session-analysis test to assert the primary BarChart visualization now that the redundant ProgressBar is removed.
Extract series/OHLC offline from captured tool/file output and render real line/candlestick charts; Series is a titled section with a provenance caption and labeled lines.

De-weight process noise in analysis, drop the refresh-reason row, enlarge base type with smaller content text, and keep large screens single-column so sections stay ordered.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces LeapBoard, a Server-Driven UI (SDUI) monitoring web dashboard for LeapFlow, adding a domain-neutral monitoring subsystem, a session-analysis producer, and TUI/CLI integrations. The code review identified several critical security and reliability issues: a CORS bypass vulnerability in the origin check, insecure masking of short secrets, a potential risk of terminating unrelated processes due to PID reuse, sequential blocking when stopping active watches on exit, a bug in CLI typo suggestions discarding global flags, and a regex limitation when parsing floats with trailing dots.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +97 to +102
def _check_origin(request: Any) -> bool:
origin = request.headers.get("Origin")
if not origin:
return True # non-browser or same-origin requests carry no Origin
return "127.0.0.1" in origin or "localhost" in origin

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

The _check_origin method uses simple substring matching ("127.0.0.1" in origin or "localhost" in origin) to validate the Origin header. This can be easily bypassed by an attacker registering a domain like attacker127.0.0.1.com or localhost.attacker.com. Use proper URL parsing to validate the hostname exactly.

Suggested change
def _check_origin(request: Any) -> bool:
origin = request.headers.get("Origin")
if not origin:
return True # non-browser or same-origin requests carry no Origin
return "127.0.0.1" in origin or "localhost" in origin
@staticmethod
def _check_origin(request: Any) -> bool:
from urllib.parse import urlparse
origin = request.headers.get("Origin")
if not origin:
return True # non-browser or same-origin requests carry no Origin
try:
parsed = urlparse(origin)
return parsed.hostname in ("127.0.0.1", "localhost")
except Exception:
return False

Comment thread src/leapflow/cli/cli.py
if len(non_flag_tokens) <= 3:
suggestion = _suggest_known_command(first_token, known_commands)
if suggestion is not None:
corrected = " ".join(["leap", suggestion, *effective_argv[first_pos + 1:]])

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The typo correction suggestion discards any global flags/options that appeared before first_pos (e.g., leap --profile dev deamon status becomes leap daemon status, losing --profile dev). Preserve the original arguments by replacing only the typo token.

                corrected_args = list(effective_argv)
                corrected_args[first_pos] = suggestion
                corrected = " ".join(["leap", *corrected_args])

Comment on lines +118 to +127
stopped = 0
for watch in active:
watch_id = str(watch.get("watch_id", ""))
if not watch_id:
continue
try:
await asyncio.wait_for(client.watch_stop(watch_id), timeout=1.0)
stopped += 1
except Exception:
logger.debug("interactive: failed to stop board watch %s", watch_id, exc_info=True)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

In _stop_board_watches_on_exit, active watches are stopped sequentially in a loop. If there are multiple active watches, this can block the TUI exit process sequentially for up to 1.0 second per watch if the daemon is slow or unresponsive. Running these concurrently using asyncio.gather prevents sequential blocking.

    tasks = []
    for watch in active:
        watch_id = str(watch.get("watch_id", ""))
        if watch_id:
            tasks.append(asyncio.wait_for(client.watch_stop(watch_id), timeout=1.0))
    if tasks:
        results = await asyncio.gather(*tasks, return_exceptions=True)
        stopped = sum(1 for r in results if not isinstance(r, Exception))

Comment on lines +639 to +641
# Reveal last 3 chars only when the secret is long enough to remain safe.
suffix = text[-3:] if len(text) >= 8 else ""
return f"***{suffix}" if suffix else "***"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-medium medium

In _mask_secret, revealing the last 3 characters of a secret when its length is >= 8 is a security risk. For short secrets (e.g., exactly 8 characters), revealing 3 characters exposes 37.5% of the secret, significantly reducing the search space for brute-forcing. The threshold should be increased to at least 16 characters to ensure sufficient entropy remains masked.

Suggested change
# Reveal last 3 chars only when the secret is long enough to remain safe.
suffix = text[-3:] if len(text) >= 8 else ""
return f"***{suffix}" if suffix else "***"
# Reveal last 3 chars only when the secret is long enough to remain safe.
suffix = text[-3:] if len(text) >= 16 else ""
return f"***{suffix}" if suffix else "***"

Comment on lines +174 to +196
def _retire_stale_server(settings: Any) -> None:
"""Best-effort retire a stale dashboard server recorded in discovery state.

Called only after ``server_running`` rejected the state (dead server or a
token the server no longer accepts). Signals the recorded pid so the port
frees for a fresh, trusted server, then drops the stale state file.
"""
state = load_state(settings)
if not state:
return
port = int(state.get("port") or 0)
bind = str(state.get("bind") or getattr(settings, "dashboard_bind", "127.0.0.1"))
pid = int(state.get("pid") or 0)
if port and pid > 0 and is_port_open(bind, port):
try:
os.kill(pid, signal.SIGTERM)
except OSError:
logger.debug("dashboard: stale server pid=%s not signalable", pid, exc_info=True)
else:
deadline = time.monotonic() + 2.0
while time.monotonic() < deadline and is_port_open(bind, port):
time.sleep(0.1)
clear_state(settings)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

In _retire_stale_server, the launcher attempts to terminate a stale process by calling os.kill(pid, signal.SIGTERM) based solely on the PID stored in the state file. If the original dashboard process has died and the OS has reused the PID for an unrelated process, this will terminate an innocent process. Consider verifying that the process is indeed the dashboard server (e.g., by checking its command line or process name) before sending the signal, or avoid terminating it if the token probe fails.

Comment on lines +279 to +283
if re.fullmatch(r"[-+]?\d*\.?\d+([eE][-+]?\d+)?", cleaned or ""):
try:
return float(cleaned)
except ValueError:
return None

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The regex r"[-+]?\d*\.?\d+([eE][-+]?\d+)?" used in _num to validate float strings does not match valid float representations with a trailing dot but no decimal digits (e.g., 1.), which are common in some data outputs. Using a more robust regex pattern will improve parsing correctness.

Suggested change
if re.fullmatch(r"[-+]?\d*\.?\d+([eE][-+]?\d+)?", cleaned or ""):
try:
return float(cleaned)
except ValueError:
return None
if re.fullmatch(r"[-+]?(\d+(\.\d*)?|\.\d+)([eE][-+]?\d+)?", cleaned or ""):
try:
return float(cleaned)
except ValueError:
return None

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant