Skip to content

Desktop: Phase 3 (step 3) broker serves /api/pending - #72

Merged
zkann merged 1 commit into
mainfrom
desktop-broker-pending
Jun 17, 2026
Merged

Desktop: Phase 3 (step 3) broker serves /api/pending#72
zkann merged 1 commit into
mainfrom
desktop-broker-pending

Conversation

@zkann

@zkann zkann commented Jun 17, 2026

Copy link
Copy Markdown
Owner

Continues Phase 3: the broker now answers /api/pending, alongside plate / queue / procedures.

What moved

desktop/store.js ports generate_dashboard.parse_candidates + sop_next + dashboard_app._pending: the parked results awaiting a decision ("Needs your eyes"), each with:

  • a human title from the body's # Pending: X heading,
  • the candidate list (the ## Candidates fenced-json block → [{title,url,note}], with title falling back to url),
  • the downstream next SOP (the source SOP's first next:), null when there are no candidates.

Reuses the strict-UTF-8 read (a corrupt file is skipped, like Python).

Parity

Verified by semantic parity against the live FastAPI.

Read-migration status

The broker now serves the static reads directly: plate, queue, procedures, pending. Still forwarded:

  • settings — its terminal field is detected from the serving process's TERM_PROGRAM, which differs between the broker and FastAPI, so it can't be ported cleanly; it stays on FastAPI.
  • the liveness-bearing reads (inflight, runs) and the SSE live mirror — these need the flock/pid liveness, the Phase 5 native-layer migration.

Tests

node --test: 23 (pending: candidates + url-title fallback, plain-heading title, next from the source SOP, non-pending exclusion, empty-when-absent) + the live parity check.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added support for pending items functionality with a new /api/pending endpoint to track and retrieve parked results.
  • Tests

    • Added test coverage for pending items parsing and retrieval functionality.

@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a pending() function to desktop/store.js that scans pending/*.md files, filters by status: pending, parses candidate JSON blocks, resolves next-SOP ids from linked SOP frontmatter, and exports it. The broker registers a new /api/pending local endpoint backed by this function. Tests cover the parsing, filtering, and fallback behaviors.

Changes

Pending store reader and broker endpoint

Layer / File(s) Summary
store.pending() implementation and export
desktop/store.js
Adds parseCandidates (extracts and JSON-parses a fenced ## Candidates block), findSopFile (recursively locates ${sopId}.md), sopNext (reads first next id from SOP frontmatter), and pending (scans pending/*.md, filters by status, builds structured result per item). Exports pending alongside plate, queue, and procedures.
Broker /api/pending wiring
desktop/broker.js
Adds /api/pending to the SERVED route map calling store.pending(sopDir), and updates the comment describing the served/forwarded routing split.
Tests for store.pending()
desktop/store.test.js
Three new tests validate pending-item filtering, title extraction with Pending: prefix stripping, candidate field normalization and coercion, next-SOP derivation, empty candidates default, and missing pending/ directory handling.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • zkann/smbos#69: Introduces the broker reverse-proxy facade in desktop/broker.js that this PR extends with the /api/pending locally-served endpoint.
  • zkann/smbos#70: Extends desktop/broker.js and desktop/store.js with the same local static API response pattern (e.g., /api/plate, /api/queue) that /api/pending now follows.

Poem

🐇 Hopping through the pending/ path,
I parse each file with careful math.
Candidates JSON, titles trimmed neat,
A next SOP to keep the beat!
The broker now serves what the store has found —
/api/pending is homeward bound! 🥕

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: the broker now serves the /api/pending endpoint. It is concise, specific, and directly reflects the primary objective of this PR.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch desktop-broker-pending

Comment @coderabbitai help to get the list of available commands and usage tips.

The broker now also answers /api/pending from the pending/ files directly: the parked
results awaiting a decision ('Needs your eyes'), each with its human title (from the
body's '# Pending: X' heading), the candidate list (the '## Candidates' fenced-json
block), and the downstream SOP for an apply (the source SOP's first 'next:'). Ports
generate_dashboard.parse_candidates + sop_next + dashboard_app._pending into store.js,
reusing the strict-UTF-8 read so a corrupt file is skipped like Python.

Verified by semantic parity against the live FastAPI. With this, the broker serves the
static reads (plate, queue, procedures, pending) directly; FastAPI still owns settings
(its terminal field is detected from the serving process's TERM_PROGRAM env, which
differs between the broker and FastAPI, so it can't be ported cleanly) and the
liveness-bearing reads + the SSE live mirror (the Phase 5 flock/pid migration).

- desktop/store.js: parseCandidates, sopNext (non-pruned rglob-style find), pending().
- tests: candidates incl. url-title fallback, plain-heading title, next from the source
  SOP, non-pending exclusion, empty when absent (23 total) + the live pending parity check.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@zkann

zkann commented Jun 17, 2026

Copy link
Copy Markdown
Owner Author

Adversarial self-review hardening (folded into the latest push):

  • [P2] non-string candidate fields: Python str([...]) vs JS String([...]) diverge for a non-scalar title/url/note. Now coerces non-strings to "" (so a list/dict title falls back through to the url or "Item") -- identical to Python for valid string fields, saner than emitting "[object Object]" for malformed ones. Test added.
  • [P3] truncation: now truncates by CODE POINT (Array.from(...).slice) to match Python [:n] and avoid splitting a surrogate pair on an astral-char title near the limit.
  • [P2 NaN/Infinity, accepted+documented]: JSON.parse rejects bare NaN/Infinity (invalid JSON) where Python json.loads accepts them -> such a non-standard block yields [] here. Not regex-stripped, since that would corrupt a string value legitimately containing "NaN"; the broker following the JSON spec is the more correct behavior.
  • [P3 sop_id glob, noted]: Python rglob globs a sop id with *; the Node finder is exact-match. sop comes from frontmatter (not attacker-controlled); exact match is arguably more correct.
    node --test green; live pending parity exact.

@zkann
zkann force-pushed the desktop-broker-pending branch from a258b13 to 7bf51bc Compare June 17, 2026 23:45

@coderabbitai coderabbitai 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.

🧹 Nitpick comments (1)
desktop/store.test.js (1)

118-125: 💤 Low value

Consider adding a test for code-point-safe truncation.

The implementation truncates titles at 140 code points, URLs at 500, and notes at 300 using Array.from().slice() to handle astral characters correctly. A test with a long string containing emoji or other multi-byte characters would verify this edge case and serve as regression protection for the code-point vs code-unit distinction.

📝 Example test for truncation
test('pending candidates: truncates by code point, not code unit', () => {
  const d = tmpSop(); fs.mkdirSync(path.join(d, 'pending'))
  const longTitle = '🎉'.repeat(150)  // 150 emoji = 150 code points, but 300 UTF-16 code units
  fs.writeFileSync(path.join(d, 'pending', 'p.md'),
    '---\nstatus: pending\nsop: s\n---\n# T\n## Candidates\n```json\n' +
    `[{"title":"${longTitle}","url":"http://x"}]\n\`\`\`\n`)
  const title = store.pending(d)[0].candidates[0].title
  assert.equal(Array.from(title).length, 140)  // truncated to 140 code points
  assert.ok(title.endsWith('🎉'))  // no broken surrogate pairs
})
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@desktop/store.test.js` around lines 118 - 125, Add a new test function to
verify that store.pending() correctly performs code-point-safe truncation for
titles when handling multi-byte characters like emoji. Create a test that writes
a pending markdown file with a title containing many emoji characters (which
consume multiple UTF-16 code units per code point), calls store.pending(), and
asserts that the resulting title is truncated to exactly 140 code points
(verified using Array.from(title).length) and doesn't contain broken surrogate
pairs, ensuring the implementation correctly uses Array.from().slice() instead
of relying on code-unit based string operations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@desktop/store.test.js`:
- Around line 118-125: Add a new test function to verify that store.pending()
correctly performs code-point-safe truncation for titles when handling
multi-byte characters like emoji. Create a test that writes a pending markdown
file with a title containing many emoji characters (which consume multiple
UTF-16 code units per code point), calls store.pending(), and asserts that the
resulting title is truncated to exactly 140 code points (verified using
Array.from(title).length) and doesn't contain broken surrogate pairs, ensuring
the implementation correctly uses Array.from().slice() instead of relying on
code-unit based string operations.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 962c2c2a-219f-4c27-bbd0-661a79f50c55

📥 Commits

Reviewing files that changed from the base of the PR and between c1531d9 and 7bf51bc.

📒 Files selected for processing (3)
  • desktop/broker.js
  • desktop/store.js
  • desktop/store.test.js

@zkann
zkann merged commit f5cc2fc into main Jun 17, 2026
7 checks passed
@zkann
zkann deleted the desktop-broker-pending branch June 17, 2026 23:55
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