Skip to content

fix(register): a successful registration must update the identity it registered - #27

Merged
jpelaez-23blocks merged 2 commits into
mainfrom
fix/register-updates-config
Sep 3, 2026
Merged

fix(register): a successful registration must update the identity it registered#27
jpelaez-23blocks merged 2 commits into
mainfrom
fix/register-updates-config

Conversation

@jpelaez-23blocks

Copy link
Copy Markdown
Contributor

Reported by salesland-dev-3metas (Salesland iCPA, 3Metas), reproduced against a local AI Maestro provider. Both confirmed by reading the success path.

1. config.json was never updated

amp-register.sh writes registrations/<provider>.json and IDENTITY.md, and calls back to the Maestro API — but never touches config.json. Every tool that reads .agent.address therefore keeps reporting the pre-registration identity:

tool showed
amp-statusline.sh agent@default.local
amp-identity.sh Tenant: default
amp-inbox.sh Your address: agent@default.local

Meanwhile the registration had genuinely succeeded — apiKey issued, registration file present, address salesland-dev-3metas@rnd23blocks.aimaestro.local. A correctly registered agent looked unregistered in every place a human would think to check.

Now patched surgically with jq.agent.tenant, .agent.address, and a provider block — not through save_config, which rebuilds the object and would drop agent.id, fingerprint and createdAt. That is precisely the destructive shape of the old load_config auto-fix (#26) and worth not repeating. Two of the new tests pin exactly that.

config.json carries one address, so it reflects the most recent registration; registrations/ remains the authoritative per-provider list. Noted in the code for multi-provider agents.

2. Exit 0 having persisted nothing

The reporter's first invocation printed Updating identity file... and exited 0 with registrations/ still empty; only a second run with --force worked.

set -e does not catch this, because the failure mode is a jq that writes an empty file rather than a command returning non-zero. The success path now verifies the registration file exists and contains an apiKey, and exits 1 otherwise.

An exit code that does not track whether the work happened is worse than no exit code at all — it makes the caller confident about something nobody checked.

3. --api-url needs the /api base

The script posts to {API_URL}/v1/register, so http://localhost:23000 yields a 404 that reads like the provider is down. Documented in --help with a self-hosted example, since only crabmail.ai was listed as supported.

amp-register --provider aimaestro.local --tenant myorg \
             --api-url http://localhost:23000/api

Tests

8 new in tests/unit/register_config_sync.bats, covering the address/tenant/provider patch, that it preserves agent id and fingerprint, that load_config reports the registered address afterwards, and that --help documents the base path.

Full suite: 206 passing.

The reporter offered to test a patch against their agent — worth taking them up on it before this lands anywhere wide.

Juan Pelaez and others added 2 commits September 2, 2026 19:15
…registered

Reported by salesland-dev-3metas (Salesland iCPA, 3Metas), reproduced against
a local AI Maestro provider.

1. amp-register.sh wrote registrations/<provider>.json and IDENTITY.md but
   NEVER touched config.json. Every tool that reads .agent.address therefore
   kept reporting the pre-registration identity — amp-statusline, amp-identity
   and amp-inbox all showed @default.local for an agent whose registration had
   succeeded, apiKey issued and registration file present.

   A correctly registered agent looked unregistered everywhere a human would
   think to check.

   Now patched surgically with jq — .agent.tenant, .agent.address and a
   provider block — rather than through save_config, which rebuilds the object
   and would drop agent.id, fingerprint and createdAt. That is the same
   destructive shape as the old load_config auto-fix and worth not repeating.

   config.json carries one address, so it reflects the most recent
   registration; registrations/ keeps the authoritative per-provider detail.

2. A first invocation could print "Updating identity file..." and exit 0
   having persisted nothing, with registrations/ left empty; only a second run
   with --force worked. `set -e` does not catch it, because the failure is a
   jq that writes an empty file rather than a command returning non-zero. The
   success path now verifies the registration file exists and contains an
   apiKey, and exits 1 otherwise. An exit code that does not track whether the
   work happened is worse than none: it makes the caller confident about
   something nobody checked.

3. Docs: --api-url must include the API base, because the script posts to
   {API_URL}/v1/register — http://localhost:23000 gives a 404 that reads like
   the provider is down. Documented in --help with a self-hosted example.

Tests: 8 new in tests/unit/register_config_sync.bats, including that the patch
preserves agent id and fingerprint. Full suite 206 passing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Root cause of the second half of the salesland-dev-3metas report — the run
that "exited 0 having persisted nothing".

amp-helper resolves AMP_DIR from the working directory when nothing more
explicit is available: a .claude/settings.local.json hint, else the AI
Maestro agent that uniquely owns $PWD. That is an INFERENCE about which
agent a directory belongs to, and it was being treated as licence to CREATE
that agent's identity. A hint naming an agent with no entry in this home
fell through to the raw NAME path and the auto-create manufactured an empty
shell — keys/, messages/, registrations/, no config.

Reproduced from a clean home: one `amp-init --name foo` inside a project
whose settings.local.json names a different agent produced TWO directories,
the real uuid one and a stray shell for the unrelated name, because
amp-init overrides AMP_DIR only after the helper has already created the
wrong one. That is also where salesland's registration went: written under
one resolution while they inspected another.

The shells are not harmless. A later name-based resolution can select the
empty shell over the real identity, which is one of the ways an agent ends
up looking unregistered or reading an empty inbox.

Fix: the cwd hint resolves only when the identity already exists (index
entry or directory). Otherwise the inference was wrong and we fall through.

That exposed a latent dependency worth naming: amp-init only worked because
SOMETHING resolved, so it relied on a cwd inference firing. On a genuinely
clean home with no project hint it would have failed outright. It now sets
AMP_ALLOW_UNRESOLVED=1 — every other script keeps refusing to guess.

Verified end to end from a clean home: init creates exactly one directory,
register updates config.json, and amp-identity reports the registered
tenant instead of `default`.

Tests: 5 new in tests/unit/no_stray_identity_dirs.bats. Suite 211 passing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jpelaez-23blocks
jpelaez-23blocks merged commit 144140b into main Sep 3, 2026
2 checks passed
@jpelaez-23blocks
jpelaez-23blocks deleted the fix/register-updates-config branch September 3, 2026 01:26
jpelaez-23blocks added a commit to 23blocks-OS/ai-maestro-plugins that referenced this pull request Sep 3, 2026
…entity dirs (#33)

Picks up agentmessaging/claude-plugin#27:
- amp-register.sh patches config.json on success, so a registered agent no
  longer reports @default.local in statusline, amp-identity and amp-inbox
- exits non-zero when the registration could not be persisted, instead of
  printing success it had not earned
- a cwd inference no longer manufactures an empty identity directory, which
  is where a registration could land while the operator inspected another
- amp-init sets AMP_ALLOW_UNRESOLVED=1; it previously depended on some
  inference firing and would fail on a genuinely clean home

Until this rebuild and the submodule bump, hosts keep the old scripts —
install-plugin.sh reads this built output, not claude-plugin directly.

Co-authored-by: Juan Pelaez <juan.pelaez@3metas.com>
Co-authored-by: Claude Opus 5 (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.

1 participant