Skip to content

mng/minds-better#861

Open
joshalbrecht wants to merge 95 commits intojosh/llm_agentfrom
mng/minds-better
Open

mng/minds-better#861
joshalbrecht wants to merge 95 commits intojosh/llm_agentfrom
mng/minds-better

Conversation

@joshalbrecht
Copy link
Contributor

Automated PR created by Claude Code session.

evgunter and others added 30 commits March 9, 2026 17:46
…amic choices

Extends the tab completion system to support:
- Host name completion for create --host/--target-host (from discovery stream)
- Host name positional completion for the events command
- Plugin name completion for --plugin/--enable-plugin/--disable-plugin options
- Plugin name positional completion for plugin enable/disable subcommands
- Config key positional completion for config get/set/unset subcommands
- Dynamic option choices for create --agent-type, --template, --in, --new-host
  and list --provider (injected from runtime context at cache write time)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Problem: Three test functions in completion_writer_test.py were named
testflatten_dict_keys_* instead of test_flatten_dict_keys_*, violating
the codebase naming convention for test functions.
Fix: Added the missing underscore separator between "test" and "flatten"
in all three function names.
Problem: _get_positional_candidates called _read_agent_names() and
_read_host_names() independently, each parsing the full discovery
events file. For commands like "events" that appear in both
agent_name_arguments and host_name_arguments, the file was read twice.
Fix: Added _read_discovery_names() that returns both agent and host
names from a single call, and refactored _get_positional_candidates
to use it when either (or both) are needed.
…instead

Problem: _read_agent_names() in complete.py had no remaining call sites
after the refactoring in 96e123c that replaced it with
_read_discovery_names(). The function was dead production code, only
exercised by two tests.
Fix: Removed _read_agent_names() from complete.py and replaced its
tests with equivalent tests for _read_discovery_names(), which is the
function that now handles agent name reading in the positional
completion path.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Problem: write_cli_completions_cache contained 4 near-identical loops that
each filtered a frozenset of dotted keys (e.g. "create.--host") by checking
whether the top-level command was in the set of canonical command names.
This repetition made the already-long function harder to scan.

Fix: Extracted the common pattern into _filter_keys_by_registered_commands,
which takes a frozenset of dotted keys and a set of canonical names and
returns the matching subset. All 5 occurrences (git branches, host names,
host name arguments via _AGENT_NAME_SUBCOMMANDS, plugin names, config keys)
now use this helper.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…letion

Moves the choice_key elif chain (predefined choices, git branches, host
names, plugin names) out of _get_completions into a dedicated helper,
mirroring the existing _get_positional_candidates pattern. This makes
_get_completions read as a clean dispatcher.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Moves command/subcommand resolution, prev_word detection, and cache
destructuring into _parse_completion_context(), which returns a
NamedTuple. This makes _get_completions a pure dispatch function.

Uses NamedTuple instead of FrozenModel to keep complete.py free of
pydantic imports (it runs on every TAB keypress). Bumps the NamedTuple
ratchet from 0 to 1.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…te in background

Moves the dynamic completion builder from list.py into completion_writer.py
where it belongs (next to the cache writer that consumes it). The cache
write now runs as a background thread on mng_ctx.concurrency_group so it
does not block list output. Removes the ctx.meta bridging that was needed
when the builder and writer lived in different scopes.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The dynamic_completions parameter on write_cli_completions_cache was
test-only infrastructure leaking into production code. Replace it with
a proper integration test that passes temp_mng_ctx and asserts on the
real dynamic values it produces.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Introduces completion_types.py as a lightweight shared module imported by
both the cache writer and reader. Contains:
- CompletionCacheData: typed schema for the JSON cache (replaces raw dicts
  and the _make_cache_data test helper)
