[fork-ship] Persist sender device attribution (desktop half) - #261
Merged
Conversation
…tored desktop thread thread.tsx was split upstream into thread/*; port the senderDevice label into thread/user-message.tsx (preserving upstream's clampActive body wrapper). Python side (hermes_constants get_device_name, hermes_state sender_device column/INSERTs) hand-resolved onto refactored SCHEMA_SQL + _insert_message_rows. Relax one conversation-replay assertion for upstream's added replay timestamp (sender attribution stays local; no behavior change). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ice-attribution-20260611 Resolve the conflict in hermes_state.py's two `INSERT INTO messages` statements (append_message and replace_messages). Both sides added a new column to the same messages row: - this PR added `sender_device` (before `observed`); - upstream added `active` (after `observed`). The column-list line conflicted, but git auto-merged the surrounding `VALUES (...)` placeholder line and the values tuple non-atomically: the tuple already contained BOTH new values (sender_device + the trailing active `1`), yet the placeholder line kept only 17 `?` — a latent 18-columns / 17-placeholders / 18-values mismatch that would fail at runtime. Resolution (both INSERTs, identical): union the column list to `... platform_message_id, sender_device, observed, active` and bump the placeholder list to 18 `?`, so columns (18) == placeholders (18) == tuple values (18), in matching order. The merged CREATE TABLE messages already carries both `sender_device TEXT` and `active INTEGER NOT NULL DEFAULT 1` (schema auto-merged cleanly). Verified: no markers; py_compile clean; module imports; a static pass confirms both resolved INSERTs are 18-cols/18-placeholders; 427 tests pass (tests/test_hermes_state.py incl. test_append_message_sender_device_override_and_failure and test_replace_messages_preserves_sender_device, tests/test_hermes_constants.py, tests/agent/test_codex_app_server_persist.py, tests/gateway/test_undo_rewind_session.py — exercising both sender_device and the active column round-trip). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
# Conflicts: # hermes_state.py
OmarB97
added a commit
that referenced
this pull request
Jul 11, 2026
…ne -> green main (#262) Head-of-main CI is red on `Check uv.lock / uv lock --check`, all 8 `Python tests / Run tests slice N/8`, and `Python tests / e2e`. Root cause is a uv.lock <-> pyproject.toml inconsistency (not a test-logic failure): every slice and e2e install via `uv sync --locked`, which hard-fails on a stale lockfile. Two stale entries, both pre-dating the five fork-ship merges (#257-#261, none of which touched pyproject.toml/uv.lock): 1. pytest-timeout==2.4.0 - the fork's pytest addopts use `--timeout=30 --timeout-method=thread` (needs the plugin); PR #248 declared it in the `dev` extra, but the v0.18.2 upstream merge (ba510aa) dropped that declaration while keeping the addopts and the lock entry. 2. A vestigial `{ name = "pillow", marker = "extra == 'vision'" }` line in uv.lock, left over from Pillow's promotion to a core dependency (`vision = []`). Fix: re-declare pytest-timeout==2.4.0 in the dev extra (reverts the merge drop; the existing lock entry is already correct) and regenerate uv.lock to drop the stale pillow vision-marker line. Net: +1 pyproject dep, -1 uv.lock line. Verified locally: `uv lock --check` clean; `uv sync --locked --python 3.11 --extra dev` installs; pytest_timeout plugin registered and a trivial test passes under the repo addopts (1 passed) - i.e. `--timeout` is accepted instead of aborting on an unknown arg. Co-authored-by: Omar Baradei <omar@kostudios.io> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
OmarB97
added a commit
that referenced
this pull request
Jul 20, 2026
…tored desktop thread (#261) thread.tsx was split upstream into thread/*; port the senderDevice label into thread/user-message.tsx (preserving upstream's clampActive body wrapper). Python side (hermes_constants get_device_name, hermes_state sender_device column/INSERTs) hand-resolved onto refactored SCHEMA_SQL + _insert_message_rows. Relax one conversation-replay assertion for upstream's added replay timestamp (sender attribution stays local; no behavior change). Co-authored-by: Omar Baradei <omar@kostudios.io> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
OmarB97
added a commit
that referenced
this pull request
Jul 20, 2026
…ne -> green main (#262) Head-of-main CI is red on `Check uv.lock / uv lock --check`, all 8 `Python tests / Run tests slice N/8`, and `Python tests / e2e`. Root cause is a uv.lock <-> pyproject.toml inconsistency (not a test-logic failure): every slice and e2e install via `uv sync --locked`, which hard-fails on a stale lockfile. Two stale entries, both pre-dating the five fork-ship merges (#257-#261, none of which touched pyproject.toml/uv.lock): 1. pytest-timeout==2.4.0 - the fork's pytest addopts use `--timeout=30 --timeout-method=thread` (needs the plugin); PR #248 declared it in the `dev` extra, but the v0.18.2 upstream merge (ba510aa) dropped that declaration while keeping the addopts and the lock entry. 2. A vestigial `{ name = "pillow", marker = "extra == 'vision'" }` line in uv.lock, left over from Pillow's promotion to a core dependency (`vision = []`). Fix: re-declare pytest-timeout==2.4.0 in the dev extra (reverts the merge drop; the existing lock entry is already correct) and regenerate uv.lock to drop the stale pillow vision-marker line. Net: +1 pyproject dep, -1 uv.lock line. Verified locally: `uv lock --check` clean; `uv sync --locked --python 3.11 --extra dev` installs; pytest_timeout plugin registered and a trivial test passes under the repo addopts (1 passed) - i.e. `--timeout` is accepted instead of aborting on an unknown arg. Co-authored-by: Omar Baradei <omar@kostudios.io> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
OmarB97
added a commit
that referenced
this pull request
Aug 2, 2026
… NameErrors) (#287) `Python lints / ruff enforcement (blocking)` is red on main: undefined-name: Undefined name `json` --> hermes_constants.py:1247:20 undefined-name: Undefined name `json` --> hermes_constants.py:1313:16 Found 2 errors. `hermes_constants.py` imports os, shutil, socket, stat, sys, sysconfig, contextvars and pathlib — but not json, which it calls in two resolvers. Both landed in e210fee (fork PR #261, 2026-07-10). This never crashed anyone because both call sites sit inside blanket `except Exception:` handlers, so the NameError is swallowed: * `_resolve_meshboard_device_name` — `except Exception: continue`, so every mesh root is skipped and the MeshBoard device label is never found. * `_resolve_tailscale_device_name` — `except Exception: pass` then `return None`, so the Tailscale hostname is never resolved. Net effect: `get_device_name()` silently skips both preferred sources and falls through to the bare socket hostname. Two features that quietly did nothing, with no traceback to notice. F821 (turned on in #280) is what finally surfaced it. Fix: `import json` at module scope, alongside the other stdlib imports. The module docstring's "no dependencies" means no Hermes-internal imports (circular-import safety), not no stdlib — os/socket/shutil are already there. Tests: the existing `TestGetDeviceName` monkeypatches BOTH resolvers away to isolate the precedence logic, so neither body was ever executed under test — that is how this shipped. Added `TestDeviceNameResolvers`, which exercises the real functions. Verified it actually catches the bug: with the import reverted, both new tests fail (`assert None == 'ko-mac'`); with it, the file is 132 passed. The meshboard test pins `Path.home()` to an empty tmp dir, because the resolver probes `~/Workspaces/.mesh` before `MESHBOARD_RUNTIME` — without that, a real mesh checkout on a developer's machine answers first and the test passes for the wrong reason (caught locally: it returned this machine's actual device label). Verified on macOS 15 (darwin 25.6.0, CPython 3.11): * `pytest tests/test_hermes_constants.py` — 132 passed * `ruff check .` — clean under both the pinned 0.15.10 and the 0.16.1 that CI's `uv tool install ruff` currently resolves to Co-authored-by: Omar Baradei <omar@kostudios.io> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Mirrors the desktop-TS half of upstream NousResearch#44078 onto the fork; the python half was already on fork main via a separate commit (hermes_state.py:766/1574/3756). The one real conflict (hermes_state.py's
append_messagesignature/docstring) was resolved by keeping fork main's landed version, plus a content-verified union fix: fork main's variant never threadedsender_devicethrough_insert_message_rows(used byreplace_messages/archive_and_compactfor /retry, /undo, /compress, and compaction), which would have silently dropped device attribution on any transcript rewrite — this branch carries upstream's fix for that gap (confirmed by upstream's own regression test,test_replace_messages_preserves_sender_device, which now passes). All content verified against upstream head 4cb47d2.