feat: read-only AppFolio, Hostaway, Tenant Turner, and zInspector connectors - #21
NancyBraun wants to merge 5 commits into
Conversation
Add a read-only connector for AppFolio's Reporting (Data) API v2 so agents can pull property data (rent roll, delinquency, work orders, leasing) out of AppFolio. The Reporting API only returns data, so there is no write path. - src/appfolio/api.ts: AppFolioAPI client — HTTP Basic auth (sent as a header, not embedded in the URL), automatic next_page_url pagination with a page cap, one 429 retry honoring Retry-After, bounded timeout. - src/bus/appfolio.ts: fetchAppfolioReport + credential loader that prefers process.env and falls back to orgs/<org>/secrets.env, so it works from both the agent PTY and a plain CLI shell. - src/cli/bus.ts: wire `bus appfolio-report <report>` (--filters/--max-pages/ --max-rows/--rows-only/--org). - tests/unit/bus/appfolio.test.ts: 9 unit tests (auth header, pagination, bare-array shape, caps, 429 retry, auth/404 error mapping). Credentials live in orgs/<org>/secrets.env (gitignored). Verified live against a real account; npm run build and tsc --noEmit are clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a read-only connector for Hostaway's Public API v1 so agents can read vacation-rental data (listings, reservations, calendar, guests, conversations). Read-only by construction: the client only issues HTTP GET and exposes no create/update/delete methods. Hostaway keys are account-level (no read-only key option), so the safety guarantee is that the tool has no write code path. - src/hostaway/api.ts: HostawayAPI — OAuth2 client-credentials token exchange (cached per process), GET-only fetchResource with limit/offset pagination, bounded timeout, status:fail + auth/404 error mapping. - src/bus/hostaway.ts: fetchHostawayResource + credential loader (process.env then orgs/<org>/secrets.env), mirroring the AppFolio connector. - src/cli/bus.ts: wire `bus hostaway-get <resource>` (--query/--max-pages/ --max-rows/--rows-only/--org). - tests/unit/bus/hostaway.test.ts: 8 unit tests (token exchange, GET-only, pagination, token reuse, caps, auth/status-fail errors). Credentials live in orgs/<org>/secrets.env (gitignored). Verified live against a real account; npm run build and tsc --noEmit are clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the real account subdomain used as the illustrative base-URL example with a neutral placeholder (yourcompany/acme.appfolio.com) and genericize the verified-reports heading. No behavior change — the test's input and expected URL change together. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds a read-only Tenant Turner API v1 client, matching the AppFolio and Hostaway connector pattern: - src/tenantturner/api.ts — TenantTurnerAPI (GET-only, Basic base64(apiKey) auth, NextPage cursor pagination with page/row caps, 30s timeout). - src/bus/tenantturner.ts — creds loader (process.env -> secrets.env fallback) and fetchTenantTurnerResource. - src/cli/bus.ts — `bus tenantturner-get <resource> [--since] [--query] [--max-pages] [--max-rows] [--rows-only] [--org]`. - tests + README. The one read endpoint of note is `applications` (requires SinceDateUpdated), which returns the unified lead record: contact, income, acquisition source, pre-screening answers, and a nested Showings[] array. Read-only by construction: no create/update/delete code path (POST /v1/showings is a write and is never called). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
NancyBraun
left a comment
There was a problem hiding this comment.
Code Review — AppFolio / Hostaway / Tenant Turner connectors
Reviewed src/appfolio/api.ts, src/hostaway/api.ts, src/tenantturner/api.ts, src/bus/appfolio.ts, and the three unit test files.
LGTM — approve with minor notes
What's correct:
- All three are read-only by construction. AppFolio: POST only to
/api/v2/reports/(a reporting endpoint that has no write surface). Hostaway and Tenant Turner: GET-only. No code path can mutate data. - Auth credentials go in headers, never in URLs — error messages are sanitized, credential values are not echoed. ✓
- Input validation (regex on report/resource names) blocks path-injection before any network call is made. ✓
AbortSignal.timeout()used correctly (Node 20+). ✓- Pagination has configurable page + row caps — a runaway/huge report can't spin forever. ✓
- Credential loading: process.env first,
secrets.envfallback — correct two-context pattern. ✓ - Unit tests cover: auth header construction, pagination follow-through, bare-array response shape, row/page caps, 429 retry, 401/404 error mapping, path-injection rejection. ✓
Minor items (non-blocking, document or address before merge)
1. Hostaway token has no expiry guard (src/hostaway/api.ts)
this.token is cached for the process lifetime with no TTL check. OAuth client-credentials tokens typically expire in ~1h. CLI invocations are short-lived so this won't bite in practice, but a long-running agent that holds a HostawayAPI instance across a heartbeat cycle could get silent 401s on page 2+. Simple fix: store the expiry from the token response and re-mint if within 60s of expiry.
2. parseEnvFile is duplicated (in bus/appfolio.ts — probably in bus/hostaway.ts and bus/tenantturner.ts too)
This function already exists (or an equivalent) in other bus modules. Nice-to-have: extract to src/utils/env.ts. Not a correctness issue.
3. AppFolio baseUrl — no HTTPS enforcement
If someone passes http:// the credentials go over cleartext. AppFolio's API requires HTTPS. Worth adding: if (!this.baseUrl.startsWith('https://')) throw new Error(...) in the constructor. Low-priority since AppFolio itself would reject the connection, but defense-in-depth.
4. Tenant Turner Basic auth (intentional, but confirm)
Basic ${base64(apiKey)} with no colon — non-standard but matches TT's docs. The comment explains it. Just confirming this worked against a real account per the PR description? If yes, no action needed.
Verdict
No correctness bugs, no security issues with secrets handling. The three non-blocking items are worth addressing but don't need to hold up merge. The PR is stacked on the upstream commits — once that batch lands and the Windows PRs are merged, this one can go in cleanly.
Adds a read-only zInspector API client, matching the AppFolio / Hostaway / Tenant Turner connector pattern: - src/zinspector/api.ts — ZInspectorAPI (GET-only, x-api-key header auth, required trailing-slash resource paths, follows the `next` URL for both cursor- and page-style pagination, page/row caps, 30s timeout). - src/bus/zinspector.ts — creds loader (process.env -> secrets.env fallback, base URL defaulting to portfolio.zinspector.com) and fetchZinspectorResource. - src/cli/bus.ts — `bus zinspector-get <resource> [--query] [--max-pages] [--max-rows] [--rows-only] [--org]`. - tests + README. Read endpoints of note: propertiesCursor, documents, media (photos), process. Read-only by construction: no create/update/delete code path. The key's data access is governed by its Linked User (must be an Admin) in zInspector. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Added two more read-only vendor connectors following the exact same pattern as the AppFolio/Hostaway ones in this PR:
Both are GET-only by construction (no write path), fetch credentials from 🤖 Generated with Claude Code |
|
We opened and reviewed the four connector implementations, including the page caps, bounded timeouts, header-based credentials, AppFolio 429 handling, and the separate AppFolio, Hostaway, Tenant Turner, and zInspector command surfaces. None of that work has been independently reimplemented in our framework. You were explicitly thinking about credential exposure: the AppFolio client puts Basic auth in a header instead of the URL so logs and error messages do not capture it. That closes one real leak path. The adjacent path we found is server-controlled pagination: AppFolio's This work is wanted, but we are not merging the four-connector branch as submitted. The path to yes is explicit: enforce same-origin pagination for every authenticated next URL, then split the connectors into independently reviewable current-main changes so each external API and credential contract has a clear owner. We should have told you that weeks ago instead of leaving the PR unanswered. |
|
This is directly on-mission for us and the construction is right: read-only by design, mirroring the existing propertymeld pattern, pagination caps and retry handling, no new runtime deps, and purely additive (which is why it's still mergeable). Verdict: adopting, through our heaviest review lane. Setting expectations on that review: ~2k lines touching credential loading and external API calls gets a full security pass, not a skim - we'll be verifying there are no write paths anywhere, no secret logging, and that the Thanks for the quality here - live-testing against real accounts before submitting is exactly the standard we hope for. |
What
Adds two read-only connectors for common property-management SaaS platforms, exposed as bus commands so agents can pull operational data directly instead of relying on manual CSV exports:
cortextos bus appfolio-report <report> [--filters json] [--max-pages N] [--max-rows N] [--rows-only] [--org]cortextos bus hostaway-get <resource> [--query json] [--max-pages N] [--max-rows N] [--rows-only] [--org]Mirrors the existing connector pattern (e.g. propertymeld).
Design — read-only by construction
Retry-After(AppFolio), request timeouts.process.envwith asecrets.envfallback, so the same command works from both the agent PTY and a plain CLI. No secrets are committed.Tests
tests/unit/bus/appfolio.test.ts,tests/unit/bus/hostaway.test.ts— auth header construction, base-URL normalization, pagination, and error handling.Files
9 files, additive only (~1075 insertions):
src/appfolio/*,src/hostaway/*,src/bus/{appfolio,hostaway}.ts,src/cli/bus.ts(command wiring), unit tests, and per-connector READMEs.🤖 Generated with Claude Code