Skip to content

fix: approve-join validation and mailbox corrupted message handling#9

Closed
xzq-xu wants to merge 1 commit intoHKUDS:mainfrom
xzq-xu:fix/critical-bugs
Closed

fix: approve-join validation and mailbox corrupted message handling#9
xzq-xu wants to merge 1 commit intoHKUDS:mainfrom
xzq-xu:fix/critical-bugs

Conversation

@xzq-xu
Copy link
Copy Markdown
Contributor

@xzq-xu xzq-xu commented Mar 18, 2026

Summary

  • Fix approve-join proceeding without a valid join request: team approve-join would silently add a member with a fabricated name (agent-{id[:6]}) when no matching join request was found. It now returns an error and exits.
  • Fix receive()/peek() crashing on corrupted messages: A single invalid message in an inbox caused the entire receive() or peek() call to fail. With consume=True, already-deleted valid messages were lost. Invalid messages are now skipped gracefully via per-message try/except.

Note: the plans cleanup fix was dropped from this PR as it was already resolved upstream via PR #6.

Test plan

  • All 135 existing tests pass (rebased on latest main)
  • Ruff lint clean
  • Manually test team approve-join with an invalid request ID
  • Manually test mailbox with a corrupted message file in inbox

Longado pushed a commit to Longado/ClawTeam that referenced this pull request Mar 18, 2026
Combines the best of both open PRs, resolving their overlapping approaches
and applying all 6 fixes that are not yet in main:

**mailbox.py**
- broadcast(): resolve logical agent names to inbox directory names before
  exclusion check, so the sender is not delivered their own broadcast when
  inboxes use user-prefixed format (e.g. "alice_worker" vs "worker")
  [from PR HKUDS#15]
- receive() / peek(): replace bare list comprehension with _parse_messages()
  helper that wraps each message in try/except, so a single corrupted file
  no longer crashes the entire fetch [from PR HKUDS#9]

**transport/file.py**
- fetch(consume=True): atomically rename msg file to .consumed before
  reading, so concurrent callers cannot both receive the same message
  [from PR HKUDS#15]

**cli/commands.py**
- config set / config get: derive valid_keys from ClawTeamConfig.model_fields
  instead of a hardcoded 3-item set, making all 7 config keys accessible;
  handle boolean fields (skip_permissions) correctly [from PR HKUDS#15]
- approve-join: return an error and exit(1) when no matching join request
  is found, instead of silently fabricating an agent name [from PR HKUDS#9]
- launch_team: read skip_permissions from config and pass it to be.spawn(),
  matching the behaviour of the standalone spawn command [from PR HKUDS#15]

Note: the plans-cleanup fix (PR HKUDS#9 subdirectory approach, PR HKUDS#15 member-filter
approach) was already merged via PR HKUDS#6 using the subdirectory strategy, so it
is intentionally excluded here.

All 135 existing tests pass.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Fix approve-join proceeding even when no matching join request is
  found, which silently added members with fabricated names. Now
  returns an error and exits.
- Fix receive()/peek() crashing on any corrupted message, which also
  caused valid consumed messages to be lost. Invalid messages are now
  skipped gracefully via per-message try/except.

Note: the plans cleanup fix was dropped from this PR as it was already
resolved upstream via PR HKUDS#6.

Made-with: Cursor
@xzq-xu xzq-xu force-pushed the fix/critical-bugs branch from 601fbcc to 3523cdf Compare March 18, 2026 10:43
@xzq-xu xzq-xu changed the title fix: critical bug fixes for team cleanup, join approval and mailbox fix: approve-join validation and mailbox corrupted message handling Mar 18, 2026
@tjb-tech
Copy link
Copy Markdown
Collaborator

Thanks for the fixes here.

I’m closing this PR because the relevant bug fixes have already been absorbed into main through later changes.

In particular, the following issues covered here are now fixed upstream:

  • team approve-join now errors when the request ID does not exist
  • mailbox receive/peek now skips corrupted messages instead of failing the whole batch

Merged into main via:

  • 791ce16 Fix CLI and mailbox edge cases from PR #16

Thanks again — this PR was useful and directly informed the final fixes.

@tjb-tech tjb-tech closed this Mar 19, 2026
@xzq-xu xzq-xu deleted the fix/critical-bugs branch March 19, 2026 10:08
juntaochi added a commit to novix-science/ClawTeam-gstack that referenced this pull request Apr 20, 2026
Covers:
- Task 2 core: safety subscriber registration, start_sprint state.json + event emit,
  gate chain order + short-circuit, auto_advance semantics (4 combinations),
  MissingTeamError, resolve_sprint prefix/ambiguous/not-found, list_sprints
  team-scoped (Pitfall HKUDS#9), three-layer cap override (D-29), advance_phase happy-path
- Task 3 pause/resume: in-process round-trip, cross-subprocess restart (CORE-07),
  last PhaseTransition re-emit on resume, careful_enabled restoration on resume
juntaochi added a commit to novix-science/ClawTeam-gstack that referenced this pull request Apr 20, 2026
- Register sprint_app = typer.Typer(...) via app.add_typer(name='sprint').
- Ship 6 sub-commands: start, status, show, list, pause, resume — each
  dispatches into SprintConductor and owns error-to-envelope translation.
- Add _sprint_emit_ok / _sprint_emit_err helpers emitting the uniform
  {ok, data, warnings, error} envelope per D-26 (mirrors guard_app pattern).
- Add _resolve_team_arg (Pitfall HKUDS#9: MISSING_TEAM when --team + env absent)
  and _resolve_sprint_or_err (translates AmbiguousSprintError /
  SprintNotFoundError → AMBIGUOUS_SPRINT / SPRINT_NOT_FOUND codes).
- CLI flags --artifact-cap / --phase-artifact-cap honor the three-layer D-29
  override; --auto-advance/--no-auto-advance toggles INT-06 semantics.
- Human renderer uses existing module-level rich console (BC-preserving).
- Close UX-02 (start), UX-03 (status), UX-04 (list), UX-05 (show), UX-09
  (uniform JSON envelope across all 6 commands).
- 15 CliRunner tests cover success + error-code branches; Phase 0 regression
  matrix 12/12 stays green (sub-app is additive, no existing command edits).
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.

2 participants