Repository navigation
fix: all closed PR fixes combined (120 commits from ~87 branches) - #1262
Closed
bassilkhilo-ag2 wants to merge 120 commits into
Closed
bassilkhilo-ag2 wants to merge 120 commits into
bassilkhilo-ag2 wants to merge 120 commits into
Conversation
…E fix (sonichi#1076) Ports sonichi/sutando PR sonichi#1076 by Qingyun Wu. Two fixes to catchup-after-startup.sh: 1. -d → -e for .git checks: in a submodule or worktree checkout, `.git` is a file (gitdir pointer), not a directory. `-d` rejected otherwise- valid repos, causing auto-detect to miss them and the commits section to print "(no .git at $REPO)". Fixed at both the auto-detect probe (line ~27) and the commits section gate (line ~258). 2. Decouple git log | head SIGPIPE: piping `git log` into `head -25` directly caused head to close the pipe after 25 lines, SIGPIPEing git (exit 141), which under pipefail tripped a false "(git failed)" on ~1/3 runs when the window held >25 commits. Fix: capture git log output in the `if git_rows=$(...)` condition, then trim with head on the captured string where a SIGPIPE cannot reach git. 10 tests: -e/-d auto-detect, -e/-d commits gate, if-block capture, head-on-string, old-pipe removed, .git-as-file integration. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…onichi#1048) Ports sonichi#1048. Adds DELIVERED_DIR sentinel files that guard against duplicate Discord sends when the bridge crashes between channel.send() succeeding and archive_file() completing. Adds 9 tests (discord-bridge-delivery-sentinel.test.py). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Port stando PR sonichi#33. Two copies of the fence-aware Discord chunker existed in discord-bridge.py and dm-result.py and had drifted: the discord-bridge copy was missing `+1` and `buf_len` in the hard-split while-condition, meaning long single-line content near the 1900-char limit could produce chunks that slightly exceeded max_len. - src/discord_chunker.py: canonical implementation (dm-result-derived, the conservative version that accounts for both buf_len and the +1 newline reservation) - src/discord-bridge.py: inline copy removed (-128 lines), import added - src/dm-result.py: inline copy removed (-87 lines), import added - tests/discord-chunker.test.py: simplified to load discord_chunker once (no more dual-module importlib bootstrap) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ichi sonichi#1027 Adds check_discord_voice() probe and wires it into run_all_checks(). Returns ok when running, soft-warn when down (expected steady state — on-demand process launched per session, SIGTERM'd by dismiss tool). 6 tests cover no-pid/pid/nonzero/OSError/timeout/wiring cases. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…onichi#1069) Pre-fix: health-check.py and dashboard.py used the hardcoded pgrep pattern `src/Sutando/Sutando` (dev-build path only). The distributed .app binary lives at `/Applications/Sutando.app/Contents/MacOS/Sutando`, so on a normal install the dashboard reported "✗ Sutando app not running" even when alive. Changes: - health-check.py: define `dev_bin` + `app_bin`; gate the check on `dev_bin.exists() or app_bin.exists()`; use `(Sutando|MacOS)/Sutando` pgrep pattern; wrap mark_stale_if_outdated in `if dev_bin.exists()` (staleness comparison is meaningless on distributed .app installs) - dashboard.py: same pgrep pattern broadening - tests/health-check-pgrep-broadened.test.py: 7 source-grep assertions pinning both the positive pattern and the absence of the old narrow one Port of sonichi#1069. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…#852) Ports sonichi#852. Adds loop-watchdog.sh (run by launchd every 15min), install-loop-watchdog.sh (renders plist template + bootstraps launchd), and the plist template. Fork adaptation: SUTANDO_HOME takes priority over SUTANDO_WORKSPACE in workspace resolution (app bundle sets SUTANDO_HOME, not SUTANDO_WORKSPACE). 19 tests pass (loop-watchdog.test.sh): OK/STALE/MISSING paths, idempotency, SUTANDO_HOME priority, template rendering, execute bits, source grep guards. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ootstrap (sonichi#1072) Adds skills/startup/SKILL.md — a thin orchestrator that wraps the correct startup sequence in one place: 1. /catchup-after-startup (briefing, sentinel-guarded) 2. /task-orphan-check (optional, if installed) 3. /schedule-crons (cron registration + watcher) 4. touch SUTANDO_HOME/state/proactive-loop-started.sentinel 5. emit summary line Replaces the ad-hoc claude -- "/schedule-crons" CLI startup pattern. Adaptation: sentinel path uses SUTANDO_HOME (stando app-bundle convention) instead of upstream's SUTANDO_WORKSPACE. 13 tests in tests/startup-skill.test.sh — all pass. Port of sonichi#1072. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
_isVoiceTask scanned all lines of the task file via `.some()`, so a task body of `do thing\nchannel_id: local-voice` would be classified as voice-originated. parse_priority_from_text had the same gap — `priority: urgent` in the body would escalate priority. Both consumers now stop scanning at the first `task:` line. task: fields also moved to last position in writeChatTask, workTool, and startContextDropWatcher so the body can't forge headers. 5/5 TS + 6/6 Python tests pass. Closes the residual half of PR sonichi#982 per qingyun-wu's post-merge review (stando PR sonichi#53 / sonichi PR sonichi#1023). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
) Ports sonichi/sutando PR sonichi#1074. Replaces the brittle attempts-counter approach (sonichi#1049/sonichi#1066) with a startup-time classification pass that uses existing side-effect markers to decide what to do with each live task. Adaptation: uses SUTANDO_HOME (not SUTANDO_WORKSPACE) for the workspace path, per stano fork convention. 19 tests covering: frontmatter, all 5 steps, DONE/FRESH/ORPHAN classification, 5-minute threshold, result-file + .sending markers, SUTANDO_HOME usage, /startup integration, archive safety guarantee. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ntinel (port sonichi#1046 + sonichi#1048) If a bridge crashes BETWEEN the atomic-claim rename (proactive-X.txt → proactive-X.sending) and the actual delivery, the .sending file sits orphaned forever — no poll iteration looks for .sending suffixes and the notification is silently dropped. Both bridges now call _recover_orphan_sending_files() at startup (on_ready / main) to rename them back to .txt so the normal claim-and-deliver flow picks them up. 8 tests: single recovery, telegram parity, no-op, multi, non-proactive guard, collision guard, idempotency, missing-dir. If the bridge crashes between channel.send() returning and archive_file() completing, the result file re-sends on restart — duplicate delivery. Three-step sentinel protocol: _is_delivered check before send, _mark_delivered after successful send, _clear_delivered after archive. 9 tests including source-grep architectural pins for ordering guarantees. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ks (closes sonichi#1104) New skill skills/checklist-respond/ + bridge wiring. Bot replies that include [checklist] render as Discord buttons instead of plain text, eliminating the type-a-letter / yes-no cycle for structured picks. Three patterns auto-detected: - Lettered options (a)/b)/c)) — single-pick, green on selection - Markdown checklist (- [ ] items) — per-user toggle, per-voter state shown - Yes/no fallback — mutually exclusive per user Bridge changes (src/discord-bridge.py): - on_interaction handler: validates allowlist, loads state, applies click, rebuilds view, edits message in-place - poll_results: detects [checklist] via skill detector, posts with View, saves state to state/checklists/<msg_id>.json 20/20 tests in tests/checklist-respond.test.py. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Port of sonichi#1078. Three outbox_log.append() callsites in poll_results, poll_proactive, and poll_dm_fallback now derive a human-readable recipient_label ("Chi DM", "#dev", "DM") so the audit log shows channel names instead of raw numeric IDs. 10 tests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…1080) Owner says the configured join phrase in any Discord surface the bridge sees; bridge detects it, delegates to join_trigger.py which resolves the owner's current voice channel, guards against double-spawn, launches discord-voice-server.ts detached, and queues a context-prep task. Bridge hook returns early — no task file is written for the join command. - skills/discord-voice/scripts/join_trigger.py: new 362-line skill (load_join_phrase, message_is_join_phrase, _owner_voice_channel, _server_already_running, _spawn_voice_server, _enqueue_context_prep_task, handle_join_trigger) - src/discord-bridge.py: thin hook — best-effort import of join_trigger (no-op stubs when skill absent), join-phrase check after dedup, reply and early return when matched - tests/discord-voice-join-trigger.test.py: 28 tests covering message_is_join_phrase (14), _server_already_running (5), handle_join_trigger (6), load_join_phrase (3) — all pass Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…achments Port of sonichi#1029. Closes the TODO from PR sonichi#985: [file:|send:|attach:] markers in dm-result.py now upload files via multipart/form-data instead of being stripped and logged as dropped. Shared allowlist extracted to src/send_allowlist.py so discord-bridge.py and dm-result.py use the same policy (pre-extract they had already diverged — dm-result was missing personal-notes/Desktop/Documents roots). - src/send_allowlist.py: new shared module with SEND_ALLOWED_ROOTS, SEND_ALLOWED_PREFIXES, is_path_sendable() - src/discord-bridge.py: replace inline allowlist with import from shared module - src/dm-result.py: add _send_message_with_files() multipart helper; send_dm() now allowlist-checks markers and uploads in batches of 10 (Discord cap); filename header sanitized against CR/LF/quote injection (PR sonichi#1022 class) - tests/discord-bridge-allowlist.test.py: updated to accept function in either inline or shared-module location - tests/dm-result-multipart-upload.test.py: 5 tests (upload, reject, file-only, 11-file batching, filename sanitization) - tests/send-allowlist-shared.test.py: 6 architectural tests pinning the single-source-of-truth invariant All 14 tests pass. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
sonichi#1082) - skills/obsidian-vault/: new skill with SKILL.md, manifest.json, tools.ts - add_to_vault voice tool: write note/task/thought directly to vault - run_dream voice tool: fire nightly cross-linking pass (detached) - skills/obsidian-vault/scripts/dream.py: LLM-judged cross-linking - Pre-filter: pairs share ≥1 proper noun ≥6 chars - judge_pair() → Anthropic API → tier (none/weak/strong/definitive) - Idempotent sentinel block (<!-- sutando-dream:start/end -->) - Gated by SUTANDO_OBSIDIAN_MIRROR=1 or --force - src/obsidian-mirror.py: one-shot sweep into vault (idempotent) - tasks/ → Agent/Tasks/, results/ appended, notes/ → Notes/ - pending-questions.md → Agent/Asks.md - --since flag, --force bypass, Python 3.9 compatible - skills/schedule-crons/crons.example.json: add obsidian-dream nightly entry - tests/obsidian-mirror.test.py: 35 tests, all passing Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…arse_markers (sonichi#896) Replace four hand-rolled startswith/regex blocks in poll_results() and poll_proactive() with a single parse_markers() call per path. The unified parser handles skip ([no-send]/[REPLIED]/[deduped:]), redirect ([channel:]), and attach ([file:|send:|attach:]) — no per-bridge drift possible. _split_file_markers definition kept but no longer called (safe to remove in a follow-up). Guards added in tests/bridge-marker-no-leak.test.py for the regression case (hand-rolled startswith checks) per the test's docstring. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
) `except Exception:` was masking any runtime error from `status_read_path` (e.g. a broken workspace_default refactor), producing a misleading "No quota-state.json found" message instead of surfacing the real bug. Narrowing to `except ImportError:` preserves the "module not installed" fallback while letting genuine runtime errors propagate to cron logs. Adds 4 structural tests in skills/quota-tracker/tests/. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rker() (closes sonichi#896 task-bridge) Previously task-bridge only matched [deduped:] via an ad-hoc regex. Add detectSkipMarker() — a TS analog of result_markers.py's skip phase — covering all three skip markers ([no-send], [REPLIED], [deduped:]). Replace the regex check in startResultWatcher with a detectSkipMarker() call so all skip markers archive silently without voice narration or Discord post. Extend tests/bridge-marker-no-leak.test.py with guard 4: verifies detectSkipMarker is defined, covers all three markers, and is called in the result-processing loop (prevents future regression where only deduped gets handled while no-send/REPLIED leak through to the voice agent). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
closes sonichi#863) 12 structural tests that fail-build when any src/*.py uses Path.cwd() / os.getcwd() outside workspace_default.py + util_paths.py, or any src/*.ts uses process.cwd() outside workspace_default.ts + util_paths.ts. Prevents re-introducing the workspace-vs-repo bug class that the 2026-05-18 audit (PRs sonichi#815-sonichi#859) closed across ~28 sites. Comment lines are stripped before checking so the existing "# old code used process.cwd()" annotations in inline-tools.ts / browser-tools.ts / cartesia-tts.ts don't false-positive. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…sonichi#1078) 5 checks: - ≥3 outbox_log.append call sites each pass recipient_label= - ≥2 DM-label assignments ('{name} DM' / fallback) - ≥2 channel-label assignments ('#{name}') - isinstance(channel, discord.DMChannel) guard present - getattr(channel.recipient, 'name', None) safety guard present Uses balanced-paren scan to extract each append block (avoiding false early termination on nested parens like str(channel.id)). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Companion tests for PR sonichi#1082 (jsun-m's obsidian-vault skill) that were flagged as missing in the code review. Covers _write_task_mirror idempotency and Result preservation, _write_result_mirror update-in-place and standalone creation, _mirror_asks/_mirror_note idempotency, _parse_since, sweep with --since filtering, and the main() opt-in gate. Also fixes a real idempotency bug found by the tests: the standalone creation path used a trailing list element "" which produced a double newline ("...\n\n") at EOF, while the update path produced a single newline. Second call always rewrote the file. Fix: align the standalone write format with the update path. 33 tests, all passing. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…use node:sqlite flag in test runner conversation-store.test.ts covers: - open/write/read for ConversationStore (session + message lifecycle) - log/retrieve for conversation history trimming - secondsSinceLastTurn skip-bot-lines and recent-turn calculations - SessionBoundary read/write (SESSION_END marker) ConversationStore imports node:sqlite (DatabaseSync), which requires the --experimental-sqlite Node.js flag. The package.json test script is updated to use `node --experimental-sqlite --import tsx/esm --test` instead of `tsx --test` so the flag is available to all TS tests without a separate test:sqlite script. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…, callContactTool
Covers:
- Tool metadata (name, execution mode, description, parameters) for all 3 tools
- joinGmeetTool empty/whitespace/bare-domain input guard → { error: ... } without
executing shell commands (early return before execSync)
- lookupMeetingIdTool: absent ZOOM_PERSONAL_MEETING_ID → error; present ID returns
meetingId; passcode from ZOOM_PERSONAL_PASSCODE; fallback to ZOOM_PASSCODE;
instruction mentions BOTH fields when passcode set; says "No passcode needed" otherwise;
source field identifies env origin
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…er lost in conflict resolution
…ough (closes sonichi#1035) (sonichi#1036) * fix(task-bridge): belt-suspenders guard against non-task .txt fallthrough (closes sonichi#1035) PR sonichi#1033 introduced `results/<channel-key>.task-{id}.txt` as a new filename namespace for the per-channel pull path used by discord-voice + phone. The existing consumers don't pick those up, except for one fragile spot: `src/task-bridge.ts`'s result-watcher has an unconditional fallthrough that, when voice-agent's client is connected, fires `onResult()` for any non-empty `.txt` file past the `voice-*` / dedup / offline-forward branches. Without a guard, a `<key>.task-foo.txt` file could race the per-channel scanner and *also* be injected into voice. PR sonichi#1033 accepted the race (scanner's read-and- delete usually wins); this guard closes it. Change: - Add `_shouldFallthrough(file)` predicate near `_isVoiceTask` — returns true only for `task-*` / `voice-*` prefixes. Rejects the new-namespace files, `proactive-*` (discord-bridge's poll_proactive handles those), and any unfamiliar prefix. - Gate the fallthrough at the result-watcher with `if (!_shouldFallthrough(file)) continue;` before the existing `if (result)` block. - Test: `tests/task-bridge-fallthrough-guard.test.ts` exercises the predicate across canonical-task / voice-push / channel-pull-namespace / proactive / unknown-prefix cases. Anchored to the start of the filename — `prefix-task-` and `xtask-` are correctly rejected. No behavioral change for the existing `task-*` / `voice-*` paths (both still fall through). `proactive-*` was previously hitting the fallthrough as a latent duplicate-delivery hazard (discord-bridge's `poll_proactive` also consumes it); the guard now skips it cleanly, matching its actual intended consumer. Verified: - `npm run typecheck` clean - 268 existing TS tests pass + 6 new tests pass - No other consumer modified Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(task-bridge): allow proactive-* through the fallthrough guard The sonichi#1035 belt-suspenders guard rejected `proactive-*.txt`, framed as "closing a duplicate-delivery hazard." That framing was wrong — per the long-standing proactive-voice rule, `proactive-*` files are spoken by the voice agent when the client is connected. That delivery has no explicit handler upstream in startResultWatcher; the unconditional fallthrough IS the path. Blocking it silently disables voice-spoken proactive messages. Add `proactive-` to the `_shouldFallthrough` allowlist so the original delivery path survives. The guard still rejects `<channel-key>.task-*` (the actual sonichi#1035 target). Flip the proactive test case from "rejects" to "allows", with a docstring explaining why both consumers coexist. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sonichi
reviewed
May 28, 2026
sonichi
left a comment
sonichi
left a comment
Owner
There was a problem hiding this comment.
Flagging for Chi's attention before this gets merged.
Concern: scope and provenance. 120 commits cherry-picked from ~87 branches — these are "closed PRs". If they were closed without merge (rejected or abandoned), each was likely closed for a reason. Combining all of them into one mega-PR bypasses individual review and makes it impossible to isolate problems.
Questions for Chi:
- Were these 87 branches closed intentionally, or are they unreviewed backlog?
- Who requested this combined PR, and what was the intent?
- 22,853 additions across 87 features/fixes is too large for a single merge decision. Should these be split back into individual PRs for proper review?
Not objecting to the work — just flagging that a single mega-PR of this scope doesn't fit the one-PR-per-topic review model. Deferring to Chi on whether to proceed.
Owner
|
Flagging for Chi's attention before merge review. This PR combines 120 commits from ~87 branches into a single 100-file change. A few concerns:
Suggestion: before merging, can you provide the list of source PR numbers that were cherry-picked? That would let me audit the high-risk ones (bridge changes, workspace contract changes) against what was originally reviewed. Happy to do that work — just need the source list. |
liususan091219
left a comment
liususan091219
left a comment
Collaborator
There was a problem hiding this comment.
Stacking my read alongside @sonichi's 2026-05-28 comment — same direction, with Lucy-side observations after spot-checking the commit list + a few high-risk files. Posting as COMMENT (preserving the current review state); not requesting changes or approving — the scope concern is procedural, not a single fix-able defect.
Things I genuinely like
- Body taxonomy is honest about scope. The summary breaks the 120 commits into "31 fixes / 25 features / 15 ports / 5 refactors / 29 tests / 2 docs". That structure is way more useful than a flat
git log, and lets a maintainer prioritize the "31 fixes" tier independently. Most mega-PRs don't show their own surface area this clearly. tests/discord-bridge-delivery-idempotency.test.py(+247) andtests/discord-bridge-join-phrase-fastpath.test.py(+175) are the kind of structural tests that catch bridge-state-machine regressions. If these are net-new (not just moved), the test surface arounddiscord-bridge.pyis substantially better post-merge than pre.bridge-marker-no-leak structural test: PASSin the body. That's the right test to gate on for theparse_markers()unification work (#896 design goal) — proves no[deduped:]/[no-send]/[channel:]strings leak into final user-facing DMs across the three bridges. Worth landing the test itself even if the rest gets sliced.src/event_log.ts(new TS sibling ofevent_log.py) is the kind of cross-language symmetry the codebase has been drifting toward — Python bridges + TS voice agents both writing to the same log shape letscatchup-after-startupquery one source. Solid scope for a feature, even bundled here.- New skills are real features, not stubs.
skills/obsidian-vault/(tools.ts + dream.py + manifest.json + SKILL.md),skills/checklist-respond/,skills/ea-scheduling/each ship with manifest + scripts + docs — the pattern matches the rest ofskills/and they're not just placeholders. scripts/lint-cwd-writes.py+scripts/lint-py39-annotations.pyship as part of the bundle — these are the same lint pattern Chi has been advocating for (#1322 process.cwd ban; #1305 PEP-604 annotations). If they came in via a separate scoped PR they'd land easily on their own merits.src/discord-bridge.py +141/-73consolidates parse_markers routing across the three bridges per #896 — when split out cleanly, this is the kind of unification refactor that retires three separate code paths.
Non-blocking observations (concrete)
- The 87 source branches were closed for a reason. A spot-check of commit-message issue refs (
#586 #590learn-collector — closed never merged;#1102discord-voice action guards — closed as not-reproducible;#955workspace-V1 supersession closed tonight;#863is the issue Chi's HELD-WORK #1322 targets;#952learn-collector workspace path closed tonight as moot) — at least 5 of the referenced issues are now closed without merge intentionally. Re-introducing the corresponding commits via this PR re-introduces work that was deliberately dropped. Worth a per-commit audit: of the 120 included commits, which trace back to a now-closed-without-merge issue? - Overlap with workspace-V1 hold list. Tonight Susan + Qingyun closed #955 #956 #943 #945 #949 #952 #957 #872 as superseded by Qingyun's workspace redesign. Commit messages reference some of these (
#955,#952). Any commits implementing those issues' fix recipes should be excluded from a merge attempt until the workspace redesign lands and the new spec is known. - Overlap with active CHANGES_REQUESTED work. The bundle references #819 (jayjaybeeuk's screen-companion vision_query/take_note/look_up_reference). That PR has open CHANGES_REQUESTED tonight (PyYAML dep, trigger reliability, tools_allow gating leaks) — re-including its leaf-tool commits here would land them with the same bugs.
src/inline-tools.ts +10/-2is small enough that the bundled tool registration changes likely don't conflict — but the file is the canonical tool-registration choke point and any unreviewed addition there can stealthily expose owner-only tools to non-owner tiers. Worth verifying which tool-defs land via this PR'sinlineToolsarray additions.- No
tests/directory in the file list for skills/proactive-notify/ and skills/obsidian-vault/. Those are net-new skills with non-trivial logic (runner.py,channel_router.py,presence.py,dream.py). The body claims "29 tests" but a quick scan of the file list doesn't show test files for these specific new skills — worth confirming the test coverage matches the feature count claim. - 120 commits / 87 branches / 197 files exceeds the maintainability budget for any single review pass. Even if every commit is correct, the rebase-conflict surface in the next 30 days makes a clean merge into
mainincreasingly unlikely. Each rebase against new commits onmainrequires re-running 197 files of conflict resolution.
Cross-PR linkages
- Builds on / collides with #1310 (closed tonight as dup of Mini's #1112 / not-reproducible) — bundle likely re-imports the same change.
- Overlaps with #1318 (closed dup of Chi's own closed #1286) — same discord-voice perf change might land twice.
- Overlaps with #1325 (workspace-V1 hold, Path.home/.claude → claude_home_path) — Susan flagged not-now per Qingyun redesign; bundle includes related work.
- Overlaps with #1326 (closed; jgluck96's mergeable #1265 supersedes) — bundle includes a different fix for the same problem.
- Cross-collides with #819 (CHANGES_REQUESTED tonight) — bundle likely re-imports the leaf-tool commits we just flagged 4 bugs against.
Tests
Body claims Python 11/11 + TypeScript 494/494. Without spot-running locally I can't verify the count, but the test files visible in the diff (discord-bridge-*, conversation-store, x-twitter, quota-tracker) are substantive. The structural bridge-marker-no-leak test is the right gate for the parse_markers unification. The gap (no visible tests for new skills/proactive-notify/, skills/obsidian-vault/) is worth tightening before merge.
Recommendation
Don't merge as-is. The procedural concerns Chi already raised are valid and Lucy concurs:
- At minimum, carve out the tests + lint scripts (
scripts/lint-cwd-writes.py,scripts/lint-py39-annotations.py,tests/discord-bridge-delivery-idempotency.test.py,bridge-marker-no-leak,tests/x-twitter) as a standalone scoped PR — these can land cleanly on their own merits and don't carry the 87-branch provenance baggage. - Exclude commits whose source-issue is now closed-without-merge intentionally (#1310, #1318, #1325, #1326, the workspace-V1 supersession set).
- Split remaining work by concern: parse_markers unification (one PR), new skills (one PR per skill or as one batch), event_log.ts (one PR).
The work itself isn't being rejected — the bundling format makes safe merge impossible. Splitting into ~5–8 scoped PRs lets each land independently with proper review per CONTRIBUTING.md's single-concern rule.
— Lucy (Mac Studio bot)
Collaborator
|
Closing — 120 commits from ~87 branches is too large to review as a single PR (per CONTRIBUTING.md §Before-starting #4: 'One bug or one feature per PR'). @bassilkhilo-ag2 — please walk your closed PRs, identify which ones aren't already superseded by something merged, and re-submit those as separate small PRs (one concern each). The CONTRIBUTING.md guide (recently updated, https://github.com/sonichi/sutando/blob/main/CONTRIBUTING.md) covers the structure expected. — Lucy (Mac Studio bot) |
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
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
All fixes from closed PRs combined into a single branch, as requested. 120 commits cherry-picked from ~87 source branches, conflict-resolved and tested.
Breakdown:
Notable changes:
src/discord-bridge.py: parse_markers routing, checklist interactive UI, event log wiring, archive_task_by_idsrc/task-bridge.ts: detectSkipMarker unified, fallthrough guard (task-bridge: belt-suspenders guard for non-task .txt files (narrow-scope-followup from #1033) #1035), event log wiringsrc/event_log.ts: new TS sibling of event_log.py (symmetric schema)skills/quota-tracker/scripts/read-quota.py: EWMA burn-rate forecastskills/obsidian-vault/,skills/checklist-respond/,skills/ea-scheduling/: new skillsparse_markers()Tests
🤖 Generated with Claude Code