feat: add live Paperclip workspace tab#3
Open
contentscoin wants to merge 1 commit into
Open
Conversation
- Embed the local Paperclip web app as a main workspace tab with status probing and refresh/open controls - Add Paperclip status endpoint plus approval-safe console synchronization - Include memory candidate, multi-agent, setup-pack, and desktop install pack updates - Rebuild desktop-install-pack.zip with PAPERCLIP_WEB_URL guidance
📝 WalkthroughWalkthroughThe PR introduces a multi-layered enhancement to Hermes WebUI: profile-aware sessions with per-workspace configuration, a new local memory candidate/fact store with approval gates, desktop installers for macOS and Windows, a Paperclip console with decision intelligence and approval workflow management, a multi-agent workspace for parallel lane execution, and comprehensive documentation packs (AutoResearch, ShareNote publishing, Memory Bank adoption). Default model changes to GPT-5.5. ChangesProfile and Multi-Workspace Support
Memory Bank: Candidates and Facts Store
Desktop Installation Infrastructure
Paperclip Console and Decision Intelligence
Multi-Agent Workspace
Documentation and Implementation Plans
Sequence Diagram(s)sequenceDiagram
participant CEO as CEO User
participant Inbox as Memory Inbox Panel
participant API as /api/memory-candidates
participant Store as facts.jsonl Storage
participant Session as Session State
CEO->>Inbox: Manual JSON entry (category, scope, statement)
Inbox->>API: POST create_candidate(...)
API->>Store: Append pending candidate
Store-->>API: candidate_id
API-->>Inbox: {candidate, ok: true}
Inbox->>Inbox: Render card with edit/approve/reject
CEO->>Inbox: Edit statement + Approve
Inbox->>API: POST approve_candidate(id, edited_statement)
API->>Store: Mark candidate approved, create fact
Store-->>API: {candidate, fact}
API-->>Inbox: {ok: true, summary}
Inbox->>Inbox: Refresh, show active fact
sequenceDiagram
participant CEO as CEO
participant UI as Paperclip Console
participant Routes as /api/paperclip/status
participant Boot as boot.js Workflow
participant Artifact as /api/file/create
CEO->>UI: Click Workflow Action (e.g., Decision Report)
UI->>Boot: detectPaperclipApprovalScope()
Boot-->>UI: {scope, hasApproval}
alt Approval Present
UI->>Boot: createPaperclipResultArtifact(kind)
Boot->>Artifact: POST create artifact
Artifact-->>Boot: artifact_path
Boot->>Boot: recordPaperclipWorkflowRun(scope, path)
Boot-->>UI: artifact_path
UI->>Routes: Fetch /api/paperclip/status
Routes-->>UI: {health, url}
UI->>UI: Show result in iframe/console
else No Approval
UI-->>CEO: Block action, show approval hint
end
sequenceDiagram
participant CEO as CEO User
participant Router as Multi-Agent Router
participant Hela as Hela Lane
participant Plan as Plan Lane
participant Dev as Dev Lane
CEO->>Router: Submit prompt "plan the architecture"
Router->>Router: resolveTargetLanes(text)
Router-->>Router: Match keywords → [plan, dev]
Router->>Plan: createLaneSession(), sendLaneMessage()
Router->>Dev: createLaneSession(), sendLaneMessage()
Plan->>Plan: consumeLaneStream() → SSE /api/chat/stream
Dev->>Dev: consumeLaneStream() → SSE /api/chat/stream
Plan-->>Router: token events, done
Dev-->>Router: token events, done
Router->>Router: renderMultiAgentSummary()
Router-->>CEO: Show latest Plan/Dev outputs side-by-side
CEO->>Hela: Handoff: "synthesize insights"
Hela->>Plan: Read latest Plan summary
Hela->>Hela: buildHandoffPrompt(plan_summary)
Hela->>Hela: consumeLaneStream()
Hela-->>CEO: Synthesis output
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes This PR is substantial and heterogeneous, spanning backend infrastructure (profile support, facts store, API routes), desktop packaging (platform-specific installers), frontend UI (three major new panels: memory inbox, Paperclip console, multi-agent workspace), styling overhauls, documentation packs, and comprehensive testing. The logic density is high in multi-agent lane routing, Paperclip approval gating, and JSONL storage semantics. Multiple independent feature streams require separate reasoning. While many changes follow consistent patterns (e.g., repeated UI panel/style blocks), the diversity of technologies (Bash/PowerShell/Electron/Python/JavaScript/CSS/Markdown) and interconnected feature dependencies demand thorough cross-layer validation. Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (10)
desktop-install-pack/templates/env.example-6-6 (1)
6-6:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
PAPERCLIP_DEFAULT_COMPANY=FMGhardcodes what appears to be a real company name.In a public repository, shipping an example env file with a real company identifier as a default value is inadvertent information disclosure and will also confuse unrelated users of this template who copy it verbatim. Replace with a generic placeholder.
🔒 Proposed fix
-PAPERCLIP_DEFAULT_COMPANY=FMG +PAPERCLIP_DEFAULT_COMPANY=YOUR_COMPANY🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@desktop-install-pack/templates/env.example` at line 6, The env example currently hardcodes a real company via the PAPERCLIP_DEFAULT_COMPANY=FMG entry; replace that value with a generic placeholder (e.g. PAPERCLIP_DEFAULT_COMPANY=your_company_name or PAPERCLIP_DEFAULT_COMPANY=REPLACE_WITH_COMPANY) so the template no longer discloses a real company or causes confusion—update the PAPERCLIP_DEFAULT_COMPANY line in the env.example to the placeholder and, if present, adjust any documentation referencing that default to note it must be set by the user.sharenote-telegram-pack/templates/telegram-message-template.md-1-9 (1)
1-9:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTemplate is disconnected from
create_sharenote_telegram_draft.pyand is structurally incomplete.
create_sharenote_telegram_draft.pyconstructs the draft message inline using an f-string (lines 84–98) and never reads or renders this template file. As a result:
- Dead template — edits here have no effect on the script's output.
- Missing fields — the template omits the
대상:and전송 상태: 미승인 / 미전송lines that the script always writes.- Name mismatch — the template uses
{{note_path}}while the script variable isnote.Either wire the script to load-and-substitute this template (using
string.Templateor similar), or update the template to fully mirror the script's actual f-string output so it at least serves as accurate reference documentation.📄 Template corrected to match script output (reference-doc approach)
{{title}} {{summary}} 공유 링크: {{share_url}} 저장 위치: -{{note_path}} +{{note}} + +대상: +{{target}} + +전송 상태: 미승인 / 미전송🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sharenote-telegram-pack/templates/telegram-message-template.md` around lines 1 - 9, The template file telegram-message-template.md is not used by create_sharenote_telegram_draft.py (the script builds the message inline with an f-string) and its contents are incomplete/mismatched; either load and render this template from create_sharenote_telegram_draft.py (e.g., read templates/telegram-message-template.md and substitute variables using string.Template or Jinja2) and replace the current inline f-string in the function that builds the draft (around lines 84–98), or update telegram-message-template.md to exactly match the script’s f-string output (add the missing "대상:" and "전송 상태: 미승인 / 미전송" lines and rename {{note_path}} to {{note}}) so it accurately reflects the message format used by the function that constructs the share note draft.sharenote-telegram-pack/scripts/check_sharenote_env.py-8-17 (1)
8-17:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
json.JSONDecodeErrornot caught indiscover_vaults().
errors="ignore"suppresses UTF-8 decoding errors only; it does not protect against malformed JSON. Ifobsidian.jsoncontains invalid JSON (e.g., partially written during a crash), the script aborts with an unhandled traceback rather than gracefully returning an empty list.🐛 Proposed fix
def discover_vaults(): if not APP_CONFIG.exists(): return [] - data = json.loads(APP_CONFIG.read_text(errors="ignore")) + try: + data = json.loads(APP_CONFIG.read_text(errors="ignore")) + except (json.JSONDecodeError, OSError): + return [] out = []🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sharenote-telegram-pack/scripts/check_sharenote_env.py` around lines 8 - 17, The discover_vaults() function calls json.loads(APP_CONFIG.read_text(...)) without handling malformed JSON; wrap the json.loads call in a try/except that catches json.JSONDecodeError (and optionally ValueError) and returns [] on error so the function fails gracefully instead of raising; keep the existing read_text(errors="ignore") behavior and then proceed to iterate data.get("vaults") only when data is a dict to avoid attribute errors.desktop-install-pack/install-macos.sh-83-85 (1)
83-85:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFixed
sleep 2health-check is fragile — fails on slow or loaded machines.A 2-second sleep before hitting
/healthwill cause the installer to abort (exit 1) on any machine where the WebUI process takes longer to bind its port. There is no retry, no cleanup, and the failure message only prints the log path without indicating this is likely a timing issue.💡 Proposed fix — poll with retries
-"$HOME/.hermes/bin/hermes-ceo-console" >/tmp/hermes-ceo-console.log 2>&1 & -sleep 2 -curl -fsS "http://127.0.0.1:$PORT/health" || { echo "Health check failed; see /tmp/hermes-ceo-console.log"; exit 1; } +"$HOME/.hermes/bin/hermes-ceo-console" >/tmp/hermes-ceo-console.log 2>&1 & +echo "Waiting for WebUI to start..." +for _i in $(seq 1 15); do + if curl -fsS "http://127.0.0.1:$PORT/health" >/dev/null 2>&1; then + break + fi + if [ "$_i" -eq 15 ]; then + echo "Health check failed after 15s; see /tmp/hermes-ceo-console.log" + exit 1 + fi + sleep 1 +done🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@desktop-install-pack/install-macos.sh` around lines 83 - 85, Replace the brittle fixed sleep + single curl with a retrying poll loop that checks http://127.0.0.1:$PORT/health until success or a configurable timeout; specifically, after launching "$HOME/.hermes/bin/hermes-ceo-console" (logging to /tmp/hermes-ceo-console.log) implement a loop that attempts curl -fsS to /health every 0.5–1s up to N seconds, exiting 0 on success and on final failure printing a clear error that includes the log path and a hint that it likely timed out, then perform any needed cleanup/kill of the background process; reference the launched command string, the $PORT variable, /tmp/hermes-ceo-console.log, and the /health endpoint when making the changes.sharenote-telegram-pack/scripts/create_sharenote_telegram_draft.py-11-21 (1)
11-21:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
json.JSONDecodeErrorindiscover_vault()is unhandled.
errors="ignore"at line 16 only suppresses UTF-8 decoding problems; malformed JSON (e.g., partially-writtenobsidian.json) will propagate ajson.JSONDecodeErrorthat bypasses theSystemExitat line 21 and surfaces as a confusing raw traceback.🐛 Proposed fix
if APP_CONFIG.exists(): - data = json.loads(APP_CONFIG.read_text(errors="ignore")) + try: + data = json.loads(APP_CONFIG.read_text(errors="ignore")) + except (json.JSONDecodeError, OSError): + raise SystemExit("Could not parse Obsidian config. Set OBSIDIAN_VAULT_PATH instead.")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sharenote-telegram-pack/scripts/create_sharenote_telegram_draft.py` around lines 11 - 21, discover_vault() currently calls json.loads(APP_CONFIG.read_text(...)) without handling malformed JSON so a json.JSONDecodeError can leak; wrap the json.loads(...) (or the whole APP_CONFIG.read_text + json.loads sequence) in a try/except that catches json.JSONDecodeError (and optionally ValueError) and treats it the same as "no vault found" — e.g., warn or ignore the broken APP_CONFIG and proceed to raise SystemExit("No Obsidian vault found. Set OBSIDIAN_VAULT_PATH first.") if no valid vault path is discovered; reference the discover_vault function and APP_CONFIG when making this change.sharenote-telegram-pack/scripts/create_sharenote_telegram_draft.py-55-77 (1)
55-77:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGenerated frontmatter diverges from
publishing-note-template.mdon two fields.
- The script writes
created:(line 58) but the template declarescreated_at:. Any downstream parser or operator workflow that keys oncreated_atwill not find the field.telegram_targetappears only in the Markdown body (line 75) but is absent from the YAML frontmatter, while the template defines it there — automation that reads frontmatter for routing will miss the target.💡 Proposed fix — align with template field names
note_text = f"""--- title: "{args.title}" -created: "{created}" +created_at: "{created}" status: draft share: "" +telegram_target: "{args.target}" tags: - publishing - sharenote ---🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sharenote-telegram-pack/scripts/create_sharenote_telegram_draft.py` around lines 55 - 77, The YAML frontmatter written into note_text uses "created" and lacks a frontmatter "telegram_target", diverging from publishing-note-template.md; update the multi-line string assigned to note_text (inside the block guarded by if not note.exists()) to use created_at: "{created}" instead of created: and add a telegram_target: "{args.target}" entry in the frontmatter so downstream parsers read the same fields as the template.desktop-install-pack/install-macos.sh-4-18 (1)
4-18:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
YESflag is defined but never read —--yeshas no effect.ShellCheck (SC2034) correctly flags this. The script has no interactive prompts and never branches on
$YES, so passing--yes/-ydoes nothing. Either add a confirmation prompt that--yesbypasses, or remove the flag to avoid misleading callers.💡 Minimal fix — remove the dead flag
-YES=0 PORT=8788 REPO_URL="https://github.com/reallygood83/hermes-for-web.git" INSTALL_DIR="$HOME/.hermes/webui/workspace/hermes-for-web" APP_NAME="Hermes CEO Console" while [[ $# -gt 0 ]]; do case "$1" in - --yes|-y) YES=1; shift ;; --port) PORT="${2:-8788}"; shift 2 ;;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@desktop-install-pack/install-macos.sh` around lines 4 - 18, The YES variable and the --yes|-y case branch are dead and misleading; remove the YES=0 declaration and the "--yes|-y) YES=1; shift ;;" case entry (and any references if added elsewhere) so the script no longer accepts a no-op flag, or alternatively implement a confirmation prompt using YES to skip it—prefer the minimal fix of deleting the YES variable and that case branch in the argument-parsing block (referencing YES and the "--yes|-y" case) to keep behavior consistent.api/streaming.py-97-101 (1)
97-101:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
get_profile_home()is called without validating the profile directory exists.Unlike
switch_profile(), which validates profile existence before callingget_profile_home(), streaming uses the session's stored profile name directly. If that profile was deleted after the session was created,HERMES_HOMEis set to a non-existent path, silently breaking any agent tool that accesses state.db or config files under that directory for the duration of the stream. The previousget_active_hermes_home()always returned a valid directory.🛡️ Proposed fix — fall back to default profile home if directory is missing
try: from api.profiles import get_profile_home - _profile_home = str(get_profile_home(getattr(s, 'profile', None))) + _ph = get_profile_home(getattr(s, 'profile', None)) + if not _ph.exists(): + _ph = get_profile_home('default') + _profile_home = str(_ph) except ImportError: _profile_home = os.environ.get('HERMES_HOME', '')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/streaming.py` around lines 97 - 101, The code calls get_profile_home(getattr(s, 'profile', None)) and assigns it to _profile_home without verifying the directory exists, which can point HERMES_HOME to a deleted profile; modify the block after importing get_profile_home to check that the returned path is a valid directory (os.path.isdir), and if not, fall back to a known-good home (e.g., call get_active_hermes_home() if available or use os.environ.get('HERMES_HOME','') / default) so _profile_home always references an existing directory; keep the ImportError fallback but ensure the existence check and fallback occur whether get_profile_home returns None/invalid or raises.desktop-install-pack/electron-wrapper/package.json-19-19 (1)
19-19:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
preload.jslisted infilesbut the file does not exist
electron-builderwill attempt to locatepreload.jsduring packaging. If the file is absent, the build may silently omit it or fail with a "does not exist" error, blocking the.dmg/.nsisbuild step. Either create a minimalpreload.jsstub (even an empty one) or remove it from thefilesarray until a preload script is actually needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@desktop-install-pack/electron-wrapper/package.json` at line 19, package.json's "files" array references preload.js but that file is missing; either add a minimal preload.js stub in the electron-wrapper package (e.g., an empty file or a simple DOM-safe export) so electron-builder can include it, or remove "preload.js" from the "files" array in package.json to stop packaging from expecting it—update the package.json "files" entry accordingly and ensure the preload.js file (if added) is placed alongside main.js and preload.js in the package root so electron-builder can find it.tests/test_fact_routes.py-44-49 (1)
44-49:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing
FACTS_DIRisolation may leave side effects in the real facts storeUnlike the first test,
test_memory_candidate_create_requires_statementdoes not monkeypatchroutes.facts_store.FACTS_DIR. If_handle_memory_candidate_createtouches the filesystem before it validates the missingstatementfield, this test will create artifacts under the realFACTS_DIR. Addtmp_pathisolation for safety:🛡️ Proposed fix
-def test_memory_candidate_create_requires_statement(monkeypatch): +def test_memory_candidate_create_requires_statement(tmp_path, monkeypatch): + monkeypatch.setattr(routes.facts_store, 'FACTS_DIR', tmp_path) monkeypatch.setattr(routes, 'j', lambda handler, payload, status=200: (payload, status))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_fact_routes.py` around lines 44 - 49, The test test_memory_candidate_create_requires_statement should isolate the facts storage by monkeypatching routes.facts_store.FACTS_DIR to a temporary directory so the call to routes._handle_memory_candidate_create(SimpleNamespace(), {'scope': 'global'}) cannot create real artifacts; update the test to accept a tmp_path fixture and set monkeypatch.setattr(routes.facts_store, 'FACTS_DIR', str(tmp_path)) (or equivalent) before invoking _handle_memory_candidate_create, keeping the existing monkeypatches for routes.j and routes.bad.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/facts.py`:
- Around line 67-70: _write_jsonl currently writes directly to the target file
which can leave it truncated if the process crashes; change it to write
atomically by creating a temp file in the same directory (e.g., using
tempfile.NamedTemporaryFile or a .tmp Path in path.parent), write the JSONL
contents to that temp file, flush and fsync the file, then atomically replace
the target with os.replace(temp_path, path) (and optionally fsync the parent
directory) so readers (_read_jsonl) never see a partially-written file; keep
path.parent.mkdir as is so the temp file can be created.
In `@api/routes.py`:
- Around line 1420-1470: The three handlers _handle_memory_candidate_create,
_handle_memory_candidate_approve, and _handle_memory_candidate_reject are
implemented but never dispatched from handle_post(), so POST requests for
create/approve/reject return 404; update the request routing in handle_post() to
recognize the corresponding POST paths or action names used by static/facts.js
and call these helpers (e.g., call _handle_memory_candidate_create(handler,
body) for the create route, _handle_memory_candidate_approve(handler, body) for
approve, and _handle_memory_candidate_reject(handler, body) for reject), ensure
you extract the request body the same way handle_post() does and return their
responses directly so the inbox is writable.
In `@desktop-install-pack/electron-wrapper/main.js`:
- Around line 9-13: In startServer, the Windows spawn currently passes a
'--port' arg which server.py ignores; update the Windows branch so args do not
include '--port' (use ['server.py'] or empty as appropriate) and pass the port
via the spawn env (set HERMES_WEBUI_PORT = port while preserving process.env) so
server.py reads the correct port; adjust the spawn call that assigns to server
(spawn(cmd, args, {cwd: repo, shell: isWin, env: {...process.env,
HERMES_WEBUI_PORT: String(port)}, stdio: 'inherit' /* or 'ignore' for production
*/})) to ensure the server binds to the intended port.
- Line 22: Replace the fixed 1800ms delay after startServer() with a
health-check polling loop that waits for the server to be ready before calling
createWindow(): implement a helper (e.g., pollServerHealth or
waitForServerReady) that repeatedly requests the server's /health endpoint with
a short interval and timeout/max-retries, call startServer() then await
pollServerHealth() inside the app.whenReady() promise, and only call
createWindow() after the health check succeeds (on failure after max retries log
an error and show a retry/fatal UI instead of opening the window). Ensure the
code uses the existing startServer() and createWindow() symbols and removes the
setTimeout-based call.
- Line 23: The current app.on('window-all-closed') handler unconditionally calls
server.kill(), which kills the backend on macOS when windows close; change the
logic so server.kill() is only invoked when the app is actually quitting (i.e.,
keep the server running when process.platform === 'darwin' and only kill it in
an app.on('before-quit' or quit) handler), and add an app.on('activate') handler
that recreates the window (call the existing createWindow or equivalent) and
restarts/creates the server if it is not running so clicking the dock icon
restores the UI and backend; ensure you reference and update the handlers around
app.on('window-all-closed'), app.on('activate'), and the server lifecycle calls
(server.kill(), server start/creation function) accordingly.
In `@desktop-install-pack/install-windows.ps1`:
- Around line 12-13: The installer currently fails when only the Windows Python
launcher (py.exe) exists because the launcher generation code uses a hardcoded
python executable; update the prerequisite check and launcher creation to accept
the Windows Python launcher by reusing the discovered $PythonExe variable (which
may be "py.exe" or "python.exe") wherever the script builds the launcher (the
code that generates the launcher/entry point), so the script uses $PythonExe
instead of assuming "python.exe" and thus works for both launcher and full
Python installs.
In `@sharenote-telegram-pack/README.md`:
- Around line 8-22: Replace all personal and machine-specific data in the
README: remove or redact the absolute path "/Users/jakeshin/antigravity/..." and
the Obsidian app path if it reveals a username, replacing them with generic
placeholders like "<USER_HOME>/obsidian-vault" and "/Applications/Obsidian.app"
(or a note "path to Obsidian app"); remove or genericize all Telegram target
names such as "telegram:홈원", "telegram:Pax Team Group", and "telegram:조계종" and
instead show the target format (e.g. "telegram:<target-name> or
telegram:<group-name>") or a short example like "telegram:your-target"; ensure
any other occurrences of "jakeshin" or other personal identifiers are replaced
with "<USERNAME>" and that credentials/redacted entries remain non-identifying.
In `@static/boot.js`:
- Around line 616-618: The current logic returns early when no issue key is
found (const match = text.match(...); if(!match) return null;), which leaves
runs stuck; instead ensure finalizePaperclipWorkflowRun() is always invoked for
comment-only or no-key cases before returning. Modify the flow around the
match/identifier extraction so that when match is falsy you call
finalizePaperclipWorkflowRun() (or delegate to the same finish path used when an
identifier exists) and then return; keep using the existing symbols (match,
identifier) but guard access to identifier only when match is truthy.
- Around line 355-373: The function detectPaperclipApprovalScope treats
patterns.generic matches as full approval, which is too permissive; update the
logic in detectPaperclipApprovalScope so that patterns.generic does NOT set
scope='full' — instead either set a lower-privilege scope (e.g.,
scope='comment') or leave scope unchanged (keep 'none') unless an explicit full
pattern in patterns.full matched; specifically change the last branch that
checks patterns.generic (currently "else
if(patterns.generic.some(rx=>rx.test(text)) && scope==='none') scope='full'") to
assign a non-full scope (or no assignment), ensuring only patterns.full can set
scope='full' and preserve the existing precedence rules in that function.
- Around line 541-550: The handler currently calls createPaperclipResultArtifact
and recordPaperclipWorkflowRun even when S.busy is true, causing orphan
artifacts; before awaiting createPaperclipResultArtifact or recording history,
check S.busy and return/abort if true (or wrap the artifact creation + record
steps inside a conditional that only runs when !S.busy). Specifically, guard
around the artifactPath creation and the call to recordPaperclipWorkflowRun
(references: S.busy, createPaperclipResultArtifact, recordPaperclipWorkflowRun,
buildWorkflowPrompt, artifactPath) so no result file or console history is
created while a workflow is already in-flight.
In `@static/multiagent.js`:
- Around line 180-191: routePromptToLanes currently awaits sendLaneText inside a
for loop which causes requests to each lane to run serially; instead collect
sendLaneText promises for each valid lane (use getLane and resolveTargetLanes to
filter/identify lanes) and then await them concurrently with Promise.all so all
targeted lanes are dispatched in parallel; keep clearing the input value before
awaiting and preserve existing continue/skip behavior for missing lanes.
- Around line 340-345: The 'error' event handler on src currently just clears
lane.busy, lane.streamId and re-renders, leaving an empty assistant bubble;
update the handler in the src.addEventListener('error', ...) block to capture
the ErrorEvent (or its message), set a visible error field on the lane (e.g.
lane.apperror or lane.errorMessage) with a user-friendly message plus the native
error detail, clear lane.streamId and lane.busy, call renderMultiAgentShell(),
and then resolve(); ensure you reference the existing symbols
(src.addEventListener('error', ...), lane.streamId, lane.busy,
renderMultiAgentShell(), resolve()) so the UI shows a failure message instead of
a blank assistant bubble.
---
Minor comments:
In `@api/streaming.py`:
- Around line 97-101: The code calls get_profile_home(getattr(s, 'profile',
None)) and assigns it to _profile_home without verifying the directory exists,
which can point HERMES_HOME to a deleted profile; modify the block after
importing get_profile_home to check that the returned path is a valid directory
(os.path.isdir), and if not, fall back to a known-good home (e.g., call
get_active_hermes_home() if available or use os.environ.get('HERMES_HOME','') /
default) so _profile_home always references an existing directory; keep the
ImportError fallback but ensure the existence check and fallback occur whether
get_profile_home returns None/invalid or raises.
In `@desktop-install-pack/electron-wrapper/package.json`:
- Line 19: package.json's "files" array references preload.js but that file is
missing; either add a minimal preload.js stub in the electron-wrapper package
(e.g., an empty file or a simple DOM-safe export) so electron-builder can
include it, or remove "preload.js" from the "files" array in package.json to
stop packaging from expecting it—update the package.json "files" entry
accordingly and ensure the preload.js file (if added) is placed alongside
main.js and preload.js in the package root so electron-builder can find it.
In `@desktop-install-pack/install-macos.sh`:
- Around line 83-85: Replace the brittle fixed sleep + single curl with a
retrying poll loop that checks http://127.0.0.1:$PORT/health until success or a
configurable timeout; specifically, after launching
"$HOME/.hermes/bin/hermes-ceo-console" (logging to /tmp/hermes-ceo-console.log)
implement a loop that attempts curl -fsS to /health every 0.5–1s up to N
seconds, exiting 0 on success and on final failure printing a clear error that
includes the log path and a hint that it likely timed out, then perform any
needed cleanup/kill of the background process; reference the launched command
string, the $PORT variable, /tmp/hermes-ceo-console.log, and the /health
endpoint when making the changes.
- Around line 4-18: The YES variable and the --yes|-y case branch are dead and
misleading; remove the YES=0 declaration and the "--yes|-y) YES=1; shift ;;"
case entry (and any references if added elsewhere) so the script no longer
accepts a no-op flag, or alternatively implement a confirmation prompt using YES
to skip it—prefer the minimal fix of deleting the YES variable and that case
branch in the argument-parsing block (referencing YES and the "--yes|-y" case)
to keep behavior consistent.
In `@desktop-install-pack/templates/env.example`:
- Line 6: The env example currently hardcodes a real company via the
PAPERCLIP_DEFAULT_COMPANY=FMG entry; replace that value with a generic
placeholder (e.g. PAPERCLIP_DEFAULT_COMPANY=your_company_name or
PAPERCLIP_DEFAULT_COMPANY=REPLACE_WITH_COMPANY) so the template no longer
discloses a real company or causes confusion—update the
PAPERCLIP_DEFAULT_COMPANY line in the env.example to the placeholder and, if
present, adjust any documentation referencing that default to note it must be
set by the user.
In `@sharenote-telegram-pack/scripts/check_sharenote_env.py`:
- Around line 8-17: The discover_vaults() function calls
json.loads(APP_CONFIG.read_text(...)) without handling malformed JSON; wrap the
json.loads call in a try/except that catches json.JSONDecodeError (and
optionally ValueError) and returns [] on error so the function fails gracefully
instead of raising; keep the existing read_text(errors="ignore") behavior and
then proceed to iterate data.get("vaults") only when data is a dict to avoid
attribute errors.
In `@sharenote-telegram-pack/scripts/create_sharenote_telegram_draft.py`:
- Around line 11-21: discover_vault() currently calls
json.loads(APP_CONFIG.read_text(...)) without handling malformed JSON so a
json.JSONDecodeError can leak; wrap the json.loads(...) (or the whole
APP_CONFIG.read_text + json.loads sequence) in a try/except that catches
json.JSONDecodeError (and optionally ValueError) and treats it the same as "no
vault found" — e.g., warn or ignore the broken APP_CONFIG and proceed to raise
SystemExit("No Obsidian vault found. Set OBSIDIAN_VAULT_PATH first.") if no
valid vault path is discovered; reference the discover_vault function and
APP_CONFIG when making this change.
- Around line 55-77: The YAML frontmatter written into note_text uses "created"
and lacks a frontmatter "telegram_target", diverging from
publishing-note-template.md; update the multi-line string assigned to note_text
(inside the block guarded by if not note.exists()) to use created_at:
"{created}" instead of created: and add a telegram_target: "{args.target}" entry
in the frontmatter so downstream parsers read the same fields as the template.
In `@sharenote-telegram-pack/templates/telegram-message-template.md`:
- Around line 1-9: The template file telegram-message-template.md is not used by
create_sharenote_telegram_draft.py (the script builds the message inline with an
f-string) and its contents are incomplete/mismatched; either load and render
this template from create_sharenote_telegram_draft.py (e.g., read
templates/telegram-message-template.md and substitute variables using
string.Template or Jinja2) and replace the current inline f-string in the
function that builds the draft (around lines 84–98), or update
telegram-message-template.md to exactly match the script’s f-string output (add
the missing "대상:" and "전송 상태: 미승인 / 미전송" lines and rename {{note_path}} to
{{note}}) so it accurately reflects the message format used by the function that
constructs the share note draft.
In `@tests/test_fact_routes.py`:
- Around line 44-49: The test test_memory_candidate_create_requires_statement
should isolate the facts storage by monkeypatching routes.facts_store.FACTS_DIR
to a temporary directory so the call to
routes._handle_memory_candidate_create(SimpleNamespace(), {'scope': 'global'})
cannot create real artifacts; update the test to accept a tmp_path fixture and
set monkeypatch.setattr(routes.facts_store, 'FACTS_DIR', str(tmp_path)) (or
equivalent) before invoking _handle_memory_candidate_create, keeping the
existing monkeypatches for routes.j and routes.bad.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e363bd6e-4647-4a6a-b42b-8587a375a2a2
⛔ Files ignored due to path filters (1)
desktop-install-pack.zipis excluded by!**/*.zip
📒 Files selected for processing (48)
CHANGELOG.mdapi/config.pyapi/facts.pyapi/models.pyapi/profiles.pyapi/routes.pyapi/streaming.pyautoresearch-pack/README.mdautoresearch-pack/checklists/deepening-checklist.mdautoresearch-pack/templates/research-output-template.mdautoresearch-pack/templates/research-question-template.mdautoresearch-pack/workflows/default-research-loop.mddesktop-install-pack/README.mddesktop-install-pack/electron-wrapper/main.jsdesktop-install-pack/electron-wrapper/package.jsondesktop-install-pack/electron-wrapper/preload.jsdesktop-install-pack/install-macos.shdesktop-install-pack/install-windows.ps1desktop-install-pack/templates/env.exampledocs/plans/2026-05-01-cross-platform-desktop-installer-plan.mddocs/plans/2026-05-01-memory-bank-paperclip-enhancement-plan.mddocs/research-packs.mddocs/setup-packs.mdmemory-bank-adoption-pack/README.mdmemory-bank-adoption-pack/design/fact-lifecycle.mdmemory-bank-adoption-pack/design/paperclip-decision-intelligence.mdmemory-bank-adoption-pack/design/scoped-facts.mdmemory-bank-adoption-pack/templates/memory-candidate-review.mdmemory-bank-adoption-pack/templates/paperclip-decision-intelligence-report.mdsharenote-telegram-pack/README.mdsharenote-telegram-pack/checklists/prepublish-checklist.mdsharenote-telegram-pack/scripts/check_sharenote_env.pysharenote-telegram-pack/scripts/create_sharenote_telegram_draft.pysharenote-telegram-pack/templates/publishing-note-template.mdsharenote-telegram-pack/templates/telegram-message-template.mdsharenote-telegram-pack/workflows/default-publishing-flow.mdstatic/boot.jsstatic/facts.jsstatic/index.htmlstatic/messages.jsstatic/multiagent.jsstatic/panels.jsstatic/style.cssstatic/ui.jstests/test_fact_routes.pytests/test_facts_store.pytests/test_multiagent_profile_api.pytests/test_multiagent_sessions.py
Comment on lines
+67
to
+70
| def _write_jsonl(path: Path, rows: Iterable[dict[str, Any]]) -> None: | ||
| path.parent.mkdir(parents=True, exist_ok=True) | ||
| text = ''.join(json.dumps(row, ensure_ascii=False, sort_keys=True) + '\n' for row in rows) | ||
| path.write_text(text, encoding='utf-8') |
There was a problem hiding this comment.
Make JSONL rewrites atomic.
_write_jsonl() writes straight to the live file. If the process dies mid-write, the candidates file can be truncated, and _read_jsonl() will silently drop the broken rows on the next load. That makes approve/reject capable of corrupting the local store.
Suggested fix
def _write_jsonl(path: Path, rows: Iterable[dict[str, Any]]) -> None:
path.parent.mkdir(parents=True, exist_ok=True)
text = ''.join(json.dumps(row, ensure_ascii=False, sort_keys=True) + '\n' for row in rows)
- path.write_text(text, encoding='utf-8')
+ tmp = path.with_suffix(path.suffix + '.tmp')
+ tmp.write_text(text, encoding='utf-8')
+ tmp.replace(path)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/facts.py` around lines 67 - 70, _write_jsonl currently writes directly to
the target file which can leave it truncated if the process crashes; change it
to write atomically by creating a temp file in the same directory (e.g., using
tempfile.NamedTemporaryFile or a .tmp Path in path.parent), write the JSONL
contents to that temp file, flush and fsync the file, then atomically replace
the target with os.replace(temp_path, path) (and optionally fsync the parent
directory) so readers (_read_jsonl) never see a partially-written file; keep
path.parent.mkdir as is so the temp file can be created.
Comment on lines
+1420
to
+1470
| def _handle_memory_candidate_create(handler, body): | ||
| try: | ||
| require(body, 'statement') | ||
| candidate = facts_store.create_candidate( | ||
| category=body.get('category', 'knowledge'), | ||
| scope=body.get('scope', 'global'), | ||
| scope_ref=body.get('scope_ref', 'default'), | ||
| statement=body.get('statement', ''), | ||
| source_session_id=body.get('source_session_id'), | ||
| source_message_ids=body.get('source_message_ids') or [], | ||
| confidence=body.get('confidence'), | ||
| reason=body.get('reason'), | ||
| sensitivity=body.get('sensitivity', 'internal'), | ||
| recommended_action=body.get('recommended_action', 'approve'), | ||
| metadata=body.get('metadata') or {}, | ||
| ) | ||
| return j(handler, {'ok': True, 'candidate': candidate, 'summary': facts_store.store_summary()}) | ||
| except ValueError as e: | ||
| return bad(handler, str(e), 400) | ||
| except Exception as e: | ||
| return bad(handler, str(e), 500) | ||
|
|
||
|
|
||
| def _handle_memory_candidate_approve(handler, body): | ||
| try: | ||
| candidate_id = body.get('candidate_id') or body.get('id') | ||
| if not candidate_id: | ||
| return bad(handler, 'candidate_id is required') | ||
| result = facts_store.approve_candidate(candidate_id, edited_statement=body.get('edited_statement')) | ||
| return j(handler, {'ok': True, **result, 'summary': facts_store.store_summary()}) | ||
| except KeyError as e: | ||
| return bad(handler, str(e), 404) | ||
| except ValueError as e: | ||
| return bad(handler, str(e), 409) | ||
| except Exception as e: | ||
| return bad(handler, str(e), 500) | ||
|
|
||
|
|
||
| def _handle_memory_candidate_reject(handler, body): | ||
| try: | ||
| candidate_id = body.get('candidate_id') or body.get('id') | ||
| if not candidate_id: | ||
| return bad(handler, 'candidate_id is required') | ||
| candidate = facts_store.reject_candidate(candidate_id, reason=body.get('reason')) | ||
| return j(handler, {'ok': True, 'candidate': candidate, 'summary': facts_store.store_summary()}) | ||
| except KeyError as e: | ||
| return bad(handler, str(e), 404) | ||
| except ValueError as e: | ||
| return bad(handler, str(e), 409) | ||
| except Exception as e: | ||
| return bad(handler, str(e), 500) |
There was a problem hiding this comment.
Wire these handlers into handle_post().
These helpers are never dispatched from handle_post(), so the new inbox stays read-only: every create/approve/reject request from static/facts.js will 404.
Suggested router wiring
+ if parsed.path == '/api/memory-candidates':
+ return _handle_memory_candidate_create(handler, body)
+
+ if parsed.path == '/api/memory-candidates/approve':
+ return _handle_memory_candidate_approve(handler, body)
+
+ if parsed.path == '/api/memory-candidates/reject':
+ return _handle_memory_candidate_reject(handler, body)🧰 Tools
🪛 Ruff (0.15.12)
[warning] 1439-1439: Do not catch blind exception: Exception
(BLE001)
[warning] 1454-1454: Do not catch blind exception: Exception
(BLE001)
[warning] 1469-1469: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/routes.py` around lines 1420 - 1470, The three handlers
_handle_memory_candidate_create, _handle_memory_candidate_approve, and
_handle_memory_candidate_reject are implemented but never dispatched from
handle_post(), so POST requests for create/approve/reject return 404; update the
request routing in handle_post() to recognize the corresponding POST paths or
action names used by static/facts.js and call these helpers (e.g., call
_handle_memory_candidate_create(handler, body) for the create route,
_handle_memory_candidate_approve(handler, body) for approve, and
_handle_memory_candidate_reject(handler, body) for reject), ensure you extract
the request body the same way handle_post() does and return their responses
directly so the inbox is writable.
Comment on lines
+9
to
+13
| function startServer(){ | ||
| const isWin = process.platform === 'win32'; | ||
| const cmd = isWin ? 'python' : './start.sh'; | ||
| const args = isWin ? ['server.py', '--port', port] : [port]; | ||
| server = spawn(cmd, args, {cwd: repo, shell: isWin, stdio: 'ignore'}); |
There was a problem hiding this comment.
Critical: Windows server spawned on wrong port — Electron will always fail to connect when HERMES_WEBUI_PORT is unset
server.py does not accept a --port CLI argument; it reads the port exclusively from the HERMES_WEBUI_PORT environment variable (see api/config.py:27-29). On Windows the --port flag is silently ignored, so server.py falls back to its own default of 8787, while main.js connects to 8788 (its own default). The window always loads a connection-refused error page on a cold install.
start.sh (non-Windows path) already receives the port as a positional argument and re-exports it as HERMES_WEBUI_PORT, so macOS/Linux are unaffected.
Fix: drop --port from Windows args and inject the port via the env option of spawn:
🐛 Proposed fix
function startServer(){
const isWin = process.platform === 'win32';
const cmd = isWin ? 'python' : './start.sh';
- const args = isWin ? ['server.py', '--port', port] : [port];
- server = spawn(cmd, args, {cwd: repo, shell: isWin, stdio: 'ignore'});
+ const args = isWin ? ['server.py'] : [port];
+ server = spawn(cmd, args, {
+ cwd: repo, shell: isWin, stdio: 'inherit',
+ env: { ...process.env, HERMES_WEBUI_PORT: port },
+ });
}(stdio: 'inherit' is also suggested here so server errors surface during development; revert to 'ignore' for production if desired.)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@desktop-install-pack/electron-wrapper/main.js` around lines 9 - 13, In
startServer, the Windows spawn currently passes a '--port' arg which server.py
ignores; update the Windows branch so args do not include '--port' (use
['server.py'] or empty as appropriate) and pass the port via the spawn env (set
HERMES_WEBUI_PORT = port while preserving process.env) so server.py reads the
correct port; adjust the spawn call that assigns to server (spawn(cmd, args,
{cwd: repo, shell: isWin, env: {...process.env, HERMES_WEBUI_PORT:
String(port)}, stdio: 'inherit' /* or 'ignore' for production */})) to ensure
the server binds to the intended port.
| win.webContents.setWindowOpenHandler(({url}) => { shell.openExternal(url); return {action:'deny'}; }); | ||
| } | ||
|
|
||
| app.whenReady().then(()=>{ startServer(); setTimeout(createWindow, 1800); }); |
There was a problem hiding this comment.
Race condition: 1800 ms is not enough on slow machines or cold starts
start.sh kills existing processes, may activate a venv, and runs server.py — any of which can exceed 1800 ms, leaving the window showing a connection error. A lightweight health-probe loop is far more reliable:
⏱️ Proposed fix — poll /health before opening the window
-app.whenReady().then(()=>{ startServer(); setTimeout(createWindow, 1800); });
+function waitForServer(cb, n = 0) {
+ require('http').get(`http://127.0.0.1:${port}/health`, cb)
+ .on('error', () =>
+ n < 30 ? setTimeout(() => waitForServer(cb, n + 1), 500) : cb()
+ );
+}
+app.whenReady().then(() => { startServer(); waitForServer(createWindow); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@desktop-install-pack/electron-wrapper/main.js` at line 22, Replace the fixed
1800ms delay after startServer() with a health-check polling loop that waits for
the server to be ready before calling createWindow(): implement a helper (e.g.,
pollServerHealth or waitForServerReady) that repeatedly requests the server's
/health endpoint with a short interval and timeout/max-retries, call
startServer() then await pollServerHealth() inside the app.whenReady() promise,
and only call createWindow() after the health check succeeds (on failure after
max retries log an error and show a retry/fatal UI instead of opening the
window). Ensure the code uses the existing startServer() and createWindow()
symbols and removes the setTimeout-based call.
| } | ||
|
|
||
| app.whenReady().then(()=>{ startServer(); setTimeout(createWindow, 1800); }); | ||
| app.on('window-all-closed', ()=>{ if(server) server.kill(); if(process.platform !== 'darwin') app.quit(); }); |
There was a problem hiding this comment.
macOS: server is killed on window-close but there is no activate handler — the app becomes permanently unusable
When all windows are closed on macOS, server.kill() runs unconditionally (the darwin guard only protects app.quit()). The Electron process stays alive in the dock, but the backend is dead. Clicking the dock icon fires activate — which has no handler — so no window reopens and no server restarts. The only recovery is to force-quit and relaunch.
The standard Electron pattern keeps the server alive while the app is running on macOS, kills it only on actual quit, and recreates the window on activate:
🔒️ Proposed fix
-app.on('window-all-closed', ()=>{ if(server) server.kill(); if(process.platform !== 'darwin') app.quit(); });
+app.on('before-quit', () => { if (server) server.kill(); });
+app.on('window-all-closed', () => { if (process.platform !== 'darwin') app.quit(); });
+app.on('activate', () => { if (BrowserWindow.getAllWindows().length === 0) createWindow(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@desktop-install-pack/electron-wrapper/main.js` at line 23, The current
app.on('window-all-closed') handler unconditionally calls server.kill(), which
kills the backend on macOS when windows close; change the logic so server.kill()
is only invoked when the app is actually quitting (i.e., keep the server running
when process.platform === 'darwin' and only kill it in an app.on('before-quit'
or quit) handler), and add an app.on('activate') handler that recreates the
window (call the existing createWindow or equivalent) and restarts/creates the
server if it is not running so clicking the dock icon restores the UI and
backend; ensure you reference and update the handlers around
app.on('window-all-closed'), app.on('activate'), and the server lifecycle calls
(server.kill(), server start/creation function) accordingly.
Comment on lines
+355
to
+373
| function detectPaperclipApprovalScope(){ | ||
| const msgs=(S&&S.messages)||[]; | ||
| const patterns={ | ||
| full:[/full execution/, /전체 반영/, /전부 반영/, /이 안으로 반영해/, /승인, 이 안으로 반영해/], | ||
| issue:[/issue create/, /이슈 생성 승인/, /이슈 만들어/, /issue 생성/], | ||
| comment:[/comment only/, /코멘트만/, /comment 승인/, /comment only 승인/], | ||
| generic:[/(^|\s)승인(,|\s|$)/, /반영해/, /실행해/], | ||
| }; | ||
| let scope='none'; | ||
| for(const m of msgs){ | ||
| if(!m || m.role!=='user') continue; | ||
| const text=String(m.content||'').trim(); | ||
| if(!text) continue; | ||
| if([/좋아/,/괜찮네/,/그 방향으로 보자/,/맞는 듯/,/오케이/].some(rx=>rx.test(text))) continue; | ||
| if(patterns.full.some(rx=>rx.test(text))) scope='full'; | ||
| else if(patterns.issue.some(rx=>rx.test(text)) && scope!=='full') scope='issue'; | ||
| else if(patterns.comment.some(rx=>rx.test(text)) && !['full','issue'].includes(scope)) scope='comment'; | ||
| else if(patterns.generic.some(rx=>rx.test(text)) && scope==='none') scope='full'; | ||
| } |
There was a problem hiding this comment.
Do not treat generic wording as full execution approval.
Bare phrases like 승인, 반영해, or 실행해 currently unlock full scope. That breaks the “명시적 실행승인” guarantee and can enable the most privileged Paperclip actions off casual chat wording.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@static/boot.js` around lines 355 - 373, The function
detectPaperclipApprovalScope treats patterns.generic matches as full approval,
which is too permissive; update the logic in detectPaperclipApprovalScope so
that patterns.generic does NOT set scope='full' — instead either set a
lower-privilege scope (e.g., scope='comment') or leave scope unchanged (keep
'none') unless an explicit full pattern in patterns.full matched; specifically
change the last branch that checks patterns.generic (currently "else
if(patterns.generic.some(rx=>rx.test(text)) && scope==='none') scope='full'") to
assign a non-full scope (or no assignment), ensuring only patterns.full can set
scope='full' and preserve the existing precedence rules in that function.
Comment on lines
+541
to
+550
| const artifactPath=gatedKinds.includes(kind) ? await createPaperclipResultArtifact(kind) : null; | ||
| let text=buildWorkflowPrompt(kind); | ||
| if(artifactPath){ | ||
| const scopeLabelMap={ | ||
| 'reflect-paperclip-comment':'comment', | ||
| 'reflect-paperclip-issue':'issue', | ||
| 'reflect-paperclip-full':'full', | ||
| }; | ||
| recordPaperclipWorkflowRun(scopeLabelMap[kind]||kind, artifactPath); | ||
| text += ` 반영 결과는 워크스페이스의 ${artifactPath} 파일에도 정리해줘. 최소한 summary, approval scope, generated/updated identifiers, final status 를 파일에 남겨줘.`; |
There was a problem hiding this comment.
Check S.busy before creating result artifacts/history.
Right now a click during an in-flight request still creates the Paperclip result file and records a running workflow before the busy guard returns. That leaves orphan artifacts and stale console history even though nothing actually ran.
Suggested fix
- const artifactPath=gatedKinds.includes(kind) ? await createPaperclipResultArtifact(kind) : null;
- let text=buildWorkflowPrompt(kind);
+ if(S.busy){showToast('현재 작업이 끝난 뒤 다시 시도해 주세요');return;}
+ const artifactPath=gatedKinds.includes(kind) ? await createPaperclipResultArtifact(kind) : null;
+ let text=buildWorkflowPrompt(kind);
@@
- if(S.busy){showToast('현재 작업이 끝난 뒤 다시 시도해 주세요');return;}
$('msg').value=text;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@static/boot.js` around lines 541 - 550, The handler currently calls
createPaperclipResultArtifact and recordPaperclipWorkflowRun even when S.busy is
true, causing orphan artifacts; before awaiting createPaperclipResultArtifact or
recording history, check S.busy and return/abort if true (or wrap the artifact
creation + record steps inside a conditional that only runs when !S.busy).
Specifically, guard around the artifactPath creation and the call to
recordPaperclipWorkflowRun (references: S.busy, createPaperclipResultArtifact,
recordPaperclipWorkflowRun, buildWorkflowPrompt, artifactPath) so no result file
or console history is created while a workflow is already in-flight.
Comment on lines
+616
to
+618
| const match=text.match(/\b([A-Z]{2,10}-\d+)\b/); | ||
| if(!match) return null; | ||
| const identifier=match[1]; |
There was a problem hiding this comment.
Finalize runs even when no issue key is present.
If the assistant output does not contain an ABC-123 token, this returns early and never calls finalizePaperclipWorkflowRun(). Comment-only flows are especially likely to get stuck in running forever.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@static/boot.js` around lines 616 - 618, The current logic returns early when
no issue key is found (const match = text.match(...); if(!match) return null;),
which leaves runs stuck; instead ensure finalizePaperclipWorkflowRun() is always
invoked for comment-only or no-key cases before returning. Modify the flow
around the match/identifier extraction so that when match is falsy you call
finalizePaperclipWorkflowRun() (or delegate to the same finish path used when an
identifier exists) and then return; keep using the existing symbols (match,
identifier) but guard access to identifier only when match is truthy.
Comment on lines
+180
to
+191
| async function routePromptToLanes(){ | ||
| ensureMultiAgentState(); | ||
| const input=$('multiAgentPrompt'); | ||
| const text=(input&&input.value||'').trim(); | ||
| if(!text) return; | ||
| const targetLaneIds=resolveTargetLanes(text); | ||
| if(input) input.value=''; | ||
| for(const laneId of targetLaneIds){ | ||
| const lane=getLane(laneId); | ||
| if(!lane) continue; | ||
| await sendLaneText(lane, text); | ||
| } |
There was a problem hiding this comment.
Dispatch routed lanes concurrently.
This loop waits for each lane stream to finish before starting the next one. A prompt that targets Hela + Plan + Dev therefore runs strictly serially, which defeats the new multi-agent workspace behavior.
Suggested fix
- for(const laneId of targetLaneIds){
- const lane=getLane(laneId);
- if(!lane) continue;
- await sendLaneText(lane, text);
- }
+ await Promise.all(targetLaneIds.map(async laneId=>{
+ const lane=getLane(laneId);
+ if(!lane) return;
+ await sendLaneText(lane, text);
+ }));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async function routePromptToLanes(){ | |
| ensureMultiAgentState(); | |
| const input=$('multiAgentPrompt'); | |
| const text=(input&&input.value||'').trim(); | |
| if(!text) return; | |
| const targetLaneIds=resolveTargetLanes(text); | |
| if(input) input.value=''; | |
| for(const laneId of targetLaneIds){ | |
| const lane=getLane(laneId); | |
| if(!lane) continue; | |
| await sendLaneText(lane, text); | |
| } | |
| async function routePromptToLanes(){ | |
| ensureMultiAgentState(); | |
| const input=$('multiAgentPrompt'); | |
| const text=(input&&input.value||'').trim(); | |
| if(!text) return; | |
| const targetLaneIds=resolveTargetLanes(text); | |
| if(input) input.value=''; | |
| await Promise.all(targetLaneIds.map(async laneId=>{ | |
| const lane=getLane(laneId); | |
| if(!lane) return; | |
| await sendLaneText(lane, text); | |
| })); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@static/multiagent.js` around lines 180 - 191, routePromptToLanes currently
awaits sendLaneText inside a for loop which causes requests to each lane to run
serially; instead collect sendLaneText promises for each valid lane (use getLane
and resolveTargetLanes to filter/identify lanes) and then await them
concurrently with Promise.all so all targeted lanes are dispatched in parallel;
keep clearing the input value before awaiting and preserve existing
continue/skip behavior for missing lanes.
Comment on lines
+340
to
+345
| src.addEventListener('error',()=>{ | ||
| src.close(); | ||
| lane.streamId=null; | ||
| lane.busy=false; | ||
| renderMultiAgentShell(); | ||
| resolve(); |
There was a problem hiding this comment.
Surface SSE failures inside the lane.
The generic error handler only clears busy and re-renders. If the stream handshake fails or drops before apperror, the lane is left with an empty assistant bubble and no visible failure message.
Suggested fix
src.addEventListener('error',()=>{
src.close();
+ lane.messages[lane.messages.length-1]={
+ role:'assistant',
+ content:'**Error:** stream connection failed'
+ };
lane.streamId=null;
lane.busy=false;
renderMultiAgentShell();
+ showToast(`Lane ${lane.label}: stream connection failed`);
resolve();
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@static/multiagent.js` around lines 340 - 345, The 'error' event handler on
src currently just clears lane.busy, lane.streamId and re-renders, leaving an
empty assistant bubble; update the handler in the src.addEventListener('error',
...) block to capture the ErrorEvent (or its message), set a visible error field
on the lane (e.g. lane.apperror or lane.errorMessage) with a user-friendly
message plus the native error detail, clear lane.streamId and lane.busy, call
renderMultiAgentShell(), and then resolve(); ensure you reference the existing
symbols (src.addEventListener('error', ...), lane.streamId, lane.busy,
renderMultiAgentShell(), resolve()) so the UI shows a failure message instead of
a blank assistant bubble.
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
1 participant
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
Verification
Note
Full legacy test suite currently has unrelated/stale baseline failures on this local setup, including tmp path canonicalization, old sprint assertions, and live-server endpoint expectations. The targeted changed-area tests pass.
Summary by CodeRabbit
New Features
Documentation