fix(discord): auto-thread explicit @mention in reply messages - #68941
Open
LFDMcore wants to merge 48 commits into
Open
fix(discord): auto-thread explicit @mention in reply messages#68941LFDMcore wants to merge 48 commits into
LFDMcore wants to merge 48 commits into
Conversation
(cherry picked from commit 809bfb165166c7a1a33d32a1f3e461da0c0d0a92) (cherry picked from commit 019ef38cf0d248bea51fb429e6ed44f718eb5e81)
(cherry picked from commit e674aed1105eca1dbab4f2f4fb3376546b845ff1) (cherry picked from commit 3c0eda6768538ca1c643a2fafaaa36f7fc7586bd)
(cherry picked from commit feb6ac624afc84656aee942d3c275de94be738d7) (cherry picked from commit 92e083aca4947e8d3bad4332d5922f45eb4c77cf)
(cherry picked from commit 17130b5e1e451f6b2913ab102e2d97964db5de63) (cherry picked from commit 0af3be0c18b72bc9858ace75abcee6a57a81db2d)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> (cherry picked from commit 9636d88ca4e889c77c7275e31c987823a11a54db)
(cherry picked from commit e8acaf1f7c8091c696b1455980a4b598163937d8) (cherry picked from commit d53bface202eeeedf11ea4017dc80ebbcd88ff70)
CodexAuxiliaryClient and its Async wrappers stored the OpenAI SDK's
httpx.URL object as self.base_url instead of a str, which propagated
through _do_context_route_switch -> determine_api_mode where
base_url.rstrip('/') raised AttributeError (URL has no .rstrip).
Fixed in:
- CodexAuxiliaryClient.__init__ (L991): str(real_client.base_url)
- AsyncCodexAuxiliaryClient.__init__ (L1025): str(sync_wrapper.base_url)
- AsyncAnthropicAuxiliaryClient.__init__ (L1168): str(sync_wrapper.base_url)
- _do_context_route_switch (L140): str(getattr(client, 'base_url', '') or '')
Root cause: ~1,019 errors in 24h from determine_api_mode() failing on
httpx.URL objects passed as base_url.
(cherry picked from commit 03a59a86f2ca920f29109f9c7dc5f0a27c1f8e56)
(cherry picked from commit 85492d5d4123433371e05a3df19d0aae2c1f2722)
(cherry picked from commit 2f8a13d4965587a4535ded2e53eb68d37d300ec2)
(cherry picked from commit 66c133a7c2974548584367b1ac3471ce5806de8f)
…dict gating - Evidence gate: hard-reject → WARN mode (logs + comments, allows completion). DB backstop in complete_task checks result AND comments; auto-backfills from comments when evidence found there. Tool and CLI handlers are passthroughs to the authoritative DB backstop. The enforce flip (reject-on-empty) is a dated follow-up after weekly proof shows warn-count→0. - Idempotency key: key lookup now matches only non-terminal statuses (ready/todo/running/blocked), excluding done/archived. Regression test: create→complete→create-same-key yields a new task. - Verdict-aware parent gating: recompute_ready inline-block fix prevents SQLite nested-txn error when _block_spawn was called from within recompute_ready's write_txn. - Pre-spawn precondition hook: kanban_pre_spawn in VALID_HOOKS, _run_pre_spawn_checks dispatcher integration, _block_spawn helper. - Interim pre_tool_call plugin: kanban plugin vetoes kanban_complete calls with empty result when require_result_for_verify is enabled (worker-side stopgap until DB backstop lands). - Config: kanban.require_result_for_verify (default: false), kanban.pre_spawn_command, kanban.require_verdict_for_release. - Tests: 236 DB tests pass, 95 tool tests pass. New tests for evidence gate WARN + backfill, idempotency regression, verdict storage, parent gating. (cherry picked from commit cb56427b9755f4f7d458e3bb77a2c6aca6d02d14)
…ofile-scoped config Dispatched workers run with HERMES_HOME pointing to their profile directory (e.g. ~/.hermes/profiles/engineer/). A bare load_config() would read the profile-scoped config.yaml, which may lack kanban board-level settings like require_result_for_verify. This caused the evidence gate to silently skip for dispatched-worker completions while working correctly from the CLI. Root cause: four evidence-gate helpers (_kanban_require_result_for_verify, _require_result_for_verify, _kanban_require_result_for_verify_cli, _require_result_for_verify_plugin) all called load_config() which resolves to HERMES_HOME / config.yaml — the profile config. Fix: - Add _load_kanban_board_config() helper in kanban_db.py that temporarily resets HERMES_HOME to the default root so load_config() reads the shared board-level config. - Also invalidates the in-process config cache before loading, so a prior profile-scoped load can't poison the root-path cache entry (fixes the stale-cache edge case described in the task where the gateway and config update land in the same second). - Add public invalidate_config_cache() to hermes_cli/config.py for external cache invalidation (SIGHUP path, sidecar config writers). - Update all four evidence-gate call sites to use the new helper. Verified: profile-scoped HERMES_HOME with missing flag → gate reads from root config and correctly returns true. All 331 existing kanban tests pass. (cherry picked from commit 5c355d5a853d23362630811cf4ee1b12864f6942)
(cherry picked from commit c8153d2446fbcda0de1aedab89b19c1ba0593783)
…cked' during compression Preflight compression holds the WAL write lock for seconds while ~20 concurrent hermes processes convoy; the old 15-retry × ~85ms (~1.3s) budget exhausted and surfaced as 'database is locked', stalling live sessions. Raised to 40 retries × up-to-400ms (~8s, worst ~16s). Same jitter-retry design; early attempts stay fast for interactive paths. Diagnosed via the cron-load audit (high-freq crons cut 66% same day to reduce the concurrent-writer count feeding this). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit a2b2ab16ffff83880a911ab2c446ac1e5298085e)
(cherry picked from commit e453a12c312136f40b15cba15bad8f1679a1eb89)
…as business signals Reads sassy-factory/navigator/data/exit_code_contracts.json; when a job's exit code matches its contract (e.g. exit 1 = "data produced"), log success instead of treating it as a crash. LFDM cron-orchestration customization. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> (cherry picked from commit d8c76b1b32445712b92f19bfa12fdbcb53fc5def)
Records the 10 LFDM commits dropped in the 0.17->0.18 merge (Discord compaction-spam x2, Discord threading x2, project_search x2, continuity CI gate x4), each with hash, files, why-dropped, 0.18 equivalent, exact cherry-pick recovery command, and revisit criteria. All remain recoverable on lfdm-main, the pre-v018 bundle, and the fork. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The 0.17->0.18 merge grafted our cron model/provider routing onto v0.18's
rewritten run_job and inverted model precedence: guarding config.yaml model
resolution on 'and not env_model' + 'model or _model_cfg' made HERMES_MODEL
win over config.yaml 'model:'. That broke v0.18's config.yaml ${VAR}
expansion AND fed the wrong current-model into the spend-safety model-drift
guard (5 upstream tests, green on pristine v0.18, regressed on our branch).
Restore v0.18 precedence (config.yaml model overrides the HERMES_MODEL env
default) and apply our cron.model as a cron-scoped override applied last, so
it still routes scheduled jobs to their own model without clobbering the
config/env resolution order or an explicit per-job model pin.
Our codex-routing commit also extended _resolve_turn_agent_config with a
platform= kwarg; update the two fixed-arity monkeypatch stubs to absorb it
(**_kw) so they tolerate the new signature.
Verified: cron failure set now byte-identical to pristine v0.18 (zero
incremental regressions); the 5 regressed tests pass in isolation.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Durable, in-repo notes so the next upgrade doesn't lose context and repeat the same mistakes. Headline lesson from 0.17->0.18: a no_agent cron job is defined by its 'script' field, not 'command' — do not call one 'malformed' because command/action/kind are null (that's correct for script jobs). Also captures the cron-schema-tightening trap (hand-authored records missing bookkeeping fields crash the tick; one bad job aborts ALL cron), the differential-testing method, and the precedence-inversion hotspot. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…t abort all cron The 0.18 cutover surfaced a single-point-of-failure: the due-scan loop in _get_due_jobs_locked let an exception in ONE job (e.g. a hand-authored record with a null/absent id hitting the recovery branch's eager-eval job['id'], or an unparseable next_run_at) propagate and abort the entire pass — every one of the 72 jobs stopped firing, not just the offender. Wrap each job's evaluation in try/except: a malformed or failing job is now skipped-and-logged (error + exc_info) and the batch continues, so the scheduler survives bad data by design. Pure resilience change — no logic moved. Adds a regression test (malformed sibling must not take down healthy jobs); verified it fails on the pre-fix code (ValueError aborts the scan) and passes after. Full cron suite: 613 passed / 0 failed (the 24 previously-failing 'environmental' tests were this same crash class and now pass). Worth reporting upstream — the fragility is in upstream's tick loop. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…jobs to prod test_jobs_changed_notify's temp_home fixture set HERMES_HOME via env var, but cron.jobs.JOBS_FILE/CRON_DIR/OUTPUT_DIR are import-frozen, so cronjob(create) wrote to the real ~/.hermes/cron/jobs.json. The create-only test never cleaned up, leaking a 'w'/'echo hi'/every-5m job into the live cron store on every run (14 accumulated + 2 left enabled, firing every 5min). Redirect the module path constants like tmp_cron_dir does. Also documents the operational lesson in the upgrade playbook (run suites against a throwaway HERMES_HOME). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
alt-glitch
added
platform/discord
Collaborator
Contributor
|
Thanks for identifying the reply-routing regression. The focused Discord change is supported by current main: Problems
Suggested changes
Automated hermes-sweeper review. |
teknium1
added
the
sweeper:blast-moderate
Contributor
SummaryThree PRs address or reference this Discord auto-threading complex: #6337 narrowly strips mention syntax from thread names; #11629 combines that fix with reply/free-response routing and thread-creation fallback handling; #68941 narrows #11629's reply rule so explicitly mentioned replies can create threads, but bundles extensive unrelated changes. Related pull requests
Duplicates#6337 and the mention-name-stripping portion of #11629 implement substantially the same #6336 fix; #6337 is the closed duplicate, while #11629 is the merged implementation. #68941 is related to #11629 but is not a duplicate because it changes explicit-mention reply semantics. Suggested consolidationKeep #68941 open with a salvage path: isolate commit f961961's narrow Discord change, place focused coverage in tests/gateway/test_discord_free_response.py, and verify that an explicit mention in a non-free-response reply routes the emitted event to the created thread while a no-mention reply remains inline. Leave merged #11629 as the reference implementation for #6336, #8487, and #9399; #6337 is already closed as its #6336 duplicate. Cross-PR triage: Reviewed 3 pull requests and 3 issues in this complex. Each diff was read against this issue; Assessment working set: 350 kB of PR diffs, 11 kB of issue/PR text, 3 kB of discussion (5 comments), 7 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
alt-glitch
added
comp/agent
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
4 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.
Summary
Narrows the over-broad carve-out added by #11629 (fixing #9399).
#9399's original problem: in
free_response_channels, a quote-reply to a bot message (e.g. a cron post) should continue the conversation inline, not spin off a thread. #11629 fixed this with a global rule —if message.type == reply: skip auto-threading.Regression this causes: in thread-first channels (not free-response), a user who quote-replies with an explicit @mention is issuing a fresh task trigger — the quote just provides context. The global reply carve-out suppresses threading for this case too, so the bot's response (often multiple thousand chars) lands inline in the parent channel and floods it. The auto-threading routing target is silently bypassed.
Fix
Only skip auto-threading for replies that have no explicit @mention. An explicit @mention in a reply is still a fresh trigger and should thread normally.
auto_thread = os.getenv("DISCORD_AUTO_THREAD", "true").lower() in {"true", "1", "yes"} is_reply_message = getattr(message, "type", None) == discord.MessageType.reply - if auto_thread and not skip_thread and not is_voice_linked_channel and not is_reply_message: + # #9399: a quote-reply without an explicit @mention continues inline. + # An explicit @mention in a reply is a fresh task trigger: thread it. + skip_for_reply = is_reply_message and not mention_prefix + if auto_thread and not skip_thread and not is_voice_linked_channel and not skip_for_reply: thread = await self._auto_create_thread(message)mention_prefixis already computed earlier in_handle_messagevia_self_is_explicitly_mentioned(message)(L5748 in current main), so no new state.Context preservation
Quote context is preserved either way —
gateway/run.py(~L10063) injectsreply_to_textinto the prompt regardless of whether the message ends up in a thread:So threading a quote-reply loses no context.
Why not drop the carve-out entirely?
That would re-break #9399 (free-response continuation). This fix keeps #9399's protection (reply without mention → inline) and only unblocks the explicit-mention case (reply with mention → thread). Related: #25310, #26058 — other symptoms of auto-thread / free-response / require_mention being treated as coupled rather than orthogonal. This PR is a step toward treating them as independent.
Tests
Two regression tests in
tests/e2e/test_discord_adapter.py::TestAutoThreadingReplyMention:test_reply_with_explicit_mention_threads— reply + @mention → thread createdtest_reply_without_mention_does_not_thread— reply without @mention in free-response channel → no thread (Discord: bot silently ignores quote-replies in free_response_channels #9399 preserved)Full file: 9/9 pass.
test_discord_double_dispatch.py: 12/12 pass.