From c274f62346a8bfd22b1be02c677fd3dbfecb5552 Mon Sep 17 00:00:00 2001 From: Lukasz Jagiello Date: Mon, 8 Jun 2026 17:28:39 -0700 Subject: [PATCH 1/2] refactor(hubspot-search): rename hubspot example to hubspot-search Rename the example agent (and its directory) from `hubspot` to `hubspot-search` to describe what it does and leave room for other HubSpot examples. Updates the agent id, package name, README/title, register path, test docstrings, and all references in the root docs and .github tooling (CI working-directory, dependabot, issue templates). References to HubSpot the product, HUBSPOT_ACCESS_TOKEN, and the API endpoints are unchanged. No version bump. --- .github/ISSUE_TEMPLATE/bug_report.yml | 2 +- .github/ISSUE_TEMPLATE/feature_request.yml | 2 +- .github/dependabot.yml | 2 +- .github/workflows/ci.yml | 4 ++-- CONTRIBUTING.md | 6 +++--- README.md | 6 +++--- SECURITY.md | 2 +- {hubspot => hubspot-search}/README.md | 4 ++-- {hubspot => hubspot-search}/agent.py | 4 ++-- {hubspot => hubspot-search}/metadata.json | 2 +- {hubspot => hubspot-search}/pyproject.toml | 2 +- {hubspot => hubspot-search}/tests/test_config.py | 2 +- {hubspot => hubspot-search}/tests/test_execute.py | 2 +- {hubspot => hubspot-search}/tests/test_search.py | 2 +- {hubspot => hubspot-search}/uv.lock | 2 +- 15 files changed, 22 insertions(+), 22 deletions(-) rename {hubspot => hubspot-search}/README.md (98%) rename {hubspot => hubspot-search}/agent.py (99%) rename {hubspot => hubspot-search}/metadata.json (95%) rename {hubspot => hubspot-search}/pyproject.toml (94%) rename {hubspot => hubspot-search}/tests/test_config.py (98%) rename {hubspot => hubspot-search}/tests/test_execute.py (99%) rename {hubspot => hubspot-search}/tests/test_search.py (97%) rename {hubspot => hubspot-search}/uv.lock (99%) diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml index dbdc0fb..3830465 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.yml +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -14,7 +14,7 @@ body: attributes: label: Which example? description: The example directory the bug is in. - placeholder: hubspot + placeholder: hubspot-search validations: required: true - type: textarea diff --git a/.github/ISSUE_TEMPLATE/feature_request.yml b/.github/ISSUE_TEMPLATE/feature_request.yml index 2b9e3f7..01a0cde 100644 --- a/.github/ISSUE_TEMPLATE/feature_request.yml +++ b/.github/ISSUE_TEMPLATE/feature_request.yml @@ -30,7 +30,7 @@ body: multiple: true options: - A new example agent - - An existing example (e.g. hubspot) + - An existing example (e.g. hubspot-search) - Repo tooling / CI - Docs / README - Other / not sure diff --git a/.github/dependabot.yml b/.github/dependabot.yml index fdf765b..79db7d2 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -2,7 +2,7 @@ version: 2 updates: # Python dependencies: the root tooling project (ruff) and every example. # "/" is the root uv project (dev tools); the "/*" glob picks up the current - # example (hubspot) and any future top-level example without editing this file. + # example (hubspot-search) and any future top-level example without editing this file. - package-ecosystem: "uv" directories: - "/" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9c4366e..9f58103 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,6 +32,6 @@ jobs: uses: astral-sh/setup-uv@v8.2.0 # Each example is its own uv project; run its pytest suite from its dir. - - name: hubspot + - name: hubspot-search run: uv run --locked pytest - working-directory: hubspot + working-directory: hubspot-search diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f274ca2..8f3ef83 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -34,7 +34,7 @@ git clone https://github.com/friday-platform/agent-sdk-examples cd agent-sdk-examples # Each example is its own uv project: -cd hubspot +cd hubspot-search uv sync ``` @@ -51,7 +51,7 @@ uv run ruff check . uv run ruff format --check . # Tests — from an example directory -cd hubspot && uv run pytest +cd hubspot-search && uv run pytest ``` ## Adding a new example @@ -76,7 +76,7 @@ Then add a row to the **Examples** table in the root [`README.md`](README.md). 5. Follow existing code style — `ruff` ([`ruff.toml`](ruff.toml)). 6. Write a clear commit message in [Conventional Commits](https://www.conventionalcommits.org/) style - (e.g. `feat(hubspot): add pagination`). + (e.g. `feat(hubspot-search): add pagination`). > These examples are unreleased and unversioned — please **don't** bump the > `version` field in `pyproject.toml` / `metadata.json` / the `@agent` decorator. diff --git a/README.md b/README.md index affe17f..d65c88d 100644 --- a/README.md +++ b/README.md @@ -72,7 +72,7 @@ before use — the examples do. ## Quick start ```bash -cd hubspot +cd hubspot-search uv sync # create .venv from the example's uv.lock # Register with a local Friday daemon (it then handles one execute call): @@ -88,7 +88,7 @@ environment variables it needs. | Example | What it shows | | --- | --- | -| [`hubspot`](hubspot) | A **deterministic** agent (no LLM): read config from the prompt, make one authenticated REST call with `ctx.http`, and return a structured result. | +| [`hubspot-search`](hubspot-search) | A **deterministic** agent (no LLM): read config from the prompt, make one authenticated REST call with `ctx.http`, and return a structured result. | More to come — each new example lands as its own top-level directory. @@ -99,7 +99,7 @@ the checks CI runs. Quick version: ```bash uv run ruff check . && uv run ruff format --check . # lint + format (repo root) -cd hubspot && uv run pytest # tests (per example) +cd hubspot-search && uv run pytest # tests (per example) ``` Security issues: see [SECURITY.md](SECURITY.md) — please don't open a public issue. diff --git a/SECURITY.md b/SECURITY.md index 9519169..a22c547 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -27,7 +27,7 @@ the latest state of `main`; there are no versioned releases to back-port to. In scope: -- The example agents in this repository (e.g. `hubspot/`) and their CI/tooling. +- The example agents in this repository (e.g. `hubspot-search/`) and their CI/tooling. Out of scope — please report to the appropriate project: diff --git a/hubspot/README.md b/hubspot-search/README.md similarity index 98% rename from hubspot/README.md rename to hubspot-search/README.md index b84001b..26d9f4e 100644 --- a/hubspot/README.md +++ b/hubspot-search/README.md @@ -1,4 +1,4 @@ -# hubspot +# hubspot-search The first example in this repo, and a good template for a **deterministic** Friday agent: a paginated REST search, no LLM, no tool loop. @@ -26,7 +26,7 @@ local Friday daemon (it connects over NATS and handles one execute call): ```bash curl -X POST http://localhost:8080/api/agents/register \ -H 'Content-Type: application/json' \ - -d '{"path": "'"$(pwd)"'/hubspot/agent.py"}' + -d '{"path": "'"$(pwd)"'/hubspot-search/agent.py"}' ``` ## Input / Output diff --git a/hubspot/agent.py b/hubspot-search/agent.py similarity index 99% rename from hubspot/agent.py rename to hubspot-search/agent.py index 16a6ee6..c4e5914 100644 --- a/hubspot/agent.py +++ b/hubspot-search/agent.py @@ -1,4 +1,4 @@ -"""hubspot — searches HubSpot for new support tickets and returns their IDs. +"""hubspot-search — searches HubSpot for new support tickets and returns their IDs. A deterministic Friday agent (no LLM, no tool loop): it runs the HubSpot CRM v3 ticket search for the configured pipeline stage(s) created within a recent time @@ -431,7 +431,7 @@ def _search( @agent( - id="hubspot", + id="hubspot-search", version="1.0.0", description=( "Searches HubSpot for support tickets in the configured pipeline stage(s) " diff --git a/hubspot/metadata.json b/hubspot-search/metadata.json similarity index 95% rename from hubspot/metadata.json rename to hubspot-search/metadata.json index 657f0d7..139716e 100644 --- a/hubspot/metadata.json +++ b/hubspot-search/metadata.json @@ -1,5 +1,5 @@ { - "id": "hubspot", + "id": "hubspot-search", "version": "1.0.0", "description": "Searches HubSpot for support tickets in the configured pipeline stage(s) created within a recent time window and returns their IDs. Paginated, with retries on rate-limit/5xx. Deterministic REST search — no LLM, no tool loop.", "entrypoint": "agent.py", diff --git a/hubspot/pyproject.toml b/hubspot-search/pyproject.toml similarity index 94% rename from hubspot/pyproject.toml rename to hubspot-search/pyproject.toml index 1e3ff4c..cb36cbb 100644 --- a/hubspot/pyproject.toml +++ b/hubspot-search/pyproject.toml @@ -1,5 +1,5 @@ [project] -name = "friday-hubspot-agent" +name = "friday-hubspot-search-agent" version = "1.0.0" description = "Deterministic HubSpot ticket-search agent — searches the configured pipeline stage(s) for tickets created within a recent window and returns their IDs. Paginated, with retries on rate-limit/5xx." readme = "README.md" diff --git a/hubspot/tests/test_config.py b/hubspot-search/tests/test_config.py similarity index 98% rename from hubspot/tests/test_config.py rename to hubspot-search/tests/test_config.py index c928b58..67f23bd 100644 --- a/hubspot/tests/test_config.py +++ b/hubspot-search/tests/test_config.py @@ -1,4 +1,4 @@ -"""Unit tests for the hubspot agent's pure config-parsing helpers. +"""Unit tests for the hubspot-search agent's pure config-parsing helpers. These functions carry the agent's trickiest logic — pulling a config envelope out of a prompt that may also contain unrelated JSON — so they're worth testing diff --git a/hubspot/tests/test_execute.py b/hubspot-search/tests/test_execute.py similarity index 99% rename from hubspot/tests/test_execute.py rename to hubspot-search/tests/test_execute.py index 5574130..f34e88a 100644 --- a/hubspot/tests/test_execute.py +++ b/hubspot-search/tests/test_execute.py @@ -1,4 +1,4 @@ -"""Integration tests for the hubspot agent's execute() handler. +"""Integration tests for the hubspot-search agent's execute() handler. `@agent` returns the handler unchanged, so we call `execute(prompt, ctx)` directly with a hand-built AgentContext whose HTTP capability is backed by a diff --git a/hubspot/tests/test_search.py b/hubspot-search/tests/test_search.py similarity index 97% rename from hubspot/tests/test_search.py rename to hubspot-search/tests/test_search.py index 92e0a0b..473461f 100644 --- a/hubspot/tests/test_search.py +++ b/hubspot-search/tests/test_search.py @@ -1,4 +1,4 @@ -"""Unit tests for the hubspot agent's request-body builder and result parser.""" +"""Unit tests for the hubspot-search agent's request-body builder and result parser.""" from datetime import UTC, datetime, timedelta from email.utils import format_datetime diff --git a/hubspot/uv.lock b/hubspot-search/uv.lock similarity index 99% rename from hubspot/uv.lock rename to hubspot-search/uv.lock index d02ffd0..1bea5ef 100644 --- a/hubspot/uv.lock +++ b/hubspot-search/uv.lock @@ -24,7 +24,7 @@ wheels = [ ] [[package]] -name = "friday-hubspot-agent" +name = "friday-hubspot-search-agent" version = "1.0.0" source = { virtual = "." } dependencies = [ From 80852e6ad68d58afd4293171398388823c404dd7 Mon Sep 17 00:00:00 2001 From: Lukasz Jagiello Date: Mon, 8 Jun 2026 17:28:48 -0700 Subject: [PATCH 2/2] chore: gitignore docs/ and stop tracking local review notes Code-review notes under docs/ are local working artifacts, not part of the published examples. Ignore docs/ and remove the tracked review file from the repo (it stays on disk locally). --- .gitignore | 3 + docs/reviews/2026-06-08-hubspot-agent.md | 127 ----------------------- 2 files changed, 3 insertions(+), 127 deletions(-) delete mode 100644 docs/reviews/2026-06-08-hubspot-agent.md diff --git a/.gitignore b/.gitignore index 2000d44..845a5e1 100644 --- a/.gitignore +++ b/.gitignore @@ -9,3 +9,6 @@ __pycache__/ *.db .DS_Store .coverage + +# Local-only working docs (e.g. code-review notes); not published with the repo. +docs/ diff --git a/docs/reviews/2026-06-08-hubspot-agent.md b/docs/reviews/2026-06-08-hubspot-agent.md deleted file mode 100644 index 947eb51..0000000 --- a/docs/reviews/2026-06-08-hubspot-agent.md +++ /dev/null @@ -1,127 +0,0 @@ -# Review: hubspot-agent - -**Date:** 2026-06-08 -**Branch:** main (no feature branch; reviewing the `hubspot/` agent + tests as they stand on HEAD `371dc8b`) -**Verdict:** Needs Work → **Resolved** - -> **Update (2026-06-08):** All findings below were fixed — the pagination loop -> now breaks on a no-progress/empty page and an explicit 10k bound; the -> misleading comment is gone; `_retry_after` handles the RFC-7231 HTTP-date -> form; and regression tests cover the empty-page-with-cursor hang and the -> malformed-`Retry-After` fallback (43 tests, all green). The two -> "Needs Decision" items (redundant `response` field, unsurfaced `properties` -> knob) were left for the author and are unchanged. - -## Summary - -The `hubspot` example agent (single-file `friday_agent_sdk` agent: paginated -ticket search with retries and structured errors) is well-architected — clean -separation of pure helpers from I/O, a correct HTTP-boundary test seam, careful -token hygiene, and ~97% test coverage that protects real behavior. One **Critical -correctness bug** blocks merge: the pagination loop can spin forever (verified: -1000+ HTTP calls) when a page returns zero tickets but still advertises a cursor. -Everything else is trim-to-taste or a documented design decision. - -## Critical - -### Pagination loop hangs on an empty page that still has a cursor -**Location:** `hubspot/agent.py:458-471` (loop), false-assurance comment at `455-457` -**Problem:** The loop exits only when `len(tickets) >= limit` or `after is None`. -A page with `results: []` but a non-null `paging.next.after` advances neither -condition → unbounded re-POSTing of the search. -**Evidence:** Verified by direct execution — a stubbed `http_fetch` returning -`{"results": [], "paging": {"next": {"after": "x"}}}` produced **1000+** calls -(harness tripwire), never terminating on its own. Two realistic triggers: (1) a -page whose rows are all malformed/missing `id`, so `_extract_tickets` returns -`[]` while `_next_after` still yields a cursor; (2) HubSpot legitimately -returning a permission/filter-trimmed empty page mid-window. The comment at -`455-457` — *"`limit` is already clamped to the 10k window, so following the -cursor can never page past it"* — is the load-bearing false assumption: -`_MAX_RESULTS` clamps `limit` (`agent.py:218`) but never bounds the loop. -**Recommendation:** Break when a page yields zero new tickets (`if not -page_tickets: break`) — cheapest fix, also handles the legitimate empty-page case -— and/or add an independent iteration cap (`ceil(_MAX_RESULTS / _PAGE_SIZE)` = 50). -Add a regression test (see Tests) and let it drive the fix. -**Worth doing: Yes** — it's a production hang that hammers a rate-limited (~5 req/s) -endpoint; traces directly to this work; fix is ~1 line. - -## Important - -### The 10k search window is not actually bounded; comment over-claims -**Location:** `hubspot/agent.py:218, 455-458` -**Problem:** Clamping `limit` to 10000 doesn't keep the agent inside HubSpot's -10,000-record paging window — the window is on the cursor offset, not the -requested total. At `limit=10000` the final page requests an offset at/near the -boundary, which HubSpot answers with a 400. -**Evidence:** Same root cause as the Critical: "clamped to 10k" is treated as -"safe to page." Degrades to a handled `_error_message` 400 (not a crash), but the -agent returns an error instead of the ~10k results it could have returned. -**Recommendation:** Bound the loop independently (the Critical fix covers this), -clamp `limit` to `_MAX_RESULTS - _PAGE_SIZE`, and delete the "can never page past -it" comment. -**Worth doing: Yes** — bundles into the Critical fix at near-zero extra cost; the -misleading comment is actively harmful to the next reader. - -### `Retry-After` only parses delta-seconds, not the HTTP-date form -**Location:** `hubspot/agent.py:338-346` -**Problem:** RFC 7231 allows `Retry-After` as seconds *or* an HTTP-date. -`float(raw)` handles only seconds; a date raises `ValueError` → `None` → falls -back to backoff. The docstring ("Seconds to wait…") overstates coverage. -**Evidence:** HubSpot sends delta-seconds in practice, so this is a *safe -degradation*, not a live bug. -**Worth doing: No** (fix), **Yes** (one-word docstring honesty + a test for the -fallback branch — see Tests). Cost of not fixing: nil for HubSpot; parsing the -date branch would be speculative generality for this example. - -### Minor speculative surface (demoted — pre-existing, harmless) -**Location:** `hubspot/agent.py:124-132` (snake_case config aliases), `409` -(unreachable "retries exhausted" return) -**Problem:** `_CONFIG_KEYS`/`_pick` accept `pipeline_stages`/`within_minutes` -aliases nothing in this repo emits; line 409 is unreachable. -**Worth doing: No.** The aliases are from the initial port (not this change) and -are harmless tolerance, not the author's mess to clean up here. The line-409 -return is a reasonable total-function guard (keeps control flow obvious to readers -and type-checkers) — I'd *keep* it; deleting it is churn, not simplification. -(This is a deliberate disagreement with the reviewing agent's "delete it" call.) - -## Tests - -Verdict from the test lens: **Solid.** Mocking is at the correct seam -(`http_fetch` only), assertions are on agent behavior (returned `OkResult`/ -`ErrResult` data + the decoded outbound request) not mock internals, the `_sleep` -monkeypatch isolates retry logic without hiding it (one test captures slept values -to prove `Retry-After` is honored), ~1:1 test-to-impl ratio, ~97% line coverage. -The `before/after` epoch-ms window assertions are robust (bounded by captured -timestamps, not fixed tolerances), and the unit/integration overlap is intentional -layering. Two gaps: - -1. **No test for the empty-page-with-cursor case** (`test_execute.py` pagination - section). Every pagination test returns ≥1 result per page, so the suite - structurally cannot catch the Critical hang above. **Worth doing: Yes** — this - is the highest-value missing test; write it first (assert a bounded - `fetch.call_count` / use a `side_effect` list that raises if exceeded) and let - it drive the loop fix. -2. **Malformed/absent `Retry-After` fallback untested** (`agent.py:344-346` - uncovered). A regression to `int(raw)` or removing the `except` would convert a - transient 429 into a hard failure, uncaught. **Worth doing: Yes** — cheap: 429 - with `Retry-After: "soon"` then success, assert the slept value lands in the - `_backoff(0)` range (`0.5–1.0s`), locking both the parse branch and the `or` - fallback. - -Not worth adding: a standalone `limit == 200` boundary test (already bracketed by -`test_page_size_capped_at_200` + `test_limit_caps_total_across_pages`) or a direct -`_backoff` jitter test (would border on testing `random.uniform`). - -## Needs Decision - -1. **Redundant `response` field** (`agent.py:478-485`). The result carries the IDs - three ways: `response` (a JSON *string* of the IDs), `ticketIds` (the same list), - and `tickets` (their source dicts); `count` is derivable too. If the Friday host - contract requires a top-level string `response`, keep it and say so in the - docstring; if not, drop `response` and keep the structured fields. Pre-existing - from the initial port — author's call. -2. **`properties` config is plumbed but never surfaced** (`agent.py:218` → - `_extract_tickets` only emits `id`/`createdAt`/`pipelineStage`). A caller setting - `"properties": ["subject"]` pays for the larger HubSpot response but never sees - `subject`. Either thread requested properties into the output `tickets`, or drop - the config knob to avoid the trap.