- COMPLETION_CACHE_FILENAME: single source of truth (was duplicated)
- get_completion_cache_dir(): single implementation (was duplicated with
  a comment saying \"mirrors completion_writer.py\")

Bumps NamedTuple ratchet from 1 to 2.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…stuff

# Conflicts:
#	libs/mng/imbue/mng/config/completion_writer_test.py
…ration test

Add positional_nargs_by_command to the completion cache, extracted from
click.Argument.nargs at cache write time. The lightweight completer
counts positional words already typed and stops offering candidates
when the limit is reached (e.g. "mng config set KEY VALUE <TAB>" no
longer suggests more config keys).

Also add test_every_option_is_classified, a reverse integration test
that walks the real CLI tree and verifies every --long option appears
in options_by_command in the cache, catching option renames and additions
that would otherwise go undetected.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace flat command->source lists (agent_name_arguments, host_name_arguments,
plugin_name_arguments, config_key_arguments) with a single positional_completions
field that maps command_key -> list of source identifiers per position. This
fixes the bug where `mng config set KEY <TAB>` would still suggest config keys
for the VALUE position, and similarly for rename, pull, push, events, etc.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Introspect MngConfig's pydantic model_fields recursively at cache write
time to build a map of config_key -> list[valid_values] for fields with
constrained types (enums, bools). Store this in the completion cache as
config_value_choices. At completion time, when the user is at the VALUE
position of config set, look up the KEY they already typed and offer the
valid values.

Changes:
- completion_cache.py: add config_value_choices field to CompletionCacheData
- completion_writer.py: add _unwrap_optional and _extract_config_value_choices
  to introspect MngConfig fields; return typed _DynamicCompletions NamedTuple
  from _build_dynamic_completions; update config.set spec to use
  config_value_for_key source
- complete.py: add first_positional_word to _CompletionContext; add
  _find_first_positional_word helper; handle config_value_for_key source
  in _resolve_sources
- Tests for all new behavior in both completion_writer_test.py and
  complete_test.py

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
_extract_config_value_choices now accepts a config instance instead of
the MngConfig class, enabling it to discover completable fields inside
dict-valued entries like plugins.modal.enabled and providers.modal.is_enabled.
The recursive walker uses model_fields for type info and __dict__ for
concrete values, descending into BaseModel values within dicts.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ions

Map annotation types (AgentTypeName, ProviderBackendName) to dynamic
completion sources so that config fields like agent_types.*.parent_type
and providers.*.backend offer tab-completable values from runtime
registries instead of being treated as freeform strings.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… tab completion

These fields are computed/set programmatically and not meaningful for users
to set directly via `mng config set`. Filter them from both config_keys and
config_value_choices in _build_dynamic_completions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ete -b hint

Three issues fixed:

1. _install_claude() added $HOME/.local/bin to PATH but the Claude Code
   installer places the binary at $HOME/.claude/local/bin (confirmed by
   the resources/Dockerfile). This caused "bash: claude: command not found"
   when connecting to docker agents.

2. The default minimal Dockerfile (generated from REQUIRED_HOST_PACKAGES)
   did not include xxd, which mng_log.sh requires for generating event IDs.
   This caused background tasks to fail with "xxd: command not found".

3. The warning message suggesting -b --file=<path> was incomplete -- docker
   build also requires a build context path, so the hint now shows
   -b --file=<path> -b .

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Problem: The docstring for build_check_and_install_packages_command listed
"(sshd, tmux, curl, rsync, git, jq)" but xxd was added to REQUIRED_HOST_PACKAGES
by the previous commit, making the docstring out of date.

Fix: Added xxd to the parenthetical package list in the docstring.
The default debian:bookworm-slim image lacks ca-certificates, so curl
cannot make HTTPS connections. This caused _install_claude() to silently
fail: the `curl ... | bash` pipeline masks curl's exit code because bash
exits 0 on empty input.

Two fixes:
1. Add ca-certificates to REQUIRED_HOST_PACKAGES so it's pre-installed
   in the default Dockerfile and checked at runtime.
2. Restructure _install_claude() to download the script to a file first,
   then run it. This way a curl failure breaks the && chain and surfaces
   as an error instead of silently producing a broken install.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tion

The Claude Code installer places the binary at ~/.local/bin/claude, not
~/.claude/local/bin. Updated the PATH in both _install_claude() and the
resources/Dockerfile.

Also added a test -x verification step after install so failures are
caught immediately, and extracted CLAUDE_INSTALL_PATH as a constant.
Refactored install_command to build from a list of steps for readability.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Problem: The docstring for _install_claude() still referenced the old
piped approach (e.g., 'bash -s 2.1.50') but the function was refactored
to download the script to a temp file first.

Fix: Updated the docstring to describe the new download-then-run approach
and corrected the example to 'bash /tmp/install_claude.sh -s 2.1.50'.
Problem: _install_claude() passed "-s" as a script argument
(bash /tmp/install_claude.sh -s 2.1.50), but "-s" is a bash flag
meaning "read from stdin". This caused the script to receive "-s" as $1
and the version as $2, instead of the version as $1.

Fix: Removed the "-s" prefix so the version is passed as a positional
argument directly (bash /tmp/install_claude.sh 2.1.50), matching how
bash passes arguments to script files. The Dockerfile already does this
correctly via the pipe approach (cat script | bash -s VERSION).
joshalbrecht and others added 30 commits March 13, 2026 05:23
Replace per-agent authentication with a single global session cookie.
One login grants access to all agents.

Auth changes:
- AuthStoreInterface: remove agent_id from add_one_time_code and
  validate_and_consume_code; remove list_agent_ids_with_valid_codes
- Cookie manager: replace per-agent cookies with a single mind_session
  cookie using create_session_cookie/verify_session_cookie
- All route handlers use _is_authenticated() instead of per-agent checks
- Login/authenticate routes take only one_time_code, not agent_id
- Create/API routes now require authentication

Server startup:
- Runner generates a one-time code and prints login URL to terminal
- Landing page shows login prompt when unauthenticated

Agent creation:
- Remove generate_login_url (no longer needed with global auth)
- AgentCreationInfo.login_url -> redirect_url (points to agent page)
- Remove forwarding_server_port from AgentCreator (not needed)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Update design.md step 4 and render_creating_page docstring to reflect
that agent creation redirects directly to the agent (user is already
authenticated via global session).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Update the auth error page to tell users to check the terminal or
restart the server (instead of the old "generate a new login URL"
text). Update user_story.md to describe the login-first flow and
use "known agent" instead of "authenticated agent".

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove tests that added no value beyond coverage numbers:
- 15 stdlib wrapper tests (Path.exists, mkdir, listdir, etc.)
- 5 no-op provisioning method tests (testing `pass` doesn't crash)
- Trivial getter/path-join tests, isinstance checks, string formatting
- Shell command substring assertions
- Enum value existence checks

Coverage remains at 80.00% with 3053 tests.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…t_id

Three changes:

1. Default git URL in creation form to https://github.com/imbue-ai/simple_mind.git
   (both as placeholder and pre-filled value when no URL is specified)

2. When backend is unavailable (agent still starting), show a "Starting up..."
   page that auto-refreshes every 3 seconds instead of immediately returning 502.
   Gives up after 30 seconds with an error message. This handles the common case
   where the user is redirected to a newly created agent before its web server
   has registered with the forwarding server.

3. In event_watcher.py, change MNG_AGENT_NAME to MNG_AGENT_ID and pass agent_id
   through all functions instead of agent_name (_send_message, _deliver_batch,
   _run_delivery_loop, _start_events_subprocess, main).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The timeout was never firing because location.reload() reset the
JavaScript startTime on each page load. Now the start timestamp is
persisted via a _wait_start URL parameter so the timeout correctly
accumulates across reloads.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…oads

The previous approach of reloading the page to check backend availability
had two problems:
1. The timeout state was lost across reloads (fixed in prior commit)
2. Page reloads interfere with the service worker bootstrap flow, causing
   the page to get stuck even when the backend is actually available

Now the waiting page polls /api/backend-ready/{agent_id}/{server_name}
via fetch() every 3 seconds. When the backend is ready, it redirects
to the agent page via window.location.href, which triggers the normal
SW bootstrap flow cleanly.

Added GET /api/backend-ready/{agent_id}/{server_name} endpoint that
returns {"ready": true/false}.

Note: test_stream_manager_full_snapshot_updates_agent_ids is a
pre-existing flaky test (timing-dependent, not related to this change).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Instead of serving a JavaScript-based waiting page that polls or
reloads, the proxy handler now waits server-side (asyncio.sleep loop)
for up to 30 seconds for the backend to become available. If it
becomes available during the wait, the request is proxied normally.
If not, the original 502 error is returned.

The wait timeout is configurable via backend_wait_timeout_seconds
on create_forwarding_server (tests set it to 0 to avoid delays).

Removed the backend waiting page template, render function, and
/api/backend-ready endpoint as they are no longer needed.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ling

1. mng events --follow: retry when file doesn't exist instead of failing
   immediately. Waits up to 2 minutes with 2s retries for the file to be
   created. Fixes race condition where events stream starts before the
   agent writes its servers/events.jsonl file.

2. Login redirect: after authentication, redirect to /create instead of /
   so users land directly on the agent creation page.

3. Agent name field: creation form now has a name field (defaults to
   "selene"). Passed through to mng create. Also default git URL to
   https://github.com/imbue-ai/simple_mind.git.

4. SSE creation logs: replace polling with Server-Sent Events for the
   creating page. Streams real-time output from git clone and mng create
   processes via a per-agent log queue. Creating page displays logs in a
   terminal-style container. Added /api/create-agent/{id}/logs endpoint.

5. Faster backend polling: reduced poll interval from 1s to 0.5s for the
   server-side wait when backend is unavailable.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The landing page at / already routes authenticated users correctly:
shows the create form when no agents exist, redirects to a single
agent, or lists multiple agents. Hardcoding /create broke the flow
for users who re-authenticate with existing agents.

Also fix render_creating_page docstring to reflect SSE (not polling).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The mng ratchet forbids time.sleep. Use threading.Event().wait(timeout=)
instead for the retry delay in the event file follow loop.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Store full DiscoveredAgent data in ParsedAgentsResult so the backend
resolver can inspect agent labels. Add list_known_mind_ids() method
that filters for agents with the "mind" label key set.

The landing page now uses list_known_mind_ids() instead of
list_known_agent_ids(), so only agents labeled as minds are shown.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The FIXME comment contained 'time.sleep()' as text which the ratchet
regex matched. Shortened the comment to avoid the match.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The retry loop for missing event files now has a maximum of 60 retries
(~2 minutes) instead of retrying infinitely. After exhausting retries,
raises MngError like before.

Added test_follow_event_file_via_host_retries_when_file_missing that
verifies the retry path works when a file appears after a delay.

Also fixed agent_creator.py docstring (iter_log_lines -> get_log_queue).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Restore the user's original retry loop in events.py (infinite retry
with FIXME comment about tenacity).

Remove the fake DiscoveredAgent construction from parse_agents_from_json
-- the real DiscoveredAgent data comes from the streaming path via
_handle_full_snapshot, not from JSON parsing. The test helper
make_resolver_with_data now builds DiscoveredAgent objects directly
for test data that needs mind label filtering.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.

3 participants