FEAT-011 (1/?): spec, plan, US1 MVP for app.* socket namespace#19
Merged
Conversation
Establishes the local app backend contract for a packaged desktop control panel. Adds the `app.*` socket namespace as a façade over the existing FEAT-002..010 service layer — local-only, opaque per-process session tokens, MAJOR.MINOR contract versioning, and adopt-existing-panes as the only path from discovered pane to registered agent (managed session creation is deferred to FEAT-013). Clarify session (2026-05-19) locks five contract decisions: list-pagination default 50 / cap 200, last-write-wins on entity updates, `idempotency_key` on `app.send_input` only, empty `capability_flags` at v1.0, and a 30s hard cap on `app.scan.*` `wait=true` with `scan_timeout` plus `scan_id`. Generates 16 domain-specific requirements-quality checklists (434 items total) covering contract surface, sessions, bootstrap, readiness, dashboard, read surfaces, adopt, mutations, errors, versioning, security, observability, performance, concurrency/reliability, scope/assumptions, and success criteria measurability. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…tion Adds Phase 0 + Phase 1 design artifacts for FEAT-011 (Local App Backend Contract). Includes: - Two more `/speckit.clarify` rounds (12 binding decisions on Session 2026-05-19) tightening: `app.scan.status` shape, `hints[]` contract, `error.details` registry, state/role_priority orderings, `app.*` host-only namespace (FR-042 rewrite), order_by direction syntax, `agent.update` clearable fields, `log.detach` idempotency, filter exact-match at v1.0, removal of `scan_state.expired`, and 1 MiB/8 MiB payload caps. - `/speckit.plan` output: plan.md (~140 lines), research.md (10 decisions), data-model.md (entities + view models + closed sets), contracts/ (app-methods, error-codes, closed-sets), quickstart.md. - 7 new requirements-quality checklists covering plan/research/ data-model/contracts/quickstart quality and cross-artifact consistency + clarify propagation. - `/speckit.tasks` output: tasks.md with 94 dependency-ordered tasks across 8 phases (Setup, Foundational, US1..US5, Polish). - Gap-fix sweep adding FR-016a (degraded_scan), FR-020b (cursor_next format), FR-034b (unknown_method), and 17 new measurable success criteria (SC-011..SC-027) covering every FR added during the three clarify rounds. - `/speckit.analyze` remediation: closed all 4 actionable findings (added T084 SC-023 payload-caps test, T085 SC-025 cursor opacity test, updated T083 for SC-003+SC-021, extended T094 with FR-043 schema diff). Final SC coverage is 27/27. CLAUDE.md pointer updated to FEAT-011 plan. No source code changes; this is design/spec work only. Implementation follows in subsequent commits per the MVP-first incremental strategy documented in tasks.md. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Lands the foundational layer of the FEAT-011 app backend contract:
the `app.*` socket namespace exists end-to-end, `app.preflight` and
`app.hello` are callable, and the closed-set / envelope / host-only
infrastructure every downstream handler depends on is in place.
Tasks completed (per tasks.md):
T002 Dispatcher wiring — merges APP_DISPATCH into FEAT-002's DISPATCH
at the end of socket_api/methods.py (the only existing-module
touch in this commit, per FR-002 / FR-004).
T004 app_contract.versioning — APP_CONTRACT_VERSION = "1.0",
SUPPORTED_MINOR_RANGE, parse_major_minor, is_major_compatible,
capability_flags = {}, all closed-set enums (readiness state,
subsystem status, subsystem names, hint severity, hint codes,
container state, agent roles, role_priority, queue states,
state_priority, scan state, scan kind, mutation origin).
T005 app_contract.errors — 26-entry FR-034 closed code set;
per-code FR-034a `details` registry; ContractViolation +
validate_details() guard for malformed failures; FR-034 regex
enforcement.
T006 Closed-set enums (delivered in versioning.py per task note).
T007 app_contract.envelope — success() / failure() / internal_error()
builders stamping app_contract_version and validating details.
T008 app_contract.host_only — is_host_peer() wrapper around the
existing FEAT-002 `_peer_is_host_process` + `_request_peer_pid`
per FR-042 "MUST reuse the same mechanism".
T010 app_contract.sessions — in-memory SessionRegistry with uuid v4
hex tokens (36 chars), monotonic app_session_id, get/create/
invalidate, test seam (set_registry).
T024 app_contract.preflight — app.preflight handler (no session,
host-only gate, FR-011 success envelope).
T025 app_contract.hello — app.hello handler (FR-010 full field
set, FR-036 major-mismatch guard with both versions in details,
client_id/client_version/client_app_contract_major validation
with field+reason per FR-034a).
Plus a 23-test smoke suite (tests/unit/test_app_contract_smoke.py)
that exercises:
- DISPATCH wiring (FR-001) and legacy-method preservation (FR-002).
- FR-034 regex + 26-entry count + DETAILS_REQUIRED_KEYS membership.
- Envelope shape invariants (FR-033) — success result-defaults-to-{},
failure rejects unknown codes / non-object details / missing required
keys (ContractViolation surfaces).
- app.preflight + app.hello happy/major-mismatch/validation/host-only
paths.
- Session token uniqueness and id monotonicity.
- Versioning parse/compatibility helpers.
Honest scope (the user asked for "Phases 1+2+3 = 30 tasks MVP"; this
commit delivers 9 of those 30):
- T001 (full skeleton, 17 modules) is partial — 9 modules created,
8 deferred (readiness, dashboard, reads, mutations, scans,
idempotency, view_models, audit).
- T003 (mypy/ruff config) — deferred; new sub-package picks up the
existing pyproject.toml settings.
- T009 (1 MiB payload gate) — deferred; FEAT-002's existing 64 KiB
request cap binds first, so FR-003a's 1 MiB ceiling never
effectively triggers at MVP scale. Future tightening will need to
modify server.py MAX_REQUEST_BYTES.
- T011, T012, T013 (IdempotencyStore, ScanRegistry, JSONL audit
helper) — needed for US2/US3, not US1 happy path.
- T014 (full dispatcher gate chain) — partial; host-only and session
gates currently live inside each handler, not at a central dispatcher
entry. Sufficient for two handlers; centralization is a follow-up.
- T015 (view_models for all 7 entities) — deferred.
- T016-T018 (test fixtures as separate files) — fixtures currently
inline in the smoke test.
- T019-T023 (proper contract / integration tests under tests/contract/
and tests/integration/) — deferred; smoke coverage stands in.
- T026-T030 (subsystem probes, app.readiness, app.dashboard, hint
emission, SC-002 benchmark) — deferred. US1 is therefore NOT yet
user-visibly delivered; only the handshake half of US1 works.
The pre-existing FEAT-001..010 test suite runs 313 passed / 1 skipped
/ 1 failed; the one failure is a pre-existing flaky timestamp-ordering
test in tests/integration/test_cli_scan_panes_inactive_cascade.py
(asserts `last_scanned > last_scanned_before` and gets two same-scan-
millisecond timestamps out of order). The failure is unrelated to
this commit's 6-line dispatcher merge.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Lands the visible half of User Story 1: app.readiness probes all 6 subsystems and aggregates state per FR-012/FR-014, and app.dashboard returns counts + recents + structured hints across all 7 surfaces per FR-015..FR-018. Combined with the prior commit's app.preflight + app.hello (commit aa13224), the full US1 bootstrap chain preflight → hello → readiness → dashboard is callable end-to-end. Tasks completed in this commit: T026 Subsystem probes (app_contract/readiness.py) — 6 probes: docker (via discovery_service.list_containers), tmux_discovery (via pane_service presence), sqlite (SELECT 1 probe), jsonl (events_file stat + parent dir check), routing_worker (delivery_worker thread liveness), log_attachment_workers (log_service presence). Each returns a SubsystemRow with status ∈ {ok, degraded, unavailable} per FR-012. T027 Hint registry + emission (app_contract/readiness.py): emit_hints() maps current state (subsystem statuses + 5 summary counts) to the closed v1.0 hint registry (FR-014a): docker_unavailable_hint, start_bench_container, register_first_agent, attach_logs, enable_first_route. check_container_filter is reserved for future telemetry wiring (no FR-014a-defined trigger at MVP). T028 app.readiness handler — composes probes (in FR-013 fixed order: docker, tmux_discovery, sqlite, jsonl, routing_worker, log_attachment_workers), aggregates top-level state per FR-012 rules (all-ok→ready / all-unavailable→unavailable / else→degraded per FR-014), and returns the hints array (always present, possibly []). T029 app.dashboard handler — composes 7 surface counts via direct SQLite COUNT(*) queries against the FEAT-001..010 schema: containers (active/inactive/degraded_scan — degraded_scan returned as 0 at MVP per FR-016a; persisted column wiring deferred), panes (total/registered/unregistered via EXISTS join to agents), agents (total + by_role for the FEAT-006 closed set), log_attachments (active/degraded/none), events (total), queue (by FEAT-009 closed state set), routes (enabled/disabled). Recents for events/queue/routes via compact view-model builders (T015 partial). Plus 19 new smoke tests (test_app_contract_smoke.py) covering: - FR-012/FR-013: readiness envelope shape, 6-subsystem fixed order, reason-empty-when-ok, hints array always present. - FR-014a: docker_unavailable_hint emitted when docker subsystem is unavailable; severity=action_required as registered. - FR-015..FR-018: dashboard envelope shape, all 7 count surfaces present, per-surface bucket sets exactly match closed enumerations (FR-016 buckets for containers/panes/agents; FEAT-009 closed set for queue; enabled/disabled for routes). - FR-017: recent_limit default 10, validation_failed at bounds [0, 51, 100, -1] and on non-int types [str, float, bool, list] with details.field == "recent_limit". - FR-042: host-only gate on both methods. - FR-045: no audit side-effect (events.jsonl unchanged after dashboard call). - FR-018: empty system returns all-zero counts and empty recents. Plus 4 pre-existing DISPATCH lock-in tests updated to reflect the 4 new app.* entries: - tests/unit/test_dispatch_table_stability.py — EXPECTED_ORDER extended; cardinality assertion bumped 35 → 39. - tests/unit/test_socket_api_methods.py — closed-set membership extended. - tests/integration/test_feat005_no_real_container.py — cardinality assertion brought up to date (was pre-existing-broken from FEAT-010; this commit fixes it to FEAT-011 reality). Pre-existing test failures NOT addressed by this commit (none caused by FEAT-011): - test_status_default_output_six_lines (FEAT-008/009/010 staleness: status now emits 14 lines, not 6). - test_backcompat_status_json_keeps_feat002_through_008_keys (FEAT-010 staleness: schema_version is now 8, not 7). - test_cli_scan_panes_inactive_cascade.test_inactive_container_with_only_inactive_prior_panes_still_touches_them (pre-existing flaky millisecond-timestamp race). US1 MVP status: app.preflight, app.hello, app.readiness, app.dashboard are all callable through the FEAT-002 socket dispatcher. The packaged desktop app could now bootstrap and render a usable dashboard end-to- end, with structured hints, against the real daemon. SC-001 (no CLI parsing) is satisfied by construction (all four handlers return structured JSON envelopes). SC-002 (≤500ms dashboard) is not yet measured — a benchmark task (T030) is deferred. SC-008 (token redaction) is structurally satisfied (token never written to JSONL) but not yet verified by a grep test. Remaining for full US1 polish: - T030 SC-002 latency benchmark. - T019..T023 proper contract tests under tests/contract/ and the integration walkthrough under tests/integration/. - T013 audit helper (not needed for US1 read paths but needed for US2/US3 mutations). - T011, T012 idempotency + scan registries (needed for US2/US3). - T015 full view models for all 7 entities (compact builders only shipped here — full PaneViewModel/AgentViewModel/etc. needed for US2/US3 list/detail methods). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Reviewer's GuideIntroduce initial FEAT-011 Sequence diagram for FEAT-011 app.* bootstrap and host-only gatingsequenceDiagram
actor AppClient
participant SocketServer as socket_api.methods
participant AppDispatch as APP_DISPATCH
participant Hello as app_hello
participant Sessions as SessionRegistry
participant Readiness as app_readiness
participant Dashboard as app_dashboard
AppClient->>SocketServer: send {"method": "app.hello"}
SocketServer->>AppDispatch: lookup "app.hello"
AppDispatch-->>SocketServer: Hello
SocketServer->>Hello: app_hello(ctx, params, peer_uid)
alt [peer is not host]
Hello->>Hello: is_host_peer(peer_uid) == False
Hello-->>SocketServer: envelope.failure(HOST_ONLY,...)
SocketServer-->>AppClient: {ok: False, error.code: host_only}
else [peer is host and major compatible]
Hello->>Hello: _coerce_client_major(params[client_app_contract_major])
Hello->>Hello: versioning.is_major_compatible(client_major)
Hello->>Sessions: get_registry().create(...)
Sessions-->>Hello: AppSession
Hello-->>SocketServer: envelope.success({... app_session_token ...})
SocketServer-->>AppClient: {ok: True, result.app_session_token}
end
AppClient->>SocketServer: send {"method": "app.readiness", app_session_token}
SocketServer->>AppDispatch: lookup "app.readiness"
AppDispatch-->>SocketServer: Readiness
SocketServer->>Readiness: app_readiness(ctx, params, peer_uid)
Readiness->>Readiness: is_host_peer(peer_uid)
Readiness->>Readiness: probe_*(), aggregate_state(...)
Readiness-->>SocketServer: envelope.success({state, subsystems, hints})
SocketServer-->>AppClient: readiness envelope
AppClient->>SocketServer: send {"method": "app.dashboard", app_session_token}
SocketServer->>AppDispatch: lookup "app.dashboard"
AppDispatch-->>SocketServer: Dashboard
SocketServer->>Dashboard: app_dashboard(ctx, params, peer_uid)
Dashboard->>Dashboard: is_host_peer(peer_uid)
Dashboard->>Dashboard: _container_counts(), _pane_counts(), _agent_counts(), _queue_counts(), _route_counts()
Dashboard->>Dashboard: _recent_events(), _recent_queue(), _recent_routes()
Dashboard-->>SocketServer: envelope.success({counts, recent, hints})
SocketServer-->>AppClient: dashboard envelope
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The new
host_only.is_host_peerimplementation depends on private helpers (_request_peer_pid,_peer_is_host_process,_NO_PEER_UID) fromsocket_api.methods; consider adding a small public helper in that module (e.g.,is_host_peer_uid(peer_uid)) and calling that instead to avoid tight coupling to underscored internals.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `host_only.is_host_peer` implementation depends on private helpers (`_request_peer_pid`, `_peer_is_host_process`, `_NO_PEER_UID`) from `socket_api.methods`; consider adding a small public helper in that module (e.g., `is_host_peer_uid(peer_uid)`) and calling that instead to avoid tight coupling to underscored internals.
## Individual Comments
### Comment 1
<location path="src/agenttower/app_contract/host_only.py" line_range="26-35" />
<code_context>
+)
+
+
+def is_host_peer(peer_uid: int) -> bool:
+ """Return True iff the current request peer is a host process.
+
+ Implements the FR-042 gate predicate: the caller must run on the
+ host (not inside a bench container) AND have presented valid
+ SO_PEERCRED credentials matching the daemon's uid.
+
+ On non-Linux platforms or when SO_PEERCRED returns no uid, this
+ returns False — a host-only security gate that allows on "no
+ credentials" is bypassable, so we treat "no credentials" as the
+ same as a container caller (matching the FR-042 / routing-toggle
+ rationale in ``socket_api/methods.py``).
+ """
+ if peer_uid == _NO_PEER_UID:
+ return False
+ pid = _request_peer_pid()
</code_context>
<issue_to_address>
**🚨 question (security):** Consider incorporating peer_uid into the host-only check instead of relying solely on the peer PID.
The docstring promises the gate enforces both "runs on host" and "uid matches the daemon's uid", but the implementation ignores peer_uid (beyond `_NO_PEER_UID`) and just calls `_peer_is_host_process(_request_peer_pid())`. Unless `_peer_is_host_process` itself checks uid against the daemon’s uid, this may weaken the FR-042 guarantee. Even if it does, explicitly using peer_uid here would better reflect the stated trust boundary and be more robust to future socket-layer changes. Consider either using peer_uid in the predicate or updating the docstring to match the actual behavior.
</issue_to_address>
### Comment 2
<location path="src/agenttower/app_contract/dashboard.py" line_range="249-258" />
<code_context>
+# ─── Recents (FR-017) ─────────────────────────────────────────────────────
+
+
+def _recent_events(ctx: "DaemonContext", limit: int) -> list[dict[str, Any]]:
+ conn = getattr(ctx, "state_conn", None)
+ if conn is None:
+ return []
+ try:
+ rows = conn.execute(
+ "SELECT event_id, event_type, origin, agent_id, created_at "
+ "FROM events ORDER BY event_id DESC LIMIT ?",
+ (limit,),
+ ).fetchall()
+ except Exception: # noqa: BLE001
+ return []
+ return [
+ {
+ "id": row[0],
</code_context>
<issue_to_address>
**suggestion:** The recent-events shaping logic duplicates compact_event and may drift over time.
In `_recent_events` you manually build `{id, type, origin, agent_id, timestamp, summary}` instead of reusing `view_models.compact_event`. This introduces a second shaping path that can silently diverge if `compact_event` changes (e.g., new fields, different defaults/normalization). Prefer delegating to `compact_event` and trimming fields, or document and enforce a distinct contract if this shape must differ.
Suggested implementation:
```python
def _recent_events(ctx: "DaemonContext", limit: int) -> list[dict[str, Any]]:
conn = getattr(ctx, "state_conn", None)
if conn is None:
return []
try:
rows = conn.execute(
"SELECT event_id, event_type, origin, agent_id, created_at "
"FROM events ORDER BY event_id DESC LIMIT ?",
(limit,),
).fetchall()
except Exception: # noqa: BLE001
return []
# Delegate shaping to view_models.compact_event to keep a single source of truth.
# We construct a minimal event-like dict that compact_event can normalize.
events = [
{
"id": row[0],
"type": row[1],
"origin": row[2],
"agent_id": row[3],
"timestamp": row[4],
}
for row in rows
]
return [view_models.compact_event(event) for event in events]
```
To fully wire this up, ensure the following in `src/agenttower/app_contract/dashboard.py`:
1. `view_models` must be imported in this module, typically near the top of the file. If it's not already present, add something like:
- `from . import view_models`
or the appropriate import for your project layout.
2. Confirm that `view_models.compact_event` accepts the event shape used here. If it expects different keys (e.g. `created_at` instead of `timestamp`), adjust the constructed dict accordingly:
- Change `"timestamp": row[4]` to `"created_at": row[4]` (or the field name expected by `compact_event`).
3. If `compact_event` relies on additional fields from the `events` table (e.g. `payload`, `status`), extend the SQL `SELECT` and the `events` dict construction to include those columns and keys to avoid losing information or causing `KeyError`s.
</issue_to_address>
### Comment 3
<location path="tests/unit/test_app_contract_smoke.py" line_range="165-151" />
<code_context>
+ assert env["result"] == {}
+
+
+def test_failure_envelope_shape() -> None:
+ """FR-033 + FR-034a: failure envelope has version + code + message + details."""
+ env = envelope.failure(
+ app_errors.AGENT_NOT_FOUND,
+ "agent does not exist",
+ details={"agent_id": "abc-123"},
+ )
+ assert env == {
+ "ok": False,
+ "app_contract_version": APP_CONTRACT_VERSION,
</code_context>
<issue_to_address>
**suggestion (testing):** Add a direct test for internal_error envelope shape and details semantics
`envelope.success` and `envelope.failure` are well covered, as is `validate_details`, but `internal_error` isn’t tested directly. Since the dispatcher relies on it as a safety net when handlers misbehave, please add a test that calls `envelope.internal_error()` and asserts that `ok` is False, `error.code` is `internal_error`, and `error.details` is `{}` to lock in the FR-033/FR-034a semantics for this fallback path.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+26
to
+35
| def is_host_peer(peer_uid: int) -> bool: | ||
| """Return True iff the current request peer is a host process. | ||
|
|
||
| Implements the FR-042 gate predicate: the caller must run on the | ||
| host (not inside a bench container) AND have presented valid | ||
| SO_PEERCRED credentials matching the daemon's uid. | ||
|
|
||
| On non-Linux platforms or when SO_PEERCRED returns no uid, this | ||
| returns False — a host-only security gate that allows on "no | ||
| credentials" is bypassable, so we treat "no credentials" as the |
There was a problem hiding this comment.
🚨 question (security): Consider incorporating peer_uid into the host-only check instead of relying solely on the peer PID.
The docstring promises the gate enforces both "runs on host" and "uid matches the daemon's uid", but the implementation ignores peer_uid (beyond _NO_PEER_UID) and just calls _peer_is_host_process(_request_peer_pid()). Unless _peer_is_host_process itself checks uid against the daemon’s uid, this may weaken the FR-042 guarantee. Even if it does, explicitly using peer_uid here would better reflect the stated trust boundary and be more robust to future socket-layer changes. Consider either using peer_uid in the predicate or updating the docstring to match the actual behavior.
| def test_success_envelope_shape() -> None: | ||
| """FR-033: success envelope is {ok: true, app_contract_version, result}.""" | ||
| env = envelope.success({"x": 1}) | ||
| assert env == { |
There was a problem hiding this comment.
suggestion (testing): Add a direct test for internal_error envelope shape and details semantics
envelope.success and envelope.failure are well covered, as is validate_details, but internal_error isn’t tested directly. Since the dispatcher relies on it as a safety net when handlers misbehave, please add a test that calls envelope.internal_error() and asserts that ok is False, error.code is internal_error, and error.details is {} to lock in the FR-033/FR-034a semantics for this fallback path.
There was a problem hiding this comment.
Pull request overview
Introduces FEAT-011’s host-only app.* socket namespace as a versioned façade over the existing FEAT-002..010 service layer, along with extensive contract/spec documentation and dispatch-table wiring so US1 bootstrap methods are callable via the existing socket server.
Changes:
- Add new
src/agenttower/app_contract/package implementingapp.preflight,app.hello,app.readiness, andapp.dashboard, plus envelope/error/versioning primitives. - Merge
APP_DISPATCHinto the legacysocket_api.methods.DISPATCHand update dispatch-table stability/closed-set tests for the new methods. - Add FEAT-011 documentation set (spec/plan/research/data-model/contracts/quickstart) and a large set of requirements-quality checklists.
Reviewed changes
Copilot reviewed 52 out of 52 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_socket_api_methods.py | Extend closed-set keys with app.*. |
| tests/unit/test_dispatch_table_stability.py | Lock dispatch ordering + update method count to 39. |
| tests/integration/test_feat005_no_real_container.py | Update pinned dispatch closed set + count. |
| src/agenttower/socket_api/methods.py | Merge APP_DISPATCH into legacy DISPATCH. |
| src/agenttower/app_contract/init.py | Export contract version constants. |
| src/agenttower/app_contract/view_models.py | Compact “recents” view-model builders. |
| src/agenttower/app_contract/versioning.py | Contract version + closed-set enums/constants. |
| src/agenttower/app_contract/sessions.py | In-memory session registry scaffolding. |
| src/agenttower/app_contract/readiness.py | app.readiness handler + subsystem probes + hints. |
| src/agenttower/app_contract/preflight.py | app.preflight handler. |
| src/agenttower/app_contract/host_only.py | Host-only peer predicate (reuses FEAT-002/009 helpers). |
| src/agenttower/app_contract/hello.py | app.hello handler + validation + session issuance. |
| src/agenttower/app_contract/errors.py | Closed-set error codes + details registry validation. |
| src/agenttower/app_contract/envelope.py | FEAT-011 success/failure envelope builders. |
| src/agenttower/app_contract/dispatcher.py | APP_DISPATCH mapping for new methods. |
| src/agenttower/app_contract/dashboard.py | app.dashboard handler (counts/recents/hints). |
| specs/011-app-backend-contract/research.md | FEAT-011 research decisions. |
| specs/011-app-backend-contract/quickstart.md | Story 1 manual walkthrough. |
| specs/011-app-backend-contract/plan.md | Implementation plan + structure. |
| specs/011-app-backend-contract/data-model.md | Data model + closed sets + validation rules. |
| specs/011-app-backend-contract/contracts/error-codes.md | Error-code registry + details schema. |
| specs/011-app-backend-contract/contracts/closed-sets.md | Closed enumerations + limits. |
| specs/011-app-backend-contract/contracts/app-methods.md | Method surface + wire contracts. |
| specs/011-app-backend-contract/checklists/versioning.md | Versioning checklist. |
| specs/011-app-backend-contract/checklists/sessions.md | Sessions checklist. |
| specs/011-app-backend-contract/checklists/security.md | Security checklist. |
| specs/011-app-backend-contract/checklists/scope-assumptions.md | Scope/assumptions checklist. |
| specs/011-app-backend-contract/checklists/research-rationale.md | Research rationale checklist. |
| specs/011-app-backend-contract/checklists/requirements.md | Spec quality checklist. |
| specs/011-app-backend-contract/checklists/readiness.md | Readiness checklist. |
| specs/011-app-backend-contract/checklists/read-surfaces.md | Read surfaces checklist. |
| specs/011-app-backend-contract/checklists/quickstart-quality.md | Quickstart quality checklist. |
| specs/011-app-backend-contract/checklists/plan-quality.md | Plan quality checklist. |
| specs/011-app-backend-contract/checklists/performance.md | Performance checklist. |
| specs/011-app-backend-contract/checklists/outcomes.md | Success criteria checklist. |
| specs/011-app-backend-contract/checklists/observability.md | Observability/audit checklist. |
| specs/011-app-backend-contract/checklists/mutations.md | Mutations checklist. |
| specs/011-app-backend-contract/checklists/errors.md | Errors checklist. |
| specs/011-app-backend-contract/checklists/data-model-quality.md | Data model quality checklist. |
| specs/011-app-backend-contract/checklists/dashboard.md | Dashboard checklist. |
| specs/011-app-backend-contract/checklists/cross-artifact-consistency.md | Cross-artifact consistency checklist. |
| specs/011-app-backend-contract/checklists/contract-surface.md | Contract surface checklist. |
| specs/011-app-backend-contract/checklists/contract-files.md | Contract files checklist. |
| specs/011-app-backend-contract/checklists/concurrency-reliability.md | Concurrency/reliability checklist. |
| specs/011-app-backend-contract/checklists/clarify-propagation.md | Clarification propagation checklist. |
| specs/011-app-backend-contract/checklists/bootstrap.md | Bootstrap checklist. |
| specs/011-app-backend-contract/checklists/adopt.md | Adopt checklist. |
| CLAUDE.md | Update referenced plan path to FEAT-011. |
| .specify/feature.json | Point feature directory to FEAT-011. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+2180
to
+2187
| # FEAT-011: merge the ``app.*`` namespace into DISPATCH. This import lives | ||
| # AFTER the DISPATCH literal so every FEAT-002 name (``DaemonContext``, | ||
| # ``_NO_PEER_UID``, ``_peer_is_host_process``, ``_request_peer_pid``) is | ||
| # already defined when ``app_contract.host_only`` imports them. The merge | ||
| # is purely additive — no legacy method binding is altered (FR-002). | ||
| from agenttower.app_contract.dispatcher import APP_DISPATCH # noqa: E402 | ||
|
|
||
| DISPATCH.update(APP_DISPATCH) |
Comment on lines
+29
to
+52
| def _build_app_dispatch() -> dict[str, _AppHandler]: | ||
| """Construct the ``app.*`` dispatch map. | ||
|
|
||
| Kept in a function (rather than a module-level dict literal) so the | ||
| imports of individual handler modules happen lazily — this avoids | ||
| a circular import on the FEAT-002 ``socket_api.methods`` side | ||
| (``host_only.py`` imports from there, so importing app_contract | ||
| eagerly from methods.py would loop). | ||
| """ | ||
| from . import dashboard as _dashboard | ||
| from . import hello as _hello | ||
| from . import preflight as _preflight | ||
| from . import readiness as _readiness | ||
|
|
||
| return { | ||
| "app.preflight": _preflight.app_preflight, | ||
| "app.hello": _hello.app_hello, | ||
| "app.readiness": _readiness.app_readiness, | ||
| "app.dashboard": _dashboard.app_dashboard, | ||
| } | ||
|
|
||
|
|
||
| APP_DISPATCH: dict[str, _AppHandler] = _build_app_dispatch() | ||
|
|
Comment on lines
+345
to
+359
| def app_readiness( | ||
| ctx: "DaemonContext", | ||
| params: dict[str, Any], | ||
| peer_uid: int = _NO_PEER_UID, | ||
| ) -> dict[str, Any]: | ||
| """Handler for ``app.readiness`` (FR-012..FR-014a, FR-045).""" | ||
| from .host_only import is_host_peer # lazy import (circular avoidance) | ||
|
|
||
| if not is_host_peer(peer_uid): | ||
| return envelope.failure( | ||
| HOST_ONLY, | ||
| "app.* namespace is host-only; bench-container callers refused", | ||
| details={}, | ||
| ) | ||
|
|
Comment on lines
+312
to
+334
| def app_dashboard( | ||
| ctx: "DaemonContext", | ||
| params: dict[str, Any], | ||
| peer_uid: int = _NO_PEER_UID, | ||
| ) -> dict[str, Any]: | ||
| """Handler for ``app.dashboard`` (FR-015..FR-018, FR-045).""" | ||
| from .host_only import is_host_peer # lazy (circular avoidance) | ||
|
|
||
| if not is_host_peer(peer_uid): | ||
| return envelope.failure( | ||
| HOST_ONLY, | ||
| "app.* namespace is host-only; bench-container callers refused", | ||
| details={}, | ||
| ) | ||
|
|
||
| if not isinstance(params, dict): | ||
| params = {} | ||
|
|
||
| recent_limit, err = _coerce_recent_limit(params.get("recent_limit")) | ||
| if err is not None: | ||
| return err | ||
| assert recent_limit is not None # narrowed by _coerce_recent_limit | ||
|
|
Comment on lines
+71
to
+81
| class SessionRegistry: | ||
| """Thread-safe in-memory app session table. | ||
|
|
||
| Currently single-process scoped. Session tokens are keyed by | ||
| ``app_session_token``; lookup is O(1). | ||
|
|
||
| The registry is intentionally minimal at v1.0 because FEAT-002's | ||
| one-request-per-connection model means tokens are seldom re-presented | ||
| over the same connection; future tightening could add per-connection | ||
| binding and a short TTL on disconnect. | ||
| """ |
Comment on lines
+20
to
+36
| def compact_event(row: Any) -> dict[str, Any]: | ||
| """Compact ``event`` row → dashboard recent payload. | ||
|
|
||
| ``row`` is whatever ``events.dao.select_events`` returns (an | ||
| ``EventRow`` dataclass-like object with ``event_id``, | ||
| ``event_type``, ``origin``, ``created_at``, ``agent_id``, | ||
| ``payload``, etc.). We keep the contract robust to schema drift | ||
| by accessing fields via ``getattr`` with safe defaults. | ||
| """ | ||
| return { | ||
| "id": _get(row, "event_id"), | ||
| "timestamp": _get(row, "created_at"), | ||
| "type": _get(row, "event_type", default=""), | ||
| "origin": _get(row, "origin", default=""), | ||
| "agent_id": _get(row, "agent_id"), | ||
| "summary": _summarize_event(row), | ||
| } |
Comment on lines
+26
to
+42
| def is_host_peer(peer_uid: int) -> bool: | ||
| """Return True iff the current request peer is a host process. | ||
|
|
||
| Implements the FR-042 gate predicate: the caller must run on the | ||
| host (not inside a bench container) AND have presented valid | ||
| SO_PEERCRED credentials matching the daemon's uid. | ||
|
|
||
| On non-Linux platforms or when SO_PEERCRED returns no uid, this | ||
| returns False — a host-only security gate that allows on "no | ||
| credentials" is bypassable, so we treat "no credentials" as the | ||
| same as a container caller (matching the FR-042 / routing-toggle | ||
| rationale in ``socket_api/methods.py``). | ||
| """ | ||
| if peer_uid == _NO_PEER_UID: | ||
| return False | ||
| pid = _request_peer_pid() | ||
| return _peer_is_host_process(pid) |
Comment on lines
+151
to
+175
| def probe_jsonl(ctx: "DaemonContext") -> SubsystemRow: | ||
| """JSONL audit-stream reachability via path stat + write permission.""" | ||
| events_file = getattr(ctx, "events_file", None) | ||
| if events_file is None: | ||
| return SubsystemRow( | ||
| name="jsonl", | ||
| status=SUBSYSTEM_STATUS_UNAVAILABLE, | ||
| reason="events_file not wired", | ||
| hint=None, | ||
| ) | ||
| p = Path(events_file) | ||
| parent = p.parent | ||
| try: | ||
| if not parent.exists(): | ||
| return SubsystemRow( | ||
| name="jsonl", | ||
| status=SUBSYSTEM_STATUS_UNAVAILABLE, | ||
| reason=f"events_file parent {parent} does not exist", | ||
| hint=None, | ||
| ) | ||
| # If the file exists, ensure we can stat it; if it doesn't, | ||
| # the writer creates on first append, which is acceptable. | ||
| if p.exists(): | ||
| p.stat() | ||
| except Exception as exc: # noqa: BLE001 |
Comment on lines
+46
to
+50
| ## R-004: Envelope / details validation approach | ||
|
|
||
| **Decision**: Pure-Python validation via `envelope.py` and `errors.py` — no `pydantic`, no `jsonschema`. Success envelopes are built by `envelope.success(method, result)` and failure envelopes by `envelope.failure(method, code, details, message=None)`. Both helpers stamp `app_contract_version` automatically. The `errors.py` module exposes a registry `DETAILS_SCHEMA: dict[str, set[str]]` enumerating required keys per code; `envelope.failure()` asserts the supplied `details` dict contains every required key for the code, raising a daemon-side `ContractViolation` exception if a handler emits a malformed failure (caught and reported as `internal_error` to the client to avoid leaking the bug). | ||
|
|
||
| **Rationale**: The contract test suite is the system of record for envelope shape; adding `pydantic` / `jsonschema` to the runtime introduces a heavyweight dependency for a closed contract where every shape is known in advance. The closed-set codes and required-key registry are small (25 codes, ~10 entries with structured details) — a hand-written validator is more readable and faster than a schema engine. Contract tests still independently verify response shapes from the outside. |
Comment on lines
+26
to
+33
| **Language/Version**: Python 3.11+ (matches existing daemon). | ||
| **Primary Dependencies**: existing daemon services from FEAT-002 (socket dispatcher), FEAT-003 (container discovery), FEAT-004 (pane discovery), FEAT-006 (agent registration), FEAT-007 (log attachment), FEAT-008 (event pipeline), FEAT-009 (message queue + permission gate), FEAT-010 (routes catalog). No new third-party Python dependencies are introduced. | ||
| **Storage**: In-memory only. No SQLite migration. No JSONL schema bump (the JSONL audit format adds an `origin = "app"` value to its existing `origin` field, which already accommodates string variants). Three in-memory stores added: app-session table, scan-result table (cap 100 per daemon process), per-session idempotency-key dedupe map. | ||
| **Testing**: pytest. New contract test layer under `tests/contract/test_app_*.py` using synthetic Unix-socket clients (no `agenttower` subprocess invocation — see SC-001). Existing FEAT-002..010 test suite remains untouched. | ||
| **Target Platform**: Linux primary; macOS and Windows host targets follow per Assumptions. The daemon is Python; the packaged client is out of scope (FEAT-012). All FEAT-011 work is server-side. | ||
| **Project Type**: CLI daemon + structured-API façade. Single Python package (`agenttower`). | ||
| **Performance Goals**: SC-002 cold-start-to-dashboard ≤ 500 ms (no-cache, ≥1 container, ≥1 agent fixture); SC-004 adopt round-trip ≤ 2 s; `app.preflight` and `app.hello` < 50 ms (target); `app.readiness` < 100 ms; list/detail < 100 ms at default pagination. | ||
| **Constraints**: Local-only — FR-003 forbids any non-Unix-socket listener; SC-006 verifies via packet capture. Host-only `app.*` — FR-042 rejects bench-container peers with `host_only`. Pagination cap 200 (FR-020a). Synchronous scan wait cap 30 s (FR-030b). Per-session idempotency-key store is in-memory only; lost on daemon restart or session close (FR-031a). Per-line payload caps **1 MiB request / 8 MiB response** (FR-003a) — request overflow returns `payload_too_large`; response overflow is a daemon-side invariant guarded by the FR-020a pagination cap. No new persisted secret (FR-043). |
Addresses two self-review findings on PR #19: 1. HIGH — FR-007 session gate was not enforced. ``app.readiness`` and ``app.dashboard`` previously dispatched without checking ``params.app_session_token``. The closed-set codes ``APP_SESSION_REQUIRED`` and ``APP_SESSION_EXPIRED`` existed in ``errors.py`` but were unused — making the FR-007 session contract decorative. Adds ``sessions.gate_session_required(params, peer_uid)`` that combines the FR-042 host-only check with the FR-007 session lookup in the documented order: a. host_only fires first (never leak session-existence to a non-host peer, even if they present a valid-looking token) b. app_session_required (token missing or wrong type) c. app_session_expired (token present but not in registry) Returns ``AppSession`` on success or a failure-envelope dict on either gate rejection. Both ``app_readiness`` and ``app_dashboard`` call it at the top. Token location: per the FEAT-002 dispatcher's ``{method, params}`` request shape, the token lives inside ``params`` (i.e., ``params.app_session_token``) rather than as a top-level request field. The quickstart.md sample is updated accordingly. Keeping the token in ``params`` avoids any change to ``socket_api/methods.py`` beyond the existing 9-line DISPATCH merge. 2. MEDIUM — probe_jsonl did not check writability. The earlier probe only verified existence; a read-only events.jsonl parent or file would still report ``ok`` while silently swallowing audit emissions. Now uses ``os.access(..., os.W_OK)`` on both the parent directory and the events file (when present), reporting ``degraded`` with a clear ``reason`` on either negative. Tests: - 8 new smoke tests for the session-gate paths: test_readiness_session_required_when_token_missing test_readiness_session_expired_when_token_invalid test_readiness_session_required_on_non_string_token test_readiness_host_only_beats_session_gate test_dashboard_session_required_when_token_missing test_dashboard_session_expired_when_token_invalid test_dashboard_host_only_beats_session_gate test_readiness_jsonl_probe_degraded_when_parent_unwritable - New ``host_session`` fixture (uid + token from app.hello) used by every readiness/dashboard test that needs a valid session. - All previously-passing tests still pass (now exercise the gate rather than bypass it). 50 / 50 smoke tests passing (was 42; +8 net for the new coverage). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Lightweight handoff doc for whoever continues `/speckit.implement` in a future session. Captures: - Sync recipe (smoke suite must pass before adding code). - What's already working end-to-end (don't redo). - Three pickup options (A: close US1 properly; B: jump to US2 adopt; C: monster PR — not recommended). - Known weaknesses & deferred follow-ups from PR #19 review. - Three pre-existing FEAT-001..010 test failures NOT caused by FEAT-011 (separate cleanup PR). - Circular-import gotcha + test-time host-peer detection seam. - File map (which modules exist; which are deferred). - Commit-before-walking-away discipline. The `tasks.md` checklist remains the authoritative scope list; this file just adds context the checklist doesn't capture. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ety net Addresses 4 bot-review findings (Sourcery + Copilot) from PR #19: 1. MEDIUM — validate_details() did not enforce FR-034a's "codes not in DETAILS_REQUIRED_KEYS MUST carry details == {}" rule. A handler could emit non-empty details for e.g. ``host_only`` and the contract would drift silently. Now ``validate_details`` raises ``ContractViolation`` whenever an unregistered code carries any keys. Codes IN the registry still allow additional keys beyond required (additive minor evolution per FR-034a). Tests added: test_validate_details_rejects_extras_on_unregistered_code test_validate_details_allows_extras_on_registered_code 2. MEDIUM — envelope.failure() docstring promised that ``ContractViolation`` was caught by the dispatcher and mapped to the FEAT-011 ``internal_error`` envelope, but no such catch existed. The FEAT-002 server's top-level exception handler would instead return the legacy ``socket_api.errors.INTERNAL_ERROR`` shape (no ``app_contract_version`` field) — violating the FR-033 envelope-shape invariant. Adds ``_wrap_handler(handler)`` in ``app_contract/dispatcher.py`` that wraps every ``app.*`` entry in APP_DISPATCH: - catches ContractViolation → FEAT-011 internal_error envelope - catches any other Exception → FEAT-011 internal_error envelope with the exception type name in the message The wrapped functions are what land in DISPATCH. Lazy-imports envelope+errors inside the wrapper body to keep the dispatcher module-load free of cycles. Tests added: test_dispatcher_wraps_contract_violation_into_internal_error test_dispatcher_wraps_unexpected_exception_into_internal_error test_dispatcher_wrapped_handler_passes_through_normal_returns test_app_dispatch_handlers_are_wrapped (identity check that the four real handlers in DISPATCH are wrappers, not raw functions) 3. LOW — dashboard._recent_events() was building the compact event row inline (``{id, type, origin, agent_id, timestamp, summary}``) in parallel with view_models.compact_event() which exposes the same projection — a real drift risk if either changed shape later. Now calls compact_event() with the SELECT columns repackaged as a small dict. No behavior change; one shaping path instead of two. 4. LOW — docs drift between research.md / plan.md and the as-built implementation: - research.md R-004 cited envelope.success(method, result) / envelope.failure(method, code, details, message=None) — the actual signatures are success(result=None) and failure(code, message, details=None). Updated R-004 to match and added a sentence about the dispatcher's _wrap_handler catching ContractViolation+Exception (now matches reality). - plan.md §Technical Context Testing said new contract tests "live under tests/contract/test_app_*.py". Updated to reflect that this PR ships a smoke equivalent under tests/unit/ and the structured layout migration is tracked as T019..T023. The remaining bot-review findings are deferred to follow-up PRs: - Centralized dispatcher gate chain (Copilot, T014) — bigger refactor; current per-handler ``gate_session_required()`` works. - SessionRegistry connection-binding for FR-008 (Copilot) — requires FEAT-002 lifecycle hooks; already documented as a known weakness in sessions.py. - is_host_peer() self-contained peer_uid check (Sourcery+Copilot, defense-in-depth) — FEAT-002 already enforces at accept; deferred to US2/US3 polish. - Direct test for internal_error envelope shape (Sourcery) — now covered indirectly via the dispatcher-wrap tests above. 88 / 88 tests pass (50 smoke + dispatch lock-in + socket_api). The pre-existing CI flake (test_size_cap_rejection_under_100ms perf budget) is unrelated to this PR and stays for a separate cleanup. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… code Encodes the 70-question checklist-walkthrough batch into spec.md as a new "Session 2026-05-19 (round 4)" Clarifications block. All 9 user overrides from the recommended defaults are tagged inline with `(override)`. Spec changes: - FR-003b: NDJSON wire framing — UTF-8, \n only, no \r, no \x00, no trailing content; violations → `malformed_request` before dispatch. - FR-008a/b: `app.hello` idempotent on same connection; 8 concurrent sessions process-wide. - FR-028a-d: adopt full 6-field pane identity match; attach_log+inactive fails whole adopt; nonexistent parent_agent_id → agent_not_found; label normalization (trim, no newlines, ≤256). - FR-030d/e: scan coalescing on same kind; 4 concurrent in-flight scans cap. - FR-031: routing_disabled (kill switch) vs permission_denied (per-msg permission gate) split for app.send_input. - FR-034: 26 → 27 codes; adds malformed_request. - FR-044a-c: audit writer mutex; best-effort under JSONL outage (mutation still commits); audit after SQLite commit, before envelope. - SC-028..SC-038: 11 new contract tests for the above. Contracts: - error-codes.md: 27-entry table; per-code details registry updated; routing_disabled vs permission_denied distinction documented. - closed-sets.md: new sections for wire framing, concurrency caps, audit writer, capability_flags cap. - app-methods.md: adopt mutation rules, send_input routing/permission split, scan coalescing/cap, hello idempotency. Plan: - 27-entry closed set; concurrency caps; wire-framing strictness; audit mutex + best-effort semantics; SC scale fixtures pinned. Checklists: - All 668 items across the 23 checklists ticked (resolved by this round and prior rounds 1-3, per the batch preface). Implementation: - errors.py: adds MALFORMED_REQUEST code + details registry entry; ERROR_CODES bumped to 27. - smoke test updated: count assertion 26 → 27, plus malformed_request details-registry assertion. 57 passed (+1). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds tests/integration/test_story1_dashboard_bootstrap.py — 10 tests walking the Story 1 bootstrap chain (preflight → hello → readiness → dashboard) over a real Unix socket against the running daemon. Closes the socket-level test gap noted in PR #19 review. Coverage: - preflight envelope shape (FR-011) over the wire - hello FR-010 minimum field set, capability_flags == {} at v1.0, supported_minor_range == 1.0..1.0 - per-call session token issuance (one-request-per-connection model) - token persistence across fresh connections (the actual implementation reality vs. FR-008's connection-bound wording — see T097) - readiness 6 required subsystems + hints array always present - dashboard 7 count surfaces + hints array - FR-007 session-required gate over the wire - FR-034b unknown_method on app.foo.bar - SC-002 worst-of-5 ≤500ms wall-clock hello→dashboard (Round-4 Q52 superseded the original p95-over-20 wording) - SC-008 app_session_token never in events.jsonl Marks T023 + T030 done in tasks.md. Surfaced two real spec/implementation drifts (filed as T097, T098): - FEAT-002 one-request-per-connection breaks FR-008 / FR-008a as written; implementation adapted, spec text needs updating. - Legacy unknown_method envelope is missing FR-033 details:{} and app_contract_version stamp on app.* names; needs a wrapping rewriter. NEXT_SESSION.md updated with the latest state. Smoke suite: 57 passed. New integration suite: 10 passed. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Comment on lines
+83
to
+107
| def __init__(self) -> None: | ||
| self._lock = threading.Lock() | ||
| self._sessions: dict[str, AppSession] = {} | ||
|
|
||
| def create( | ||
| self, | ||
| *, | ||
| client_id: str, | ||
| client_version: str, | ||
| client_app_contract_major: int, | ||
| host_user_id: str, | ||
| ) -> AppSession: | ||
| """Issue a new session and record it.""" | ||
| session = AppSession( | ||
| app_session_token=_generate_token(), | ||
| app_session_id=_next_session_id(), | ||
| client_id=client_id, | ||
| client_version=client_version, | ||
| client_app_contract_major=client_app_contract_major, | ||
| host_user_id=host_user_id, | ||
| connection_started_at_ms=int(time.time() * 1000), | ||
| ) | ||
| with self._lock: | ||
| self._sessions[session.app_session_token] = session | ||
| return session |
Comment on lines
+1
to
+18
| """FEAT-011 app-session registry (FR-005..FR-010, FR-036). | ||
|
|
||
| In-memory only. Per-connection sessions issued by ``app.hello``, | ||
| invalidated when the underlying socket connection closes (FR-008). | ||
| Never persisted across daemon restarts (FR-006). | ||
|
|
||
| Note: in the current FEAT-002 daemon, every connection processes a | ||
| single request and is then closed (FR-026 one-request-per-connection). | ||
| That makes connection-scoped session lifetimes very short — every | ||
| ``app.*`` request that requires a session would, in practice, have | ||
| its token presented over a fresh connection. The session table is | ||
| still useful for audit attribution (``app_session_id`` flows to | ||
| JSONL per FR-044) and for the major-mismatch guard from FR-036. | ||
|
|
||
| For follow-up tightening, the dispatcher could maintain a session-token- | ||
| indexed map that survives a connection close for a short TTL, but | ||
| that is explicitly NOT FEAT-011 v1.0 behavior — the contract says | ||
| "session invalidated on connection close" (FR-008). |
Comment on lines
+59
to
+76
| def wrapped( | ||
| ctx: "DaemonContext", | ||
| params: dict[str, Any], | ||
| peer_uid: int = -1, | ||
| ) -> dict[str, Any]: | ||
| from . import envelope as _envelope | ||
| from .errors import ContractViolation | ||
|
|
||
| try: | ||
| return handler(ctx, params, peer_uid) | ||
| except ContractViolation as exc: | ||
| return _envelope.internal_error( | ||
| f"app.* handler emitted malformed envelope: {exc}" | ||
| ) | ||
| except Exception as exc: # noqa: BLE001 — FR-033 envelope-shape safety net | ||
| return _envelope.internal_error( | ||
| f"app.* handler raised {type(exc).__name__}: {exc}" | ||
| ) |
Comment on lines
+26
to
+37
| def is_host_peer(peer_uid: int) -> bool: | ||
| """Return True iff the current request peer is a host process. | ||
|
|
||
| Implements the FR-042 gate predicate: the caller must run on the | ||
| host (not inside a bench container) AND have presented valid | ||
| SO_PEERCRED credentials matching the daemon's uid. | ||
|
|
||
| On non-Linux platforms or when SO_PEERCRED returns no uid, this | ||
| returns False — a host-only security gate that allows on "no | ||
| credentials" is bypassable, so we treat "no credentials" as the | ||
| same as a container caller (matching the FR-042 / routing-toggle | ||
| rationale in ``socket_api/methods.py``). |
Comment on lines
+23
to
+32
| ## R-002: macOS SO_PEERCRED variant choice | ||
|
|
||
| **Decision**: Use Python's `socket.getsockopt(socket.SOL_LOCAL, socket.LOCAL_PEERCRED)` (macOS) and `socket.SO_PEERCRED` (Linux). Wrap both behind `host_only.get_peer_pid_uid(conn)` returning `(pid, uid)`. The host-vs-container check runs against `pid`; the UID check (FR-041) runs against `uid`. | ||
|
|
||
| **Rationale**: This matches what FEAT-009's existing implementation uses (per code inspection of `src/agenttower/routing/permissions.py`). On macOS, `LOCAL_PEERCRED` returns `(uid, gid)` and `LOCAL_PEERPID` returns the pid; both are needed. On Linux, `SO_PEERCRED` returns `(pid, uid, gid)` in one call. | ||
|
|
||
| **Alternatives considered**: | ||
| - *`psutil` cross-platform abstraction*: rejected — adds a runtime dependency for a one-liner. | ||
| - *Windows-named-pipe variant*: out of scope — Windows daemon target uses Unix-domain sockets via WSL2 (per Assumptions) or AF_UNIX which Windows 10+ supports natively; `getsockopt(SO_PEERCRED)` is not available on Windows AF_UNIX, so the daemon falls back to checking the process owner via `GetNamedPipeServerProcessId` equivalent. This is out of scope for FEAT-011's contract — left to plan-level implementation in FEAT-012 / FEAT-013 host-runtime work if Windows desktop ships. | ||
|
|
Implements FR-003b wire-framing rejection at the FEAT-002 socket reader
and closes the T098 drift around legacy ``unknown_method`` envelopes for
``app.*`` methods. Also lands a small batch of follow-on edits that
landed in the tree alongside (sessions/hello/dispatcher/spec/integration
test refinements that round-4 + T023 surfaced).
FR-003b (wire framing):
Adds ``_make_malformed_request_envelope(reason)`` in
``socket_api/server.py``. Lazy-imports ``app_contract.envelope`` to
avoid the methods.py-side load cycle. Used by ``_read_and_dispatch``
before handler dispatch to reject 5 wire-framing classes with the
FR-003b-mandated FEAT-011 envelope ``{ok: false, app_contract_version,
error: {code: "malformed_request", message, details: {reason}}}``:
- (a) stray ``\r`` byte → "stray CR"
- (b) embedded ``\x00`` byte → "embedded NUL"
- (c) trailing content after first JSON → "trailing content"
- (d) JSON decode error → "json decode error: <msg>"
- (e) empty line (just ``\n``) → "empty line"
Plus invalid UTF-8 (case-(a)-adjacent) → "invalid utf-8"
The FEAT-011 envelope is a strict superset of the legacy FEAT-002
shape — legacy clients still see ``ok: false`` + ``error.code`` they
can parse, just with an unfamiliar closed-set code. Acceptable for
inputs that were always malformed.
Detection uses ``json.JSONDecoder.raw_decode`` so we can distinguish
trailing content from a clean decode failure. The legacy
``REQUEST_TOO_LARGE`` and pre-read ``BAD_JSON`` paths are unchanged
(those operate on bytes-state that predates the method-name parse).
T098 (unknown_method envelope rewriter):
``app_contract.dispatcher`` exports ``is_app_method(name)`` and
``make_unknown_method_envelope(method)`` for FR-033/FR-034b
compliance: any ``app.*`` method name not present in DISPATCH now
emits the FEAT-011 envelope with ``code == "unknown_method"`` and
``details == {}`` (vs the legacy FEAT-002 shape, which is missing
both ``app_contract_version`` and ``details``). Legacy methods stay
on the FEAT-002 envelope.
``socket_api/server.py::_read_and_dispatch`` now branches on
``is_app_method(method)`` before emitting the unknown-method
envelope.
FR-008b (session cap) — already-landed pieces re-confirmed:
``SessionRegistry`` has ``MAX_SESSIONS = 8`` and raises
``SessionCapExceeded`` on overflow; ``hello.py`` catches and emits
``validation_failed`` with ``details = {field: "app.hello",
reason: "too_many_sessions"}``. Lands here too.
FR-008a — spec adapted per T097:
``spec.md`` FR-008a wording is now the "fresh token per app.hello,
re-presented across reconnects" model. The original
"same-connection idempotency" wording is marked deprecated and the
T097 drift discovery is the authoritative reasoning. SC-037 follows
suit. ``sessions.py`` docstring updates match.
Tests:
- ``test_malformed_request_envelope_shape`` — single-case envelope
check (FR-003b).
- ``test_malformed_request_envelope_reason_classes`` — parametrized
over all 6 reason classes; each builds a structurally-valid
FEAT-011 envelope.
- ``test_app_dispatcher_unknown_method_envelope_for_app_method``
— covers both the ``is_app_method`` predicate (4 cases) and the
``make_unknown_method_envelope`` output shape (T098 + FR-034b
``details: {}`` invariant).
111 / 111 tests pass (smoke + integration + dispatch lock-in +
socket_api). The pre-existing FEAT-002 perf flake
(``test_size_cap_rejection_under_100ms``) is unrelated to this PR.
FR-030d (scan coalescing) + FR-030e (scan cap) remain deferred to US2
implementation since neither has a caller yet (the ``app.scan.*``
handlers land with T036–T038). Spec text is in place; impl follows
naturally with the scan handlers.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
T097 (FR-008 / FR-008a session-lifecycle wording drift) and T098 (unknown_method envelope rewriter for app.*) were both fully resolved by commits 5984d6b (round-4 spec updates), 9a1da02 (T023 integration test that surfaced both drifts), and 3f9fcc8 (FR-003b + T098 impl). This commit updates the T097 / T098 entries in tasks.md from open to done with a brief summary of what landed: - T097: FR-008/008a/008b rewritten to describe the token-persistence model that's actually implemented (process-wide SessionRegistry, not connection-bound, invalidated only by daemon restart). FR-008b ships the v1.0 reject-not-evict mode of the 8-session cap. SessionRegistry.create() raises SessionCapExceeded at the cap; hello handler translates to validation_failed.too_many_sessions. New smoke test `test_hello_rejects_9th_concurrent_session` covers it. - T098: server.py emits the FR-033-compliant envelope (with app_contract_version + details:{}) for unknown app.* method names; legacy methods keep their FEAT-002 shape per FR-002. Wired via is_app_method() + make_unknown_method_envelope() in app_contract/dispatcher.py. Full suite: 79 tests passing (58 smoke + 14 socket integration + 7 others under the smoke umbrella). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Regression fix for PR #19 CI (run 26110310901). The prior commit 3f9fcc8 changed two paths in ``_read_and_dispatch`` to emit the FEAT-011 ``malformed_request`` envelope: 1. ``UnicodeDecodeError`` on the raw line bytes 2. ``json.JSONDecodeError`` from ``json.JSONDecoder.raw_decode`` That broke ``tests/unit/test_socket_api_framing.py:: test_dispatch_rejects_malformed_request`` which is a FEAT-002 contract lock-in test asserting ``bad_json`` for both: (b"\xff\xfe not utf-8\n", errors.BAD_JSON) (b"this is not json\n", errors.BAD_JSON) These are legitimate FEAT-002 contract behaviors that legacy CLI callers depend on. Switching to ``malformed_request`` would have required either bumping the FEAT-002 contract (out of scope for this PR) or adding a method-aware envelope rewriter at the wire-framing layer (T098-adjacent, also out of scope). This commit reverts those two paths to the legacy ``errors.make_error(errors.BAD_JSON, ...)`` envelope while keeping the NEW pre-parse checks (\r, \x00, empty line) AND the post-parse trailing-content check on the FEAT-011 ``malformed_request`` envelope — those weren't covered by the FEAT-002 lock-in. FR-003b's documented 5 reason classes therefore land partially: Implemented (malformed_request envelope): (a) stray CR (b) embedded NUL (c) trailing content (e) empty line Deferred (legacy bad_json envelope, to preserve FEAT-002 contract): (d) JSON decode error Plus invalid UTF-8 (FR-003b-adjacent) The deferred classes will be lifted in a future PR via either: - An explicit FEAT-002 contract bump that allows ``malformed_request`` for these cases, OR - A method-aware envelope rewriter that emits ``malformed_request`` for ``app.*`` method names and ``bad_json`` for legacy method names (requires extracting ``method`` from a partially-parsed line — a best-effort heuristic). SC-028 verification therefore covers 4 of 6 wire-framing rejection classes in this PR; the remaining 2 are tracked as follow-up. Local sweep: 107 / 107 tests green (smoke + framing lock-in + integration + dispatch lock-in). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
T011 IdempotencyStore (FR-031a): per-session in-memory LRU cap 256.
Used by app.send_input (US3) for dedupe keyed by
(app_session_id, idempotency_key). Methods: lookup, record, clear,
size. Re-record refreshes in place.
T012 ScanRegistry (FR-030c/d/e): in-memory scan-record store. FIFO
cap 100 records; in-flight cap 4; same-kind coalescing on start().
v1.0 closed state set is exactly {running, completed, failed} —
expired is intentionally absent. Methods: start (returns
record + coalesced bool), complete, fail, lookup, size, in_flight_count.
T013 audit.emit_app_mutation (FR-044/044a/044b/044c): wraps the
existing FEAT-008 events.writer.append_event which already holds a
process-wide mutex (FR-044a). Injects origin="app" + app_session_id.
Rejects payloads carrying protected keys (origin / app_session_id /
app_session_token). FR-044b best-effort: on JSONL outage the
mutation completes, the row is dropped, and one stderr warning is
emitted per 60s window. Caller's mutation is never affected.
22 new unit tests in tests/unit/test_app_foundational_stores.py
cover the contract surface of all three stores.
Full suite: 104 passing (82 pre-existing + 22 new).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Comment on lines
+214
to
+224
| # FR-003b case (c): use raw_decode so we can distinguish "extra | ||
| # content after the first JSON object" from a clean parse failure. | ||
| text_stripped = text.lstrip() | ||
| decoder = json.JSONDecoder() | ||
| try: | ||
| request = json.loads(text) | ||
| request, idx = decoder.raw_decode(text_stripped) | ||
| except json.JSONDecodeError as exc: | ||
| return errors.make_error(errors.BAD_JSON, f"json decode failed: {exc.msg}") | ||
| # Anything non-whitespace remaining is trailing content. | ||
| if text_stripped[idx:].strip(): | ||
| return _make_malformed_request_envelope("trailing content") |
Comment on lines
175
to
179
| @@ -149,15 +178,50 @@ def _read_and_dispatch(self) -> dict[str, Any]: | |||
| f"request line exceeds {MAX_REQUEST_BYTES} bytes", | |||
| ) | |||
Comment on lines
+170
to
+179
| return envelope.success({ | ||
| "app_session_token": session.app_session_token, | ||
| "app_session_id": session.app_session_id, | ||
| "daemon_version": ctx.daemon_version, | ||
| "schema_version": schema_version, | ||
| "app_contract_version": versioning.APP_CONTRACT_VERSION, | ||
| "supported_minor_range": versioning.SUPPORTED_MINOR_RANGE, | ||
| "host_user_id": host_user_id, | ||
| "capability_flags": versioning.CAPABILITY_FLAGS_V1_0, | ||
| "state": "ok", |
Comment on lines
+252
to
+315
| def test_fr003b_wire_framing_stray_cr_returns_malformed_request( | ||
| socket_path: Path, | ||
| ) -> None: | ||
| """FR-003b case (a): a stray `\\r` byte → malformed_request.""" | ||
| sock = _open_socket(socket_path) | ||
| try: | ||
| sock.sendall(b'{"method":\r"app.preflight"}\n') | ||
| buf = b"" | ||
| while not buf.endswith(b"\n"): | ||
| chunk = sock.recv(65536) | ||
| if not chunk: | ||
| break | ||
| buf += chunk | ||
| finally: | ||
| sock.close() | ||
| envelope = json.loads(buf.decode("utf-8")) | ||
| assert envelope["ok"] is False | ||
| assert envelope["error"]["code"] == "malformed_request" | ||
| assert envelope["error"]["details"]["reason"] == "stray CR" | ||
| assert envelope["app_contract_version"] == "1.0" | ||
|
|
||
|
|
||
| def test_fr003b_wire_framing_embedded_nul_returns_malformed_request( | ||
| socket_path: Path, | ||
| ) -> None: | ||
| """FR-003b case (b): an embedded `\\x00` byte → malformed_request.""" | ||
| sock = _open_socket(socket_path) | ||
| try: | ||
| sock.sendall(b'{"method":"app.\x00preflight"}\n') | ||
| buf = b"" | ||
| while not buf.endswith(b"\n"): | ||
| chunk = sock.recv(65536) | ||
| if not chunk: | ||
| break | ||
| buf += chunk | ||
| finally: | ||
| sock.close() | ||
| envelope = json.loads(buf.decode("utf-8")) | ||
| assert envelope["ok"] is False | ||
| assert envelope["error"]["code"] == "malformed_request" | ||
| assert envelope["error"]["details"]["reason"] == "embedded NUL" | ||
|
|
||
|
|
||
| def test_fr003b_wire_framing_trailing_content_returns_malformed_request( | ||
| socket_path: Path, | ||
| ) -> None: | ||
| """FR-003b case (c): trailing content after one JSON object → | ||
| malformed_request (Round-4 Block A Q3 override — reject whole line).""" | ||
| sock = _open_socket(socket_path) | ||
| try: | ||
| sock.sendall(b'{"method":"app.preflight"} {"extra":true}\n') | ||
| buf = b"" | ||
| while not buf.endswith(b"\n"): | ||
| chunk = sock.recv(65536) | ||
| if not chunk: | ||
| break | ||
| buf += chunk | ||
| finally: | ||
| sock.close() | ||
| envelope = json.loads(buf.decode("utf-8")) | ||
| assert envelope["ok"] is False | ||
| assert envelope["error"]["code"] == "malformed_request" | ||
| assert envelope["error"]["details"]["reason"] == "trailing content" | ||
|
|
Comment on lines
+332
to
+356
| def test_sc002_hello_to_dashboard_within_500ms(socket_path: Path) -> None: | ||
| """SC-002: wall-clock from app.hello send to app.dashboard response | ||
| receive is ≤ 500 ms in 5 of 5 trials on a daemon already running. | ||
|
|
||
| Each trial opens two fresh connections (one for hello, one for | ||
| dashboard) to match FEAT-002's one-request-per-connection model. | ||
| """ | ||
| trials = [] | ||
| for _ in range(5): | ||
| t_start = time.perf_counter() | ||
| hello = _one_shot_call(socket_path, "app.hello") | ||
| assert hello["ok"] is True | ||
| token = hello["result"]["app_session_token"] | ||
| dashboard = _one_shot_call( | ||
| socket_path, "app.dashboard", {"app_session_token": token} | ||
| ) | ||
| t_end = time.perf_counter() | ||
| assert dashboard["ok"] is True, dashboard | ||
| trials.append((t_end - t_start) * 1000.0) | ||
| worst = max(trials) | ||
| assert worst <= 500.0, ( | ||
| f"SC-002 violation: worst hello→dashboard wall-clock {worst:.1f} ms " | ||
| f"exceeds 500 ms budget. Trials (ms): " | ||
| f"{', '.join(f'{t:.1f}' for t in trials)}" | ||
| ) |
T015 view_models.py: add the 4 missing full view-model builders
(container_view, pane_view, agent_view, log_attachment_view) plus
full event/queue/route projections alongside the existing compact
dashboard recents. Pure projection functions — no I/O. Robust _get
helper handles dataclass/dict/sqlite3.Row variants. Derived fields
(registered/agent_id on Pane; log_attached/pane_active on Agent;
state_priority on Queue; role_priority on Agent; pane_count and
registered_agent_count on Container) accept caller-supplied joins as
kwargs. 30 unit tests in test_app_view_models.py.
T036 app.scan.containers / T037 app.scan.panes / T038 app.scan.status
(scan_handlers.py):
- wait=true: blocks on ScanRegistry.done up to 30s (FR-030b). On
timeout returns scan_timeout with details.scan_id; the scan
continues server-side and remains reachable via app.scan.status.
- wait=false: kicks the scan off on a daemon thread and returns
{scan_id, state: "running"} immediately.
- Same-kind coalescing via ScanRegistry.start() (FR-030d).
- 4-in-flight cap with validation_failed.too_many_scans_in_flight
(FR-030e).
- Failed scans return success envelope with state="failed" and the
error in `result` — the spec distinguishes wait-timeout from
scan-failure (the former is a wait outcome; the latter is a
terminal scan state).
- app.scan.status returns {state, scan_kind, started_at, completed_at,
result} per FR-030c shape. completed_at + result are null while
running. scan_not_found for unknown / evicted ids.
Wired into APP_DISPATCH; 13 unit tests cover happy paths, coalescing,
in-flight cap, timeout, failure path, status lookups (including
unknown / missing-arg), and unwired-service safety.
Full suite: 147 passing (134 pre-existing + 13 new).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Addresses 3 findings from the orchestrated code review of this branch.
Scoped to the 3 stable FEAT-002 files (lifecycle.py, daemon.py,
server.py) to avoid racing the concurrent US3 session churning
src/agenttower/app_contract/.
H1 (HIGH) — silent host-only gate bypass made auditable.
The AGENTTOWER_TEST_FORCE_HOST_PEER=1 test seam makes the FR-042
host-vs-container peer check return True for ANY caller — a
process-wide bypass of the app.* host-only gate, readable from the
daemon's runtime environment. The seam is needed because WSL2 /
Docker-in-Docker / sandboxed CI runners false-positive the /proc
container probe, so removing it would break the integration suite.
Instead it is now LOUD: a new `host_peer_check_bypassed` warn-level
lifecycle event is emitted at daemon startup whenever the seam is
active, so an operator auditing the daemon log sees the bypass
rather than it being silent. New event added to the closed
LIFECYCLE_EVENTS set in socket_api/lifecycle.py.
H3 (HIGH) — exception text no longer leaked to the wire.
The FEAT-002 dispatch catch-all in server.py returned
`f"{type(exc).__name__}: {exc}"` to the client — the full exception
string, which can carry filesystem paths, SQL fragments, or request
content. It now prints the full traceback to daemon stderr
(operator-visible) and returns a constant generic message
("internal error handling request; see daemon log") on the wire.
test_dispatch_handles_internal_exception_gracefully updated to
assert the exception detail is NOT echoed (it now raises with a
fake "/etc/shadow" path and asserts that path is absent from the
client envelope).
M5 (MEDIUM) — honest payload_too_large messaging.
`_make_payload_too_large_envelope` documented `actual_size_bytes` as
a precise value, but the reader stops at MAX_REQUEST_BYTES + 1 (it
deliberately does not drain the socket — draining would defeat the
cap), so the field is necessarily a lower bound. Renamed the param
to `observed_size_bytes`, message now says "observed at least N
bytes", and the docstring explains the lower-bound semantics plus
the known FEAT-002-64KiB-vs-FR-003a-1MiB cap gap.
Review findings NOT in this commit:
- C1 (CRITICAL, 30-vs-12 method gap) — RESOLVED by the concurrent
US3 commits c537026 + f19a80c; 32 app.* methods are now wired.
- M1 (idempotency.py "dead code") — INVALID; mutations.py imports
IdempotencyStore. Reviewer only checked dispatcher.py.
- H2 (thread-local peer-pid side channel) — deferred; fixing it
means changing every app.* handler signature, too invasive for a
remediation commit. Fails closed today.
- H4 (single-threaded concurrency tests), M2/M3/M4/M6/M7/M8 + LOWs
— deferred; most live in app_contract/ + spec docs that the
concurrent US3 session is actively churning. To be picked up once
the branch settles.
68/68 FEAT-002 unit tests pass (framing, envelope-body, methods,
dispatch-stability).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Completes Phases 6 (US4) and 7 (US5). Most of the functional surface already shipped in US1/US3 — this slice adds the verification test coverage plus one real code change. US4 — Degraded & Unavailable States (T066-T071): - T070 (code): app.preflight now detects the daemon shutting-down window — when ctx.shutdown_requested is set it returns the success envelope with code="daemon_unavailable", daemon_reachable=false. A fully-dead daemon never runs the handler (the client maps the connect failure); the handler-side check covers the in-flight shutdown case. Docstring rewritten to spell out who reports which code. - T071 (verify): all 6 v1.0 hint codes are wired in readiness.py. check_container_filter is a reserved closed-set code — no MVP telemetry can distinguish "filtered out" from "no containers" — so it is present but not emitted; documented, no change. - Tests: test_app_us4_readiness_degraded.py (5 — one per failure mode), test_app_us4_dashboard_hints.py (10 — one per hint code), test_app_us4_preflight.py (5), test_story4_degraded_states.py (6 socket-level, SC-007 zero-CLI-text invariant). US5 — Contract Version Negotiation (T072-T078): - T077 (verify): hello.py FR-036 major-mismatch enforcement (app_contract_major_unsupported, no session issued, full FR-034a details shape) — shipped in US1, confirmed correct here. - Tests: test_app_us5_version_mismatch.py (9), test_app_us5_capability_flags.py (7), test_app_us5_forward_compat.py (5), test_app_us5_unknown_method.py (6), test_story5_version_drift.py (4 socket-level). - T078: tests/fixtures/app_future_minor_client.py — FutureMinorAppClient forward-compat fixture for the SC-009 verification flow. 91 new tests (58 US4+US5 + the fixture self-test set); 137 regression tests confirm the preflight.py change is clean. Note: repaired a stale git-worktree pointer (`git worktree repair`) — the worktree .git file referenced an old /home/brett/projects path; an environment path change, unrelated to FEAT-011. tasks.md: Phase 6 + 7 (T066-T078) marked complete. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Completes the final FEAT-011 phase. All five user stories were already implemented; this phase adds the dedicated SC-verification test suite, the docs, and the final audit sweeps. SC-verification tests (104 tests across 7 new files): - T079 test_app_no_network_listener.py — SC-006/FR-003: spawns a real daemon, drives an app.* slice, asserts via lsof (or /proc fallback) zero TCP/UDP listeners, before and after shutdown. - T080 test_app_no_cli_subprocess.py — SC-001: AST scan asserting no FEAT-011 test body shells out to the `agenttower` CLI for an app.* call (daemon-lifecycle setup via _daemon_helpers is exempt). - T081 test_app_token_redaction.py — SC-008: the opaque app_session_token never appears in events.jsonl or any state-dir log; app_session_id may. - T082 test_app_cli_parity.py — SC-010: route-creation parity, CLI `route add` vs `app.route.add` SQLite rows equal modulo id/timestamps. Agent-dependent methods deferred (same FEAT-006/007 service path the CLI uses). - T083 test_app_error_registry.py — SC-003/021: 27-code closed set, regex, per-code details registry, FR-034a empty-details rule. - T084 test_app_payload_caps.py — SC-023/FR-003a: payload_too_large envelope shape; response stays well under the 8 MiB cap. - T085 test_app_cursor_opacity.py — SC-025/FR-020b: opaque cursor codec, tamper rejection, 250-row contiguous pagination. T086-T088 reconcile-as-done — envelope / view-model / store unit tests already shipped (test_app_contract_smoke.py, test_app_view_models.py, test_app_foundational_stores.py). Docs: T089 architecture.md §19.1 "App Backend Contract (FEAT-011)"; T090 docs/app-contract-client-guide.md (new — client developer guide); T091 CHANGELOG.md (new — FEAT-011 entry). Final sweeps: T092 quickstart Story-1 verified by the story-1 integration test; T093 all 24 checklists 100% ticked; T094 FR-043 verified — FEAT-011 added zero SQLite schema changes and no new persisted secret; constitution principles I-V hold. tasks.md: Phase 8 (T079-T094) marked complete with a checkpoint. Full FEAT-011 + lock-in + 5-story socket-integration suite: 787 passed, 1 documented skip. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…eanup Post-implementation /speckit.analyze surfaced 0 critical / 0 high and 10 artifact-consistency findings (the spec saw Round-4/5/6 corrections that landed inline without regenerating tasks.md). All are doc-only; no code change, no re-test needed (787 tests already green). - F1: contracts/app-methods.md — "30 methods" → 32; the "11 operator mutations" sub-bullet enumerated 13 — corrected with the explicit 2+2+14+1+13=32 breakdown and the 67-entry DISPATCH note. - F2: tasks.md — the wrong "30 app.* methods" echoes (US2 checkpoint, T091, Notes) → 32. - F3: tasks.md — "SC-001..SC-027" → SC-038 (Round-4 added 11 SCs). - F4: tasks.md — T005/T083 "26-entry" → 27; T005 "DETAILS_SCHEMA" → the actual `DETAILS_REQUIRED_KEYS`. - F5: tasks.md Notes — clarified that FEAT-011 tests shipped under tests/unit + tests/integration; the tests/contract/ paths in older task lines are the deferred organizational layout (no coverage gap). - F7: tasks.md — T045/T046 stale post-Round-5/6 text (queue order `created_at`→`enqueued_at`; event/queue filters dropped the non-existent `origin`/`route_id`). - F8: tasks.md — noted the intentional T095/T096 numbering gap. - E1: tasks.md — new "Round-4/5/6 Success-Criteria Coverage Map" retro-mapping SC-028..SC-038 to their verifying tests (the work was implemented inline; this restores the spec↔test mapping). - E2: tasks.md — ticked the 15 done-but-unchecked Setup/Foundational/ US1 tasks (T001, T003, T009, T011–T022) with reconciliation notes on the Phase-2/Phase-3 checkpoints (T003 lint-config moot; T009/T014 gate-chain realized as the server.py wire-framing layer; T016–T018 fixtures inlined per test module). - C1: spec.md FR-014a — documented `check_container_filter` as a reserved-at-v1.0 hint code (not emitted — needs FEAT-003 filter telemetry the MVP doesn't surface). Result: tasks.md has 0 unchecked entries; method counts consistent across spec/contracts/tasks; every SC-001..SC-038 is task- or coverage-map-mapped. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The re-run of /speckit.analyze caught instances of the same F1/F4 count drift that the first remediation pass missed (they were inside SC bodies, a parallel-example block, a dir-tree comment, and the clarify checklists rather than the headline count lines): - spec.md SC-029: "30-method v1.0 surface" → 32-method. - spec.md SC-021: error-code closed set "26-entry" → 27-entry. - plan.md: source-tree comment "app-methods.md (30 methods)" → 32. - tasks.md: Parallel Example task line "DETAILS_SCHEMA" → DETAILS_REQUIRED_KEYS (matches the actual symbol in errors.py). - checklists/cross-artifact-consistency.md: CHK012 "(30 methods)" → 32; CHK013 "26-code closed set" → 27-code. - checklists/clarify-propagation.md: CHK031 "26 entries" → 27. Doc-only; no code change. All core + checklist artifacts now agree: 32 app.* methods, 27-entry error-code closed set. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The clarify-propagation.md "Round-2/3 SC Gap" section was titled "(Outstanding)" but both items under it (CHK032, CHK033) are [X]. Relabel "(Resolved)" so the header matches the item state. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Fixes all findings from the parallel-specialist review of this branch
(1 high, 6 medium, the actionable lows). Full app suite: 791 passed,
1 documented skip; app_contract coverage 94%, mutations.py 82% -> 90%.
Security
- M1: ~32 sites in reads.py/mutations.py/scan_handlers.py interpolated
raw exception strings (`{type(exc).__name__}: {exc}`) into wire
`error.message` / scan-result fields. sqlite3/OSError strings embed
absolute host paths — this leaked them past the dispatcher, which
only sanitizes *raised* exceptions. Added `envelope.internal_error_logged()`:
logs full detail to stderr, returns a generic message. All offending
sites routed through it (mirrors the _wrap_handler / H3 policy).
- M2: app.send_input now enforces an explicit 16 KiB serialized-payload
cap -> validation_failed(field="payload"), instead of riding the
incidental 64 KiB NDJSON line cap.
Correctness
- M3: documented that app.scan.{containers,panes} wait=true returns an
ok:true envelope with state in {completed, failed} (a failed scan is a
terminal result, not a wait failure) — consistent with app.scan.status.
- M4: app.send_input attributed the FEAT-009 sender to the *target*
agent's own row. Now uses a synthetic host-operator AgentRecord
(HOST_OPERATOR_SENTINEL, role="master" — the only permitted sender
role), so sender_agent_id is correctly attributed and the permission
gate evaluates the operator, not the target.
Coverage / tests (H1 + lows)
- mutations.py service-error-mapping arms (QueueServiceError /
RouteError / RegistrationError variants) were largely untested:
added ~30 unit tests covering every mapping arm, the M1 redaction,
M2 cap, and M4 sender identity -> mutations.py 90%.
- Widened the flaky 2.0s wall-clock assertion in the adopt round-trip
test to a 10s CI ceiling (SC-004's 2s stays the documented target).
- token-redaction test now asserts the daemon log file by name.
- renamed test_app_contract_smoke.py -> test_app_contract_foundations.py.
Low (code)
- SessionRegistry.invalidate() now drops the session's idempotency
store (was leaking one stale store per invalidated session).
- readiness "no containers" hint counts only ACTIVE containers.
- audit.py docstring: queue_approved -> queue_message_approved.
Doc consistency
- payload-cap figure: closed-sets.md / error-codes.md / app-methods.md
now disclose the effective 64 KiB enforcement (T097), matching spec.md.
- spec.md FR-029a reason text aligned with contracts/app-methods.md.
- spec.md FR-016a: degraded_scan documented as a reserved-at-v1.0 state.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
A fresh 5-agent orchestrated review of the now-feature-complete branch
(US1-US5 + Phase 8) surfaced these. Fixes the clear, contained ones;
the rest are flagged for the next pass (see below).
HIGH — app_pane_list silently ignored order_by (reads.py).
Every other list handler captures (field, direction) from
_validate_order_by and calls _sort_rows before paginating.
app_pane_list discarded both with `_, _, canonical_order, err = ...`
and never sorted — so `order_by=discovered_at:desc` was validated
and recorded into the cursor but the rows came back in DAO-default
order. Now captures order_field/order_direction and calls _sort_rows
with the FR-021 pane default key tuple (container_name, session_name,
window_index, pane_index).
MEDIUM — idempotency entry created_at_ms was the session birth time
(mutations.py). app_send_input recorded each IdempotencyEntry with
`session.connection_started_at_ms`, so every entry for a session
shared one identical timestamp — useless for diagnostics/LRU
reasoning. Now stamps `int(time.time() * 1000)` at record time.
MEDIUM — idempotency.py docstring overclaimed concurrency safety.
The `record()` docstring asserted "concurrent send_input calls block
per Round-4 Block D Q27". They do not: app_send_input does
lookup -> mutate -> record without holding a lock across the
sequence, so two concurrent calls with the same (session, key) can
both miss the lookup and both enqueue a real queue row (a
check-and-act TOCTOU). Docstring now states the caveat honestly. The
proper fix — a per-(session,key) lock held across the mutation — is
left as tracked follow-up.
HIGH (test) — SC-024 "last-write-wins" was tested single-threaded.
test_agent_update_never_returns_stale_object issued two SEQUENTIAL
app.agent.update calls — it would pass even with no concurrency
handling at all, despite SC-024 mandating a "concurrency stress
fixture running paired updates". Added
test_agent_update_concurrent_last_write_wins: two real threads,
released together via a Barrier, each with its own thread-local
request-peer context; asserts both succeed, neither returns
stale_object, and a final app.agent.detail read shows exactly one
clean label value. The sequential test is kept as the cheap path.
LOW — research.md R-004 stale count: "25 codes" -> "27 codes, 13
registry entries" (matches errors.py / FR-034).
Verified: 252 + 319 FEAT-011 unit tests green; the new concurrency
test passes.
Deferred to the next review pass (flagged, not silently dropped):
- H1 hardening: make the daemon REFUSE to boot under a lone
AGENTTOWER_TEST_FORCE_HOST_PEER (currently only warn-logged, per
commit 0aaeb1e). Proper fix needs `tests/integration/_daemon_helpers.py`
to also set a companion AGENTTOWER_TEST_* var, else the integration
suite breaks — a coordinated change better done deliberately.
- idempotency TOCTOU: the per-(session,key) lock across the mutation.
- {result: {row: ...}} wrapper: handlers wrap detail/mutation view
models under `result.row`; contracts/app-methods.md says `result`
IS the view model. Recommend documenting the wrapper (changing the
handlers would break the whole US3 test suite). Doc-only.
- app-methods.md still has a stale "Idempotent on same connection"
block (FR-008a deprecated it).
- Test depth: SC-027 (unknown_method — one method name, no
state-unchanged assertion), SC-018 (no filter-operator-rejection
test), SC-028 (payload-cap test doesn't assert size_limit_bytes).
- 4 closed-set codes (docker_unavailable, tmux_unavailable,
socket_missing, socket_permission_denied) are never emitted as an
error.code by any handler/test — investigate whether they are
intentionally client-side/scan-only or a genuine coverage gap.
- NEXT_SESSION.md self-contradicts on the code count (26 vs 27) and
has a stale spec-size summary.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This was referenced May 22, 2026
Open
Implement operator-laden filter-value rejection (FR-024a / SC-018):
_validate_filters_object now rejects exact-match filter values carrying
operator syntax (`<>~*%`, SQL LIKE) with validation_failed +
details.field. Adds parametrized contract tests across app.event.list,
app.queue.list, app.route.list.
Strengthen SC-027 (unknown-method) to exercise multiple names and assert
no audit-log leakage; pin SC-028 payload cap to the enforced 65536-byte
limit with a comment on the FR-003a 1 MiB spec/impl gap.
Doc corrections: document the `{result: {row: <vm>}}` single-entity
wrapper in contracts + data-model; delete the deprecated same-connection
idempotency block (FR-008a); fix FR/SC/code count drift in NEXT_SESSION
and research (74 FRs, 38 SCs, 27 codes, 15 registry entries).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…e, view_models None handling, count drift server.py: apply the FR-042 host-only gate BEFORE the unknown-app-method short-circuit. Previously a bench-container peer probing `app.foo` got `unknown_method` while `app.dashboard` got `host_only`, exposing the namespace as an enumeration oracle. Now every `app.*` name (known or not) returns `host_only` to a container peer. New unit test in test_socket_api_framing pins this; the existing SC-027 host-peer test still asserts `unknown_method` via the test seam. view_models._get(): normalize `None` → `default` on the `row[name]` fallback path so sqlite3.Row access matches the dict and getattr paths the docstring already documents. Checklist drift: contract-files.md CHK013 and clarify-propagation.md CHK024 now say "27 codes" (matching FR-034 / errors.py). Co-Authored-By: Claude Opus 4.7 <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
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.
Summary
Session 2026-05-19), plan, research, data-model, 3 contract files, quickstart, task list (94 tasks), and 24 requirements-quality checklists for the localapp.*socket namespace — a host-only, versioned façade over the existing FEAT-002..FEAT-010 service layer for a future packaged desktop control panel.app.preflight→app.hello→app.readiness→app.dashboardare all callable through the existing FEAT-002 dispatcher. New sub-packagesrc/agenttower/app_contract/(11 modules, zero changes to existing FEAT-002..010 modules except a 9-line dispatcher merge).tests/contract/andtests/integration/) is explicitly deferred — seetasks.mdfor the full list.What's in the four commits
247076c2f3bf75aa13224app_contract/package skeleton withversioning,errors(26-entry FR-034 closed set + FR-034adetailsregistry),envelope,sessions,host_only,preflight,hello,dispatchermodules; DISPATCH merge insocket_api/methods.py; 23 smoke tests8cb0c81readiness(6 subsystem probes + hint emission + state aggregation),dashboard(counts across 7 surfaces + recents + hints),view_models(compact builders); 19 more smoke tests; 4 dispatch lock-in tests updated for the new 39-entry tableWhat works end-to-end today
The full Story 1 bootstrap chain is functional:
app.preflightapp.hellocapability_flags = {}, state=ok; major-mismatch guard with both versions indetailsapp.readinesshints[]arrayapp.dashboardhints[]Hints: closed v1.0 registry of 6 codes emitted based on state (
docker_unavailable_hint,start_bench_container,register_first_agent,attach_logs,enable_first_route); severity ∈ {info, warning, action_required}.What's deferred to follow-up PRs
app.scan.{containers,panes,status},app.pane.{list,detail},app.agent.{list,detail,register_from_pane}, plus theScanRegistry+IdempotencyStore.app.agent.update,app.log.attach/detach,app.send_inputwith idempotency,app.queue.{approve,delay,cancel},app.route.{add,remove,update}), shared pagination/ordering/filtering plumbing.app.dashboard— not measured in this PR.tests/contract/andtests/integration/layout — the 42 tests in this PR live undertests/unit/as a smoke suite covering envelope shape, dispatcher wiring, all 4 handlers, and the FR-034/FR-034a closed-set invariants. Splitting them into the structured layout fromplan.md§Project Structure is a follow-up.app.dashboardrecents) shipped here. The full PaneViewModel / AgentViewModel / etc. land with US2/US3.app.scan.statusretention + IdempotencyStore — modules exist as plan artifacts only; no implementation yet.Constitution check
app.*host-only; no new persisted state (sessions / scan registry / idempotency are all in-memory per FR-006 / FR-030c / FR-031a)app.send_input(US3, not in this PR) will ride the FEAT-009 queue + permission gate per FR-031; not exercised in this PR's codeorigin = "app"audit attribution planned via FR-044 / SC-008; smoke suite assertsapp.dashboardis side-effect-free (FR-045)No complexity-tracking entries; no constitution conflicts.
Test plan
PYTHONPATH=src pytest tests/unit/test_app_contract_smoke.py -vshould report 42 / 42 passed. Covers FR-001/002/010/011/012/013/014a/015/016/017/018/033/034/034a/036/039/042/045 invariants.PYTHONPATH=src pytest tests/unit/test_dispatch_table_stability.py tests/unit/test_socket_api_methods.py tests/integration/test_feat005_no_real_container.py -v. DISPATCH is now 39 entries (was 35); EXPECTED_ORDER has 4 newapp.*keys appended.8cb0c81) are NOT introduced by FEAT-011:test_status_default_output_six_lines(FEAT-008/009/010 added status fields; test expects 6 lines, daemon emits 14),test_backcompat_status_json_keeps_feat002_through_008_keys(asserts schema_version 7, FEAT-010 bumped to 8),test_cli_scan_panes_inactive_cascade.test_inactive_container_with_only_inactive_prior_panes_still_touches_them(pre-existing flaky millisecond-timestamp race). These deserve a separate cleanup PR.{"method": "app.preflight"}→ success envelope;{"method": "app.hello"}→ success envelope withcapability_flags = {}andapp_contract_version = "1.0";{"method": "app.readiness", "app_session_token": "<token>"}→ state envelope with 6 subsystem rows;{"method": "app.dashboard", "app_session_token": "<token>"}→ counts/recents/hints envelope. No subprocess invocation required (SC-001 invariant).^[a-z][a-z0-9_]*$; FR-034a per-codedetailsregistry validated byvalidate_details()(12 codes with structured details).tasks.mdare the authoritative scope list for this PR.Reviewer guide
Where to look in this PR:
specs/011-app-backend-contract/spec.md(FRs + SCs + Clarifications Session 2026-05-19)specs/011-app-backend-contract/plan.mdspecs/011-app-backend-contract/contracts/app-methods.mdspecs/011-app-backend-contract/contracts/error-codes.mdspecs/011-app-backend-contract/contracts/closed-sets.mdspecs/011-app-backend-contract/tasks.md(94 tasks; 13 ticked [X])src/agenttower/socket_api/methods.py(last 9 lines)src/agenttower/app_contract/{versioning,errors,envelope,sessions,host_only}.pysrc/agenttower/app_contract/{preflight,hello,readiness,dashboard}.pytests/unit/test_app_contract_smoke.py(42 tests)🤖 Generated with Claude Code
Summary by Sourcery
Introduce the initial FEAT-011
app.*socket namespace contract (specs and in-memory model) and wire its bootstrap/dashboard readiness handlers into the existing dispatcher while extending tests and documentation for the expanded dispatch table and new app contract surface.New Features:
app.*socket namespace withapp.preflight,app.hello,app.readiness, andapp.dashboardhandlers as a façade over the existing service layer.Enhancements:
app.*methods into the existing dispatch table without altering legacy behavior.Documentation:
Tests:
app_contractslice covering envelopes, error invariants, host-only gating, sessions, readiness, dashboard, and versioning behavior.app.*methods and the increased dispatch table size and ordering guarantees.