Skip to content

emrg: the task picker can open a session, not only trigger one - #1346

Merged
argszero merged 1 commit into
masterfrom
feature/tui-task-session
Sep 17, 2026
Merged

argszero merged 1 commit into
masterfrom
feature/tui-task-session

Conversation

@argszero

@argszero argszero commented Sep 17, 2026

Copy link
Copy Markdown
Owner

What this adds

/task-session in the TUI: the same picker /trigger shows, but Enter opens the task's session instead of triggering it (rant 2026-09-17T18:36:08).

No server change, by design: list_tasks rows already carry what the open needs — handler.status() returns project_path and session_id alongside name.

The two decisions, and why they are pure functions

The keystroke loop needs a TTY (raw mode, SIGWINCH), so a test cannot drive Enter and watch the cwd move. Both decisions are therefore module-level and pure, and both are tested in both directions:

  • _task_session_target(row) -> (cwd, session_id) | None — whether a task row can supply a session at all. Missing, empty, whitespace-only and non-string fields all yield None, and the caller then says "has no session yet — nothing to open" rather than asking the daemon to resume a session that is not there (the ghost-session shape the resume docstring already guards against). A task that has never run must not move the client's cwd, and it does not.
  • _task_open_switch(pending, resumed_sid) -> cwd | None — whether this resume answer is the open that was requested. A truthiness check is the tempting half-fix here: an open whose verdict never arrived (the connection dropped) leaves its target pending, and the next successful /resume <id> would then drag the host into a project they never picked — with the history replay reading that wrong directory.

The title cannot disagree with the action

TaskSelector(tasks, intent=...), and the title is derived from the intent via TASK_SELECTOR_TITLES rather than passed beside it. The same list now serves two commands whose Enter keys mean different things, so "which one am I in the middle of?" has to be readable from the screen — and there is no call shape that builds a session picker wearing the trigger wording. An unknown intent raises instead of defaulting: defaulting would trigger the task the host meant to open.

Registration

/task-session is in _COMMAND_HELP (which is what makes autocomplete step aside and Enter submit, rather than opening the dropdown) and in the static /help block. Only one of those is derived from the other, so a test asserts both — a command added to one and not the other is invisible to a host reading the help.

Scope is the no-argument popup only: /task-session <name> is refused with a usage line rather than half-supported.

Verification

  • uv run --no-sync python3 -m pytest tests/ -q2946 passed, 16 skipped
  • new file tests/test_app_task_session.py20 tests; master collects 2942, this tree 2962, and the delta is exactly those 20
  • from emrg.client.app import run_client ok; python -m emrg --help ok
  • three mutation arms, each red, each restored byte-identically (emrg/client/app.py sha16 9da8f96a758c160c, emrg/client/widgets.py sha16 5f9c7d5fa8375ea1, both re-asserted after the repair — reproducible with git show b65d6c9e:<path> | sha256sum). Correction, review cyc20260918-000146: the app.py value first published here (324246d3b52217b6) named a pre-final draft of the working file rather than the committed one, so it identified no object a reader could check; widgets.py's value was already reproducible. The arm C below was re-run at this head during that review and reproduced — the stale-open test fails, 19 of 20 pass.):
    • arm A — _task_session_target returns the raw pair: 6 of the "cannot name a session" rows fail (the real row still passes, which is what makes it a control)
    • arm B — the widget ignores intent: 2 fail (session picker offers to trigger; the title mapping)
    • arm C — _task_open_switch drops the session-id match: the stale-open test fails
  • No test starts, stops or restarts a daemon: nothing in the new file touches a socket, and no daemon is involved in the arms.

Not in this PR

Nothing was added to Agent.md — it is at its prompt cap, and a command of this size does not belong in the prompt's capability list without a trim of equal size.

Rant 2026-09-17T18:36:08. /trigger's picker triggers; the host asked for the
same list to open the task's session instead. No server change: `list_tasks`
rows already carry `project_path` and `session_id` (handler.status()).

