chore: promote v2.17.20 - #469
Merged
Merged
Conversation
…t-active (#437) A heartbeat report carried no target, so sendChannelOutput resolved one from the last-active slot — whichever conversation most recently spoke to the bot. On 2026-08-25 that delivered two scheduled reports into an unrelated design thread while their own channels got nothing. The last-active chain is right for a conversational reply and stays untouched. What changes is that a non-interactive sender can now opt out of it: - ChannelSendRequest.allowActiveFallback — absent keeps historical behaviour, so every existing caller is unchanged. - HeartbeatJob.destination — three operator-meaningful fields; targetKind and peerKind are derived by targetFromChatId so the file never encodes Slack's prefix rules. - PUT /api/heartbeat inherits destination on absence, the same way runner fields already do. Both shipped UIs rebuild jobs from a fixed five-field shape, so without this an ordinary save would unpin every job. An explicit null still clears, and is deliberately not folded into undefined. - The anchor records the channel actually delivered to, not a hardcoded 'active'. Jobs with no destination keep the old path on purpose: defaulting them to "do not send" would silence every existing install. Also replaces the heartbeat-routing source-order assertion, which transcribed the resolver's implementation and read as an endorsement of the misdelivery it should have caught. The behaviour now lives in tests that can actually fail. npm test: no new failures vs baseline (comm -13 empty; 3 pre-existing unrelated).
…437) Review of a91baa3 found the fix left the original failure reachable: a job that named a destination but wrote it wrong resolved to null and took the legacy path, so a report meant for one channel still went to whoever spoke last. The test added alongside it asserted that behaviour, locking the bug in. heartbeatTarget now separates the two ways it returns no target. Never named one is the legacy path and stays. Named one we cannot resolve is a stated intent the resolver cannot satisfy, and delivering it anywhere else is the bug — so the send is refused and logged. Also restores fallback-order coverage as behaviour rather than the source-order assertion this branch deleted: last-active outranks latest-seen, which outranks the configured allowlist, each proved by where a send actually lands. npm test: no new failures vs baseline. widget-watcher is timing-flaky under the full suite and passes in isolation (3/3).
The file failed intermittently only under the full suite, which made every regression check ambiguous: a run with four failures and a run with five looked the same until someone re-ran it. Two independent causes, both timing: - The waiter resolved on ANY widget_updated. A debounce timer armed by an earlier test could still fire after that test returned, so the waiter consumed a stale event and the assertion for this test's widget found nothing. Waiters now filter on their own chatId. - fs.watch does not promise when a freshly attached watch starts observing; on macOS a write issued right after startWidgetWatcher() is routinely missed, making a single write a coin flip. Writes now retry until an event arrives, which is safe precisely because the production debounce collapses a burst — the property the debounce test still asserts. The debounce assertion moved from "exactly 1" to "at least 1, fewer than 3": with retries the burst may span more than one window, and pinning the exact count was measuring the scheduler rather than the collapse. Evidence: 8 isolated runs and 3 full-suite runs, zero widget-watcher failures; full-suite failure set now byte-identical to baseline across runs.
These failed on any developer machine running opencodex and passed in CI, so the suite's failure count varied by host and no regression check was trustworthy without diffing against a per-machine baseline first. Codex completions prefer a live catalog and fall back to the static registry only when no runtime answers (opencodex-models.ts:189). With a daemon up, the two completion tests were asserting the contents of whatever it served — locally that list has no gpt-5.3-codex-spark at all. FC-007 isolated PATH but not this lookup, so model→CLI inference resolved through the same live list and chose a CLI the isolated PATH never held. Pointing CLI_JAW_OPENCODEX_DIR at an empty directory removes the runtime from the equation, which is what these tests always meant to exercise. The completion assertions now read the model id out of CODEX_MODEL_CHOICES instead of hardcoding one, so a catalog edit no longer breaks them for an unrelated reason. npm test: 8053 pass, 0 fail — the full suite is green for the first time in this branch. Previously 4 failures, one of them intermittent.
…table override (#440, #442) #440 — sanitizeProgressDetail only knew about absolute paths, localhost URLs and executable names, so a grep pattern passed through untouched. On 2026-08-25 a Slack thread displayed the agent's raw search expression, naming an internal route and two API methods, for six minutes while the real answer was still being written. It read as the answer. Patterns are now detected by shape rather than keyword: regex metacharacters are what make a string a pattern, and an ordinary detail has none. Dotted lowercase identifiers (API method names) are redacted too, with a suffix check so real filenames stay visible — a progress line that shows nothing is not worth posting. #442 — the --mutable override replaced a sentence employee.md no longer contains, so it silently matched nothing. Granting write permission left the employee reading that writes were blocked. The pattern now tracks the current wording, and the test compares the two rendered prompts instead of asserting on source text, which is what would have caught the drift in the first place. npm test: 8059 pass, 0 fail.
… text from cancelling a goal (#439, #441) #439 — killAllAgents returns once the signal is sent, but the handler that persists a turn runs after the child actually dies. Shutdown closed sqlite in between, so every restart logged "The database connection is not open" and the last turn lost its assistant message, its session row and its trace. The waiting caller was never resolved either, leaving a Slack thread to sit until its own five-minute timeout. waitForAllProcessesEnd is bounded at two seconds, inside the existing force-exit budget: a wedged child still cannot hold the process open, but the ordinary case where the child finishes in milliseconds now completes its writes. #441 — the /goal cancel interceptor archived the goal on sight of the marker, with no gate whatsoever. /goal done immediately above it demands verification evidence, so abandoning a goal was strictly easier than finishing one, and a model that merely explained the command destroyed the goal by describing it. Stopping the continuation loop never required destroying the record. The marker now clears timers and emits goal_cancel_requested; the goal stays active and a human decides. The CLI and API cancel paths are untouched.
…configured channel (#438) Three more senders had the heartbeat's defect. A reminder fires on a schedule, a web-ai notification drains when a watcher finishes, and an alert escalates after repeated CLI failures — none of them has a conversation, yet all three resolved through the last-active slot and landed wherever the bot had most recently been spoken to. preferConfiguredTarget resolves to the operator's allowlist first. It does not refuse delivery the way the heartbeat opt-out does: an operator who never set an allowlist would simply stop receiving alerts, which is worse than the bug. What changes is the preference, not the guarantee. Conversational sends are unaffected — the flag is opt-in and only these three callers set it. npm test: 8066 pass, 0 fail.
…ateways run (#444, #445) #444 — the DISCORD_TOKEN auto-switch assigned settings.channel, but the v4 migration deletes that key and applyEnvOverrides runs after it. The assignment landed on a field nothing reads, so an install with only a Discord token kept Telegram as its home channel and outbound went nowhere useful. It now writes messaging.homeChannel and adds Discord to the enabled set, because a home channel that is not enabled has no transport behind it. #445 — two halves: Classic's channel toggle rebuilt the enabled set from an empty array, so every toggle sent a singleton. Harmless when one channel could be active; under v4 it tears down every other gateway, and restartMessagingRuntime acts on that difference — someone running Slack and Telegram lost Telegram inbound by touching the Slack tab. It now reads the current set from the server first. migrateSettings treated a malformed v4 enabled set as "no set yet" and rebuilt it from the legacy scalar, which is always 'telegram' at v4 since the key it reads was deleted. A Slack install could be handed a Telegram gateway. Recognisable channels are now salvaged, with the legacy path reserved for genuine v3 documents. PUT {channel} keeps its documented translation — AGENTS.md specifies it, and changing that is a contract decision rather than a bug fix. npm test: 8071 pass, 0 fail. build:frontend run for the Classic change.
…nt (#443) The guard tests located orchestrateAndCollect(prompt to assert the state check ran before it. That call has not existed since the runner moved to the data-returning form — the string survives only inside a comment, so the tests were measuring where a comment sat. Both directions were wrong: deleting the stale comment would have failed a test about a guard that was still fine, and moving the comment above the guard would have passed one with the guard deleted. Anchored on orchestrateAndCollectData(prompt instead, and the now-misleading comment is gone. Verified by removing it and re-running: still 6/6, which the previous version could not have managed. npm test: 8071 pass, 0 fail.
…nd the secrets it guards (#449) Turning on remoteAccess or LAN mode binds 0.0.0.0. The endpoints that hand out credentials were never adjusted for that, so on such a host anyone reachable on the network could take the token and then read everything it protects. /api/auth/token relied on Sec-Fetch-Site, which is browser-enforced — the comment above it already noted that curl omits the header and passes through. That is a correct XSS defence and no defence at all against a network peer. It now refuses non-loopback callers outright; the header check stays for the browser case. /api/settings and /api/mcp had no guard on GET while PUT had one. Redaction only covered the Slack env-managed case, so a file-configured Telegram or Discord token came back verbatim, and MCP env/headers — which is where API keys live — were never masked at all. Both reads are authenticated now and secrets are replaced with a non-empty placeholder, so a UI asking "is a token set" still works without receiving the value. serializeSettingsForSave stripped Slack's env-owned keys but not the other two, so applyEnvOverrides put an env token on the live object and any later save — a port write, a target persist — copied it to disk, contradicting the documented promise that env values never enter settings.json. Conversation, memory, prompt and heartbeat reads are authenticated too. /api/health stays public so liveness checks keep working, and there is a test to keep it that way. Default installs (mode 'off', loopback bind) were never exposed; this matters for hosts that opted into remote access, where the opt-in itself implies the traffic is authenticated. npm test: 8078 pass, 0 fail.
finishWorker marks a replay for a Boss to collect, and processQueue skips any scope that holds one. A heartbeat has no Boss, so a single successful employee run left the default queue waiting on a handoff that could never arrive — every subsequent message sat behind it. Calling markWorkerReplayed right after finishWorker closes the loop the same way the pipeline and the orchestrate routes already do. The failure path needs no change: failWorker never arms a replay. npm test: 8079 pass, 0 fail.
/search, /interview, /skill:* and /steer forwarded only `origin` to
submitMessage. The message was recorded in the tab's chat session while the queue
and PABCD scope resolved to 'default', so the turn queued behind unrelated work
and ran outside its own lane — recorded in one place, executed in another.
resolveOrcScope already accepts chatSessionId; the gateway simply had nothing to
pass because the handler dropped it. sessionScopeMeta() reads the request's
AsyncLocalStorage scope and contributes {} when there is none, which is the
correct answer with multi-session off.
The regression test asserts that EVERY submitMessage call in those handlers
carries the session, so a new slash command that forgets it fails rather than
silently reintroducing the split. Invisible with one session — it only appears
once a second session is busy, which is why it survived this long.
npm test: 8082 pass, 0 fail.
The parameter is named all; it ran the stale-only statement, which spares anything updated in the last 24 hours. An operator reaches for this endpoint precisely when a scope is stuck mid-phase after a restart — and that state is minutes old, so it was skipped. The 24-hour grace exists so a restart can resume a live cycle, which is right for the boot path and wrong here: orc_state survives a restart but the worker registry is in memory, so a phase waiting on workers from the previous process is waiting on something that no longer exists. When a human sends all=true they have already decided the cycle is dead. Split into two statements with two meanings. Boot keeps the age filter; the endpoint gets an unconditional reset, and the response no longer calls the result "stale". PS-003 asserted the route calls resetAllStaleStates, pinning the defect in place — the same shape as #443. Updated to assert the intent instead. npm test: 8086 pass, 0 fail.
Interrupt is scope-local everywhere except where it mattered. killActiveAgent and purgeQueueOnStop both take a scopeKey; the queue insert used a global unshift, so interrupting session A promoted that turn past session B's message — which had been waiting first and had nothing to do with A. The replacement lands before the first item of the SAME scope, which preserves what interrupt means while leaving every other scope's relative order alone. A scope with nothing queued simply appends rather than cutting the line. Restart had the same bug in a different shape: loadPersistedQueue hoisted every priority:'head' item to the front of the whole queue, so a saved interrupt came back ahead of an unrelated session. Ordering is now restored per scope, with scopes keeping the order they were first seen in. The existing test asserted [latestA, b] — the defect, named in the test title as "at queue head". Updated to [b, latestA] with the reasoning recorded, the same correction #443 and #452 needed. The unshift at queue.ts:577 is untouched: that restores items after a failed setup and is correctly global. npm test: 8089 pass, 0 fail.
The trigger counter was global while the flush it triggers is per session, so the two disagreed. Nine turns in session A plus one in B fired a flush that then summarised B — and if B had fewer than four new rows the flush returned early, so the budget A had filled was spent on nothing and A went back to zero unsummarised. A quiet session could stay unsummarised indefinitely. This file already learned the lesson twice: the watermark and the deferred-flush set were made per-session in 073 §2.3 for the same reason. The counter was the piece left behind. countTurnForFlush resets the session that reached the threshold and reports it in one step, so a caller cannot fire without resetting or reset without firing. The map is bounded like the deferred set — keyed by session id on a long-lived host it would otherwise leak slowly. memoryFlushCounter stays: two other call sites use it as a turn-count estimate. npm test: 8093 pass, 0 fail.
The release gate caught the drift: #441 replaced the goal_cancel broadcast with goal_cancel_requested, because an AI-authored marker should ask rather than archive a goal outright, and server_api.md still listed the old event. Line counts refreshed with verify-counts.sh --fix, which is what the gate asks for — they are derived values, not authored ones.
mermaid는 프로덕션 의존성이면서 신뢰할 수 없는 다이어그램 텍스트를 sanitizeMermaidSvg() 이전 단계인 mm.render()에서 파싱한다. 11.16.1 미만은 prototype pollution 2건과 파서 DoS 2건에 노출된다. - GHSA-3rrr-jr9j-h3q3 (moderate) Architecture 다이어그램 prototype pollution - GHSA-2v8p-3f2j-5mp7 (moderate) XY Chart 무한 루프 DoS - GHSA-rhh3-jpg6-66xh (moderate) Radar 다이어그램 DoS - GHSA-c4c3-pg64-4m4v (low) 설정 API prototype pollution dompurify는 IN_PLACE 미사용으로 실사용 노출은 없으나 위생 차원에서 함께 올린다. undici/nanoid/shell-quote/postcss/esbuild 등 나머지 9건은 이번 범위에서 제외했다. 전면 npm audit fix는 락파일 89개 항목을 바꾸고 rolldown을 1.0.0-rc.15에서 1.2.5로 올려 회귀 표면이 지나치게 넓다. 분류 근거는 #456. Refs #456
* fix(skills): fold alias links into the skill they point at (#446) Six call sites each wrote readdirSync(withFileTypes).filter(d => d.isDirectory()) and relied on a platform detail rather than a rule: a POSIX symlink reports isDirectory() === false, so the legacy `dev -> jaw-dev` aliases were skipped by accident. The migration creates Windows links with symlinkSync(..., 'junction'), and a junction can present as a directory — there the same skill appears under both names, the prompt lists it twice, and the injected count doubles. dedupeSkillDirEntries resolves each entry and keeps the first per real path, so the alias disappears however the platform reports it. Directory names come back rather than paths, and whether an entry is a usable skill stays the caller's question: moving the SKILL.md check into the helper would quietly change what `skill list` shows. Applied to all five ESM sites — the prompt loader, `skill list`, both reset counters, and the soft-reset walk, where visiting one skill twice would have removed what the previous pass restored. The Electron bootstrap is CommonJS and cannot import lib/mcp, so it repeats the rule locally with a comment saying why. Measured before the change: suji 3457 reported 38 of 69 entries, 3458 reported 37 of 67, both with zero duplicates — the loader was already correct on macOS, which is why the reported "67 skills" was a directory count and not what the prompt saw. npm test: 8099 pass, 0 fail. * test: give orc_state writers their own CLI_JAW_HOME (#458) npm test failed intermittently with `actual: 'IDLE', expected: 'P'` — never alone, only in the full suite. tests/run.mts forks per file with isolation:'process', but every child inherits the ONE CLI_JAW_HOME the parent minted in tests/setup/test-home.ts. Separate processes, one jaw.db. PABCD state lives in that db's orc_state table and setState/resetState both address the same 'default' row, so with concurrency: true one file's resetState() in afterEach lands on top of another file's setState('P') between its write and its assertion. Which file loses depends on scheduling, which is why the failing file kept moving. Nine of the thirteen files that import state-machine mutate that row; only two had an isolated home. Give the other eight one. tests/setup/isolated-home.ts already exists for exactly this (recoverBgTasks' global sweep) and has to be the first import, since src/core/config.ts binds DB_PATH at module evaluation. THC-004 keeps it from regressing by counting files that import state-machine, mutate the shared row, and lack the isolated home. It strips comments AND string literals first: heartbeat-pabcd-guard and orchestrate-state-route-contract quote "resetState(scope)" as an assertion needle without ever calling it, and the naive version flagged both. npm test: 8100 pass / 0 fail, five consecutive runs. * feat(skills): add dev-write and dev-speech as active orchestration skills Two writing surfaces were folded into one owner. AGENTS.md said k-writing owned Korean promotional content "및 윤문", so a 윤문 request and a "write me a promo thread" request routed to the same generation pipeline — channel routing, pre-search, hook scoring — when 윤문 is the opposite job: the text already exists and its meaning is frozen. Split by verb, not topic. k-writing GENERATES platform content. jaw-dev-write REVISES Korean prose that exists, including the agent's own draft before it is sent: register consistency, translationese and AI idioms, mechanical structure, rhythm and endings — four passes, meaning untouched, stop if more than ~30% changes because that is rewriting. jaw-dev-speech COMPOSES an answer in the first place: audience calibration, conclusion-first order, marking verified against guessed, and the rules for handing work to another AI (selected context over a full dump, explicit constraints, verifiable completion criteria). Registered with category: orchestration, which is what actually activates a skill — skills-distribution.ts:205 and :278 plus skills-reset.ts:129 each fold that category into autoActivate. CODEX_ACTIVE holds one entry and is not the path. Auto-activated count goes 13 -> 15. The a1-system.md additions are two lines because only the trigger belongs inline: an agent that opens with a warm-up or blurs verified against guessed does it in its first sentence, before any skill read could correct it. PSC-006 budget 38,250 -> 38,750 with that reason recorded next to the assertion. npm test: 8105 pass / 0 fail. gate:all: 23/23. * fix(dashboard): make live instances discoverable and honor --home in status (#436) Two separate defects behind one symptom — the dashboard reporting nothing about instances that were plainly running. memory federation returned an empty list. instance-discovery built its rows exclusively from dashboard registry.json and consulted the scan only for home overrides, so a host whose dashboard home had no registry.json got instances: {} while two ports were answering. Promote online scan rows to list entries. Only online: a full scan walks 50 ports from 3457 and keeps offline/timeout rows, so unioning it wholesale would bury the two real instances under 48 dead ones. Registry entries stay even when offline — an operator declared them, and dropping them silently discards that intent. But an offline declared instance has no index.sqlite to open, and federation.ts:41 pushed a missing_db warning for each one on every search. InstanceMemoryRef now records why a ref is in the list, and federation warns only for scanned-live instances, where a missing index is real news. `jaw --home <path> status` probed 3457 no matter which instance the home belonged to. --home only sets CLI_JAW_HOME; the port option carried `default: process.env.PORT || DEFAULT_PORT`, which made "--port omitted" indistinguishable from "--port 3457". Drop the default and resolve in order: explicit flag, PORT, the home's jaw.pid.json, its settings.json, then 3457. Tests run the real command in a child process rather than asserting on source text, so the resolution order is proven rather than described. npm test: 8105 + 6 new, 0 fail. * test: observe the tree walk, not child.kill, in multi-session stop (#459) Both scoped-stop assertions failed on dev HEAD with actual: [], expected: ['A'], and had nothing to do with any working-tree change. The fake child recorded termination by replacing child.kill. killActiveAgent stopped calling it: spawn.ts:607 hands the child to OwnedProcess.terminate, which walks the process TREE by pid so a CLI's own children cannot outlive it. A fake with no pid short-circuits that path at process-kill.ts:147 — no tree walk, no child.kill — and the test read that absence as "nothing was killed". Production was fine; the observation point was stale. Give the fake a pid and inject terminateTree through the OwnedProcessOptions hook that already exists. ownProcess is memoized by child identity, so the owner has to be registered when the fake is built, before killActiveAgent asks for one. Only test:all runs this file — npm test collects root and tests/unit only — so it stayed green in the default gate while the integration suite was red. * test(browser): stop waiting for a networkidle the dashboard can never reach (#461) Fourteen notes smoke tests died identically at page.goto with a 30s timeout while the dashboard answered curl in under a millisecond. networkidle requires 500ms with zero network connections. The manager holds an SSE stream open to /api/events for as long as the page lives (public/manager/src/code/useCodeEvents.ts:41, goal-status/useGoalPabcdStatus.ts:24). One open stream means idle never arrives, so the wait was guaranteed to end in a timeout even on a perfectly healthy page. Not a regression — an unreachable condition that has been there since SSE landed, invisible because npm test collects root and tests/unit only. The wait was also redundant: every call site already waits for what it actually needs (.notes-tree, .cm-content, a specific API status). Switch to domcontentloaded and let those assertions do the waiting, which is what Playwright recommends anyway. Suite time for these files drops from ~7 minutes of pure timeout to ~18s. rich-authoring 6/6, tree-multi-delete + file-management + layout + refresh 7/7. * test(browser): size notes polling deadlines for a loaded machine (#461) One toolbar test failed under test:all and passed six times in a row alone. The polls that wait for a note to hit disk carried a 5s deadline chosen when these files ran by themselves. Under test:all ~60 other files share the CPU and the save round-trip — debounced write plus disk — absorbs that scheduling pressure directly, so the deadline expired before the content landed. Raise the save/API-status polls to 20s and the DOM-settle poll to 8s. This costs nothing on a healthy run: every loop returns on first match, so only a genuine failure pays the deadline. It is the same class as the networkidle fix in the parent commit — a wait sized for conditions the suite does not actually run in. npm run test:all: 8271 pass / 0 fail, three consecutive runs. * fix(slack): stop an aborted file send from reserving an upload slot (#464) sendSlackFile made one network call after the send was already cancelled. files.getUploadURLExternal reserves a slot on Slack's side and returns a file_id; nothing completes it, so every shutdown left one behind. sendSlackText has guarded this since #417 — the file path never got the same guard. The signal was passed down to slackApi, but that only helps once a request is in flight. And slackApi recognised an abort only in its catch block, so a cancellation was reported as whatever the response happened to parse to: "Slack API error: unknown_error" with status 200, which is both wrong and self-contradictory. #417's contract is that a cancellation must never be recorded as a vendor rejection. Guard at both entry points. Zero calls for an already-aborted send, and the same slack_send_aborted/499 answer the text path gives. Tests count actual fetch invocations rather than trusting the return shape. * fix(deps): bump mermaid to 11.17.1 and dompurify to 3.4.14 (#456) (#457) mermaid는 프로덕션 의존성이면서 신뢰할 수 없는 다이어그램 텍스트를 sanitizeMermaidSvg() 이전 단계인 mm.render()에서 파싱한다. 11.16.1 미만은 prototype pollution 2건과 파서 DoS 2건에 노출된다. - GHSA-3rrr-jr9j-h3q3 (moderate) Architecture 다이어그램 prototype pollution - GHSA-2v8p-3f2j-5mp7 (moderate) XY Chart 무한 루프 DoS - GHSA-rhh3-jpg6-66xh (moderate) Radar 다이어그램 DoS - GHSA-c4c3-pg64-4m4v (low) 설정 API prototype pollution dompurify는 IN_PLACE 미사용으로 실사용 노출은 없으나 위생 차원에서 함께 올린다. undici/nanoid/shell-quote/postcss/esbuild 등 나머지 9건은 이번 범위에서 제외했다. 전면 npm audit fix는 락파일 89개 항목을 바꾸고 rolldown을 1.0.0-rc.15에서 1.2.5로 올려 회귀 표면이 지나치게 넓다. 분류 근거는 #456. Refs #456 * ci(deps): fail on any advisory without a recorded decision (#460) The step named "Deps security check" compared three hardcoded version rules — ws and node-fetch twice — and printed PASS for the other ~700 packages. Not a broken gate; a gate that could only ever catch advisories someone had already read and transcribed by hand. That is why #456's mermaid prototype pollution cleared CI and sat in dev until a human happened to read an npm install warning. Invert the default. check-deps-audit.ts asks npm for everything it knows about and fails on anything absent from scripts/audit-allowlist.json. The allowlist IS the analysis: each entry names why the vulnerable path does not exist here — esbuild's read needs its dev server, which we never start; nanoid needs a negative size, and postcss passes a fixed one; undici needs a SOCKS5 proxy agent or a cache shared across principals, and nothing here configures either. Entries carry a review date. Past it the gate warns rather than fails, because a stale analysis is a prompt to re-check reachability, not a reason to block a build. npm audit needs the registry, so an offline run reports SKIP and exits 0 instead of failing a check it could not perform. Cherry-picked 296152b (mermaid 11.17.1, dompurify 3.4.14) first — the gate flags exactly those two otherwise, which is the fix demonstrating itself. check-deps-offline.ts stays: it reads the lockfile and needs no network, so it still guards those three in an offline build. npx tsx scripts/check-deps-audit.ts: 11 advisory packages, 11 allowlisted, 0 unexpected. * docs(structure): refresh slack api/slack-file line counts (#464) --------- Co-authored-by: Joonsuh Park <93533648+parkjs101@users.noreply.github.com>
#462) sendDiscordTextRest called schedulerFor(token) inline, so a test could only reach the cached per-token DiscordRestScheduler — the object that owns rate-limit timers and a live socket. The cancellation behaviour therefore had no assertable surface: either stub one layer too low (fetch, which says nothing about whether a QUEUED job saw the signal) or leave it uncovered. That matters here specifically because the scheduler is the only layer that applies a signal while a job is still waiting behind a rate limit. discord.js's channel.send() cannot do this at all, which is why the body sends were routed through the scheduler in the first place. An untested seam is where that property silently regresses. The parameter is optional and production passes nothing, so the default path is byte-identical. openDiscordDm already takes fetchImpl for the same purpose; this follows that convention one level up rather than inventing a new one. Tests ported from the closed #431 (superseded by the #417 implementation that landed via promote v2.17.16); these four were the part with no equivalent in main. Verification: npx tsc --noEmit exit 0 discord suites + outbound-lifecycle 131/131 pass Refs #417
sendTelegramMarkdown signalled shutdown cancellation by throwing
Error('telegram_send_aborted'), while sendSlackText and sendDiscordTextRest
returned ok:false for the same event. Telegram was the odd one out.
The cost was not stylistic. A caller's catch block receives BOTH a cancellation
and a real Telegram rejection, and could only separate them by matching the
Error message — which the abort often is not, because attemptSend rethrows the
underlying transport error ('socket destroyed') when the signal fires mid-flight.
So the one distinction the queue-notice path depends on (a cancelled answer must
never close a notice as 'answered') rested on a string that is not guaranteed
to be there.
Now returns TelegramSendResult: { ok: true } or { ok: false, aborted: true }.
Vendor failures still throw — collapsing those into the result would make a real
delivery fault look like a clean stop, which is the opposite mistake.
Three call sites had to grow an explicit branch, and each one was silently wrong
before:
- bot.ts telegramSendHandler returned { ok: true } for a send that never
happened, because the throw escaped past the return. Now 499 + ok:false.
- bot.ts message handler set ackOutcome='success' and relayed images for an
answer that was never delivered. Now settles as failure and skips the relay.
- bot.ts queued-reply path relied on the catch to close the notice as 'expired';
the branch now does it directly.
OSR-006 asserted the throwing contract, so it was rewritten to assert the
reported one rather than have its expectation string swapped. Tests otherwise
ported from the closed #431.
Verification:
npx tsc --noEmit exit 0
telegram/channel/delivery/ack suites 1104 pass, 0 fail
npm test 8031 pass, 25 fail
(all 25 pre-existing;
comm -13 vs baseline = 0 new)
Refs #417
main carried the squashed v2.17.17 promotion; every line of it is already in this branch (the promotion squashed these same dev commits). This records main as an ancestor so promote-to-main.sh can fast-forward, without touching the tree. git diff origin/main HEAD -- src/ lib/ bin/ shows only this cycle's additions.
…468) promote-to-main.sh squash-merges preview into main, which folds preview's commits into one new commit - so main stops being an ancestor of preview the instant the script succeeds. The guard at the top of that same script then demands exactly that ancestry on the next cycle. The script breaks its own precondition every release, and the gap was filled only by prose in AGENTS.md. That is not cosmetic. The manual recovery the gap forces produced the 2.17.13 incident: a hand-made merge whose message said 'take preview tree' produced a commit whose tree equalled main's, contributing nothing from preview, and cli-jaw@2.17.13 shipped without #418's durable queue-notice store. CI could not catch it - a tree that lost content still passes every check. Realign automatically once the publish is dispatched. realign_branch_onto_main lives in promotion-checkout.sh and builds the commit with commit-tree, so the published tree is an INPUT rather than a merge result and cannot come out of the wrong side no matter which branch is checked out. It still verifies the tree round-trips and that main really became an ancestor before pushing. A realignment failure warns instead of failing the release: the publish already went out by then, and the only cost is that the next promotion needs the manual recipe. AGENTS.md's 'git reset --hard origin/main' is replaced. It is safe only when dev has nothing main lacks, and dev normally does - following it during this cycle would have destroyed the mermaid/dompurify security bump along with everything else. The replacement keeps the tree and records the parent, with a mandatory git diff check before pushing, plus the direction warning the 2.17.13 merge ignored. Tests run against a real repository rather than the script's text, because the failure mode is a wrong merge DIRECTION and only a tree comparison sees it. PRA-003 reproduces the 2.17.13 shape directly: -s ours from main reports success and discards preview's work. npm run test:all: 8283 pass / 0 fail. gate:all 23/23.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (6)
Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe promotion scripts now realign ChangesPromotion branch realignment
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to This promotion change has no identified merge-blocking risk at the current head and is merge-ready after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant promote_to_main.sh
participant realign_branch_onto_main
participant origin
promote_to_main.sh->>origin: publish merged main
promote_to_main.sh->>realign_branch_onto_main: realign preview and dev
realign_branch_onto_main->>origin: fetch branch and main
realign_branch_onto_main->>origin: push preserved branch tree
promote_to_main.sh->>origin: verify main ancestry on preview
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 files. (3 skipped: 3 unsupported.) ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 421581526b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+241
to
+245
| realign_branch_onto_main preview "$MERGED_MAIN_SHA" \ | ||
| "chore: record the v$STABLE_VERSION promotion as an ancestor" \ | ||
| || echo "WARN: preview realignment failed; the next promotion will need a manual realign" >&2 | ||
| realign_branch_onto_main dev "$MERGED_MAIN_SHA" \ | ||
| "chore: record the v$STABLE_VERSION promotion as an ancestor" \ |
There was a problem hiding this comment.
In the documented happy path, origin/dev, origin/preview, and the local dev checkout initially share the preview SHA, but these calls synthesize and push two commits independently while never advancing the local checkout. The commits can become siblings because commit-tree uses the current timestamp; even if their hashes happen to match, the next documented git checkout dev && git fetch && git rebase origin/main ignores the remote realignment. Consequently, the non-force git push origin HEAD:preview in release-preview.sh is rejected as non-fast-forward on the next release. Reuse one realignment tip for both remote branches and ensure the local dev checkout can fast-forward to it.
AGENTS.md reference: AGENTS.md:L64-L74
Useful? React with 👍 / 👎.
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
2 participants
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.
Promotes certified preview 2.17.20-preview.20260825175400 at 10bbe07. Tests: https://github.com/lidge-jun/cli-jaw/actions/runs/32829085961
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores