fix(tests): unwrap the CLI envelope in two E2E test classes

[padak]

What

_json_ok (tests/test_e2e.py) returns the whole {"status": ..., "data": {...}} CLI envelope. Two test classes read it as if it were the payload.

1. TestE2ENotificationSubscriptions — all 7 tests broken since #615

Introduced by ae840ba (#615). The tests index data["subscriptions"], data["errors"] and data["project_wide_excluded"] directly, so every test raised KeyError: 'subscriptions' on every run.

The command itself is fine — verified live, kbagent --json notification list returns a correct envelope with data.subscriptions present.

Worse than a plain failure: the class is deliberately designed so its data-dependent assertions pytest.skip on a project with no subscriptions (see the class docstring on vacuous passes). The KeyError fired before those skip paths could be evaluated, so the class was dead rather than honestly skipped.

Fixed by unwrapping once in a _payload() helper rather than adding ["data"] to each of the 10 call sites — the envelope/payload confusion is exactly what shipped the bug, and a per-call-site ["data"] invites the same slip again. This matches the file's existing convention (_run_ok + explicit ["data"]).

2. test_data_app_secrets_round_trip — three dead security assertions

Found by scanning the rest of the file for the same mistake (AST scan over every _json_ok-derived variable, not an eyeball pass).

The test reached for set_result["raw_output"] / list_result["raw_output"] / get_result["raw_output"] — a key that exists nowhere in the codebase (grep -rn raw_output src/ → 0 hits). Those three assertions state that a plaintext secret must never appear in CLI output. They could only ever raise KeyError, never check anything.

The intent was clearly the command's raw stdout, which _json_ok discards. Fixed by keeping the raw CliRunner result so the assertions inspect .output.

Verification

KBAGENT_E2E_CONFIG_DIR=... KBAGENT_E2E_ALIAS=e2e-snowflake \
  uv run pytest tests/test_e2e.py::TestE2ENotificationSubscriptions -q -rs

Before: 7 failed, 1 passed
After: 5 passed, 3 skipped

The 3 skips are exactly the data-dependent ones the class docstring describes (project 5946 has zero notification subscriptions), confirming the skip paths now actually trigger instead of being masked by the KeyError.

An AST scan of the whole file reports 0 remaining envelope/payload confusions.

Also clean: ruff check, ruff format --check, ty check, and collection of all 115 tests in the file.

Not verified live: test_data_app_secrets_round_trip skips in my environment — besides the token it needs E2E_DATA_APP_GIT_REPO_PUBLIC, which is not set. Static check only: set_data_app_secrets returns key names in secrets_set, never values, so the revived assertions should pass — but they still need a run with that variable set.

Notes

• Tests only, no source changes. No version bump / changelog entry — the version will be bumped once across several PRs.
• These E2E tests need E2E_API_TOKEN / E2E_URL and do not run in CI, which is why the breakage shipped unnoticed.

---

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.