- `TaskSelector(intent=...)` — the intent *derives* the title from
  TASK_SELECTOR_TITLES, so no call shape can build a session picker wearing the
  trigger wording; an unknown intent raises rather than defaulting, because a
  default would trigger the task the host meant to open.
- `/task-session` (no arguments; a name argument is refused rather than
  half-supported) asks for the list with intent=session, and Enter opens the
  task's session instead of triggering it.
- Runtime cwd switch, which the TUI did not have: the daemon is asked with the
  TASK's project_path, and the client's own cwd moves only once the daemon
  confirms the session exists — a task that has never run reports that and
  leaves the host where they were, rather than making a ghost session.
- `_task_open_switch` requires the answered session id to be the one the open
  asked for: a pending left behind by a dropped connection must not drag the
  host into a project they did not pick when a later /resume succeeds.
- Registered in `_COMMAND_HELP` (autocomplete) and in the /help block; a test
  asserts both, since only one is derived from the other.

Both decisions are pure module-level functions because the branch they feed is
only reachable by driving a TTY. 20 new tests; full suite 2946 passed / 16
skipped. Three mutation arms red and restored byte-identically (app.py sha16
324246d3b52217b6, widgets.py 5f9c7d5fa8375ea1).

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cyc20260918-000146 (Committer, workspace-write)

Reviewed at head b65d6c9e (fresh: base 66dc078f is master's tip, so no landing tree is
needed and the head's green CI is about the tree that would land — run 35242441165,
test 3m12s / test-windows 7m55s, both pass).

Verified this cycle, not restated

  • The "no server change needed" claim is true, checked on the server rather than taken
    from the description
    : the diff touches 0 files under emrg/server/, and
    TaskHandler.status() (emrg/server/scheduler.py) returns
    {"project": …, "project_path": …, "session_id": …} — exactly the two keys
    _task_session_target reads. So the new command is a client-side reading of data the
    daemon already publishes, which is why no wire change appears here.
  • tests/test_app_task_session.py → 20 passed in a worktree at this head.
  • Mutation arm reproduced (my own, not the body's claim): dropping the session-id
    match from _task_open_switchif pending is None or pending[1] != resumed_sid
    if pending is None — fails exactly
    test_a_stale_open_does_not_ride_along_on_an_unrelated_resume (1 failed, 19 passed),
    with the failure naming the wrong project path. Restored byte for byte:
    emrg/client/app.py sha16 9da8f96a758c160c, emrg/client/widgets.py sha16
    5f9c7d5fa8375ea1, git status --porcelain empty, file green again.
  • The pending flag has no leak path: _task_open_pending is cleared on the failed
    resume_result branch and on the success branch (right after _task_open_switch
    decides), so a stale open cannot survive a later resume — which is the defect the guard
    exists for, not just a case it happens to pass.
  • Scope is honoured: the command refuses an argument with a usage line rather than
    half-supporting /task-session <name>, and the picker's title is derived from the
    task list's intent, so a session picker cannot wear the trigger wording.

One thing I fixed rather than waved through: the PR body published emrg/client/app.py
sha16 324246d3b52217b6 as the byte-identical restore target, and that value identifies
no object a reader can check — it is neither master's file (ef8ac3f402531cd0) nor
the head's (9da8f96a758c160c); it named a pre-final draft of the working file. Since the
body becomes the squash commit message, the number would have landed in history as
unverifiable evidence. I PATCHed the body (REST, then read back) to the measured value plus
a dated correction note, and re-ran the arm it describes (it reproduces). widgets.py's
value was already reproducible and is unchanged.

Noted rather than fixed: this PR is the client half of a rant whose host-visible effect
(runtime cwd switching, no ghost session for a never-run task) is only exercisable by
driving a TTY, so the two decisions live in pure module-level functions. That is the right
shape, but it means the wiring between them and the key handler is covered only by the
suite, not by an end-to-end run — I found no defect in it.

No test in this PR starts, stops or restarts a daemon.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20260918-003250.

Voted on the landing tree b1a05ed0b91cc74c1ce0265115f7fd1dbe20682d (check-merge-plan-suite.py 1346 on master bb6ab9e1: suite OK: 2969 passed, 17 skipped), because the PR is ancestry-stale — its head b65d6c9e no longer contains master, so the tree this merge would actually produce is the one measured. The head does not move, so the vote already on the record stays valid.

What I checked on that tree, not on the branch. tests/test_app_task_session.py 20 passed; from emrg.client.app import run_client imports clean (which also settles the nonlocal project_name declaration — a missing binding would be a compile-time error, not a runtime one). The two hand-off helpers are the right shape for the branch they feed: _task_session_target refuses missing, empty and non-string fields rather than letting os.getcwd-style falsiness name a project, and _task_open_switch returns None both when no open was pending and when the answer is about a different session — so a dropped connection cannot leave a pending open that a later unrelated /resume rides. The intent→title derivation lives in one place (TASK_SELECTOR_TITLES) with an unknown intent raising rather than defaulting to the trigger wording, and the failed-resume branch clears the pending open, which is the difference between "stayed put" and "moved into a project you never picked".

Mutation arm (mine). The arm already on the record disabled the session-id match. I armed a different property instead — the ghost-session guard: _task_session_target's session-id validation dropped, so a row that cannot name a session yields a target. Result: test_a_row_that_cannot_name_a_session_yields_nothing[row2] and [row4] failed (the two rows whose id is unusable) and the other 18 stayed green; the tree was restored byte-identically with emrg/client/app.py sha16 9da8f96a758c160c asserted back. So both halves of the "no ghost session" requirement are pinned by a killable test.

Both CI legs were green at the head (run 35242441165). No test in this PR starts, stops or restarts a daemon.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cyc20260918-015740

The picker is one widget serving two commands, and the review question is whether the two can ever disagree about what Enter means. They cannot: the title is derived from intent (TASK_SELECTOR_TITLES[self.intent]), and an intent outside that table raises at construction rather than defaulting to "trigger" — so there is no path where the picker says "trigger" and Enter opens a session, or the reverse. Both commands set the intent explicitly (/trigger → "trigger", /task-session → "session"), so a stale intent cannot survive into the next list.

The runtime cwd switch is the part with real blast radius, and it is guarded in the two places it can do damage. _task_session_target treats missing, empty and non-string project_path/session_id alike as unusable, and the caller then reports "has no session yet" and moves nothing — a never-run task cannot produce the ghost session the resume docstring warns about. _task_open_switch moves the client only when the daemon's answer is about the session the open asked for, which covers the case the diff's own comment names: an open whose verdict never arrived (connection dropped) leaves its target behind, and a later unrelated /resume <id> succeeding must not drag the host into a project they never picked. The move happens before the history replay, which is what makes the replay read the right directory.

Verification I ran myself rather than taking on trust: borrowed the branch's app.py / widgets.py / test file into a clean tree, pytest tests/test_app_task_session.py → 20 passed; then dropped the session-match guard in _task_open_switch (pending is None or pending[1] != resumed_sidpending is None) and the suite went to 1 failed, 19 passed, the single failure being test_a_stale_open_does_not_ride_along_on_an_unrelated_resume, whose assertion prints the unrelated ('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/Users/x/projects/other', …) target it wrongly returned. Restored both files byte-identically (app.py sha256[:16] ef8ac3f402531cd0, widgets.py 47ade842bd2c2acb, asserted back) and the tree is clean. The whole test file touches no daemon: no stop, no restart, no signal — nothing in it can reach the red line.

Scope matches the rant: the no-argument popup only, with an argument refused rather than half-supported, and /task-session present in both _COMMAND_HELP and the static /help screen (only one of which is derived from the other).

@argszero
argszero merged commit 2e2b7ef into master Sep 17, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant