Skip to content

fix(apify): correct token URL, poll run --wait, add live --max-cost watchdog, reachable abort#1412

Open
michegz wants to merge 3 commits into
mvanhorn:mainfrom
michegz:amend/apify-auth-url-wait-cost-abort
Open

fix(apify): correct token URL, poll run --wait, add live --max-cost watchdog, reachable abort#1412
michegz wants to merge 3 commits into
mvanhorn:mainfrom
michegz:amend/apify-auth-url-wait-cost-abort

Conversation

@michegz

@michegz michegz commented Jun 30, 2026

Copy link
Copy Markdown

Summary

Four fixes surfaced by dogfooding apify-pp-cli against a real Actor (apify/facebook-marketplace-scraper), including a cost-safety gap where a --wait run reached $2.63 before it could be manually aborted.

Findings

ID Type Fix
F1 bug auth setup/doctor pointed to an MDN page; now points to https://console.apify.com/settings/integrations (the correct URL was already present in the spec's auth prose)
F2 bug run --wait sent waitForFinish=60 on the start POST, exceeding the 30s HTTP client read timeout → "context deadline exceeded while reading body" while the run kept billing. Now caps the server-side block under the client timeout and relies on the existing GET poll loop
F3 bug --max-cost could not protect a first run (no cached history to project from) and nothing capped a live run. Added a live watchdog: under --wait, polls usageTotalUsd and aborts the run once cost exceeds the cap
F4 bug actor-runs abort <runId> only printed parent help; now accepts a run id directly and delegates to the actor-run-post leaf

Changes


Verification

  • go build, go vet, govulncheck, manifest, phase5: PASS
  • Behaviorally verified: auth setup prints the Apify console URL; actor-runs abort [runId] accepts a run id; run --wait polls under a bounded server block

Note: a pre-existing verify-skill canonical-sections drift in SKILL.md (untouched by this PR — the CLI was generated on press v4.8.0) remains and needs a regen, separate from these fixes.

Patch record

.printing-press-patches/auth-url-wait-cost-watchdog-abort.json — includes a deferred_to_upstream entry noting F1 is also a generator auth-URL-extraction heuristic bug worth fixing at the machine level.

…atchdog, reachable abort

Four fixes from dogfooding apify-pp-cli against apify/facebook-marketplace-scraper:
- F1: auth setup/doctor pointed to an MDN page; now https://console.apify.com/settings/integrations
- F2: run --wait sent waitForFinish=60 past the 30s client timeout; now polls under a bounded server block
- F3: live --max-cost watchdog aborts a --wait run once reported cost exceeds the cap (protects first runs with no history)
- F4: actor-runs abort <runId> now works directly instead of only printing parent help
@greptile-apps

greptile-apps Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes four bugs surfaced by dogfooding the apify-pp-cli against a real Actor: corrects the API-token URL from MDN to the Apify console across all auth-hint sites (F1), caps the waitForFinish server-side block to stay under the HTTP client timeout so run --wait no longer deadlines mid-body (F2), adds a live cost watchdog that polls usageTotalUsd and aborts the Actor once it crosses --max-cost (F3), and makes actor-runs abort <runId> work directly instead of printing parent help (F4).

  • F1 (URL fix): Seven files updated consistently; MDN reference replaced with https://console.apify.com/settings/integrations in auth, doctor, helpers, mcp/tools, manifest, and tools-manifest.
  • F2 + F3 (run.go): waitForFinish is now int(flags.timeout.Seconds()) - 5 (clamped ≥ 0), preventing the POST from blocking past the client read deadline; pollRunUntilTerminal gains a maxCost parameter and aborts the run once usageTotalUsd > maxCost, with terminal-status checked first to avoid spurious aborts on naturally-finishing runs.
  • F4 (actor-runs_abort.go): Parent abort command now accepts a runId positional arg and delegates to the child actor-run-post RunE; --gracefully is forwarded by explicitly calling child.Flags().Set() before delegation, which correctly updates the pflag-bound closure variable.

Confidence Score: 5/5

Safe to merge; all four fixes are targeted and well-reasoned, with no regressions introduced in the shared poll loop.

The four bug fixes are narrow and correct: the URL replacements are mechanical, the waitForFinish cap removes a real client-timeout failure, the cost watchdog checks terminal status before cost (avoiding spurious aborts), and the abort shorthand leverages pflag pointer-binding to forward --gracefully correctly. The only open items are two style-level observations — aborted runs not being recorded in local history, and waitSecs not capped at 60 for unusually large HTTP client timeouts — neither of which affects correctness for the common default-30s-timeout case.

run.go warrants the most attention: the waitSecs formula and the cost-watchdog abort path. actor-runs_abort.go is also worth a second read to confirm the pflag pointer behaviour matches expectations.

Important Files Changed

Filename Overview
library/developer-tools/apify/internal/cli/run.go Core run logic: fixes F2 (waitForFinish cap) and F3 (live cost watchdog). Terminal-status check correctly precedes cost check; abort path well-structured. Aborted runs are not recorded in local DB history (early return skips RecordActorRun), meaning cost-cap events don't inform future projections.
library/developer-tools/apify/internal/cli/actor-runs_abort.go Fix F4: adds direct runId argument to the abort command. The --gracefully forwarding via child.Flags().Set() + pflag pointer-binding works correctly since the child RunE uses closure variables, not cmd.Flags(). --stdin is intentionally not forwarded.
library/developer-tools/apify/internal/cli/auth.go Fix F1: corrects two occurrences of the MDN auth URL to the Apify console integrations page. Straightforward and correct.
library/developer-tools/apify/internal/cli/helpers.go Fix F1: updates three auth-error hint URLs (HTTP 400/401/403 paths) from MDN to the Apify console. All occurrences across the error switch are updated consistently.
library/developer-tools/apify/internal/cli/doctor.go Fix F1: corrects auth_key_url in the doctor report output. Single-line URL replacement.
library/developer-tools/apify/internal/mcp/tools.go Fix F1 + key_url: updates four auth hint URLs (400/401/403 errors and context handler key_url) from MDN to Apify console. All occurrences consistently updated.
library/developer-tools/apify/internal/cli/ab.go Adds maxCost=0 to the pollRunUntilTerminal call — disables the cost watchdog for ab-style runs, which is correct since they have no --max-cost flag.
library/developer-tools/apify/internal/cli/workflow_run.go Adds maxCost=0 to pollRunUntilTerminal for workflow step execution — correct, workflow steps have no user-facing --max-cost flag.
library/developer-tools/apify/manifest.json Fix F1: corrects the APIFY_TOKEN credential description URL from MDN to the Apify console.
library/developer-tools/apify/tools-manifest.json Fix F1: updates key_url in auth section and the [More info] href in the API description from MDN to the Apify console. Both in-description occurrences corrected.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant User
    participant CLI as apify-pp-cli
    participant Apify as Apify API

    User->>CLI: run --wait --max-cost 1.00 actor

    Note over CLI: waitSecs = int(flags.timeout.Seconds()) - 5

    CLI->>Apify: "POST /v2/acts/actor/runs?waitForFinish=waitSecs"
    Apify-->>CLI: RunData status RUNNING (after waitSecs)

    loop pollRunUntilTerminal every 5s until deadline
        CLI->>Apify: GET /v2/actor-runs/runId
        Apify-->>CLI: RunData with status and usageTotalUsd

        alt isTerminalStatus
            CLI->>User: return RunData
        else usageTotalUsd exceeds maxCost
            CLI->>Apify: POST /v2/actor-runs/runId/abort
            Apify-->>CLI: abort ACK
            CLI->>User: error cost exceeded cap aborted run
        else context done or deadline passed
            CLI->>User: error timeout or context cancelled
        end
    end
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant User
    participant CLI as apify-pp-cli
    participant Apify as Apify API

    User->>CLI: run --wait --max-cost 1.00 actor

    Note over CLI: waitSecs = int(flags.timeout.Seconds()) - 5

    CLI->>Apify: "POST /v2/acts/actor/runs?waitForFinish=waitSecs"
    Apify-->>CLI: RunData status RUNNING (after waitSecs)

    loop pollRunUntilTerminal every 5s until deadline
        CLI->>Apify: GET /v2/actor-runs/runId
        Apify-->>CLI: RunData with status and usageTotalUsd

        alt isTerminalStatus
            CLI->>User: return RunData
        else usageTotalUsd exceeds maxCost
            CLI->>Apify: POST /v2/actor-runs/runId/abort
            Apify-->>CLI: abort ACK
            CLI->>User: error cost exceeded cap aborted run
        else context done or deadline passed
            CLI->>User: error timeout or context cancelled
        end
    end
Loading

Reviews (3): Last reviewed commit: "fix(apify): forward --gracefully through..." | Re-trigger Greptile

Comment thread library/developer-tools/apify/internal/cli/run.go Outdated
Comment thread library/developer-tools/apify/internal/cli/actor-runs_abort.go
Addresses Greptile review on mvanhorn#1412: the live --max-cost watchdog evaluated
cost before terminal status, so a run that finished naturally with final cost
just over the cap got a spurious abort + false 'aborted to stop billing' error.
Terminal status is now checked first; the watchdog only fires on a still-running
Actor.
@michegz

michegz commented Jun 30, 2026

Copy link
Copy Markdown
Author

Thanks @greptile-apps — good catch on the watchdog ordering. Fixed in 1e2b0a6: isTerminalStatus is now checked before the cost watchdog in pollRunUntilTerminal, so a run that completes naturally with a final cost just over the cap returns its successful result instead of triggering a no-op abort against an already-finished run. The watchdog now only fires on a still-running Actor. Build + vet green.

On the secondary note (F4 --gracefully not forwarded to the abort <runId> shorthand): intentional — the shorthand is a fast-stop convenience; the full actor-runs abort actor-run-post <runId> --gracefully path remains for graceful aborts.

@greptile-apps

greptile-apps Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

The ordering fix in 1e2b0a6 looks correct — terminal check first eliminates the race where a naturally-completed run with a final cost tick over the cap would hit a 404/no-op abort and potentially surface a confusing error to the user.

On --gracefully: makes sense as a design choice. The shorthand communicates "stop now" semantics; users who need a clean drain have the explicit path. Appreciate the clarification.

Addresses Greptile P2 on mvanhorn#1412: the abort <runId> shorthand delegated to the
child RunE without the child's --gracefully flag, so 'abort --gracefully <id>'
failed with 'unknown flag'. The parent now registers --gracefully and forwards
it to the child's bound flag, making the shorthand equivalent to the leaf form.
@michegz

michegz commented Jun 30, 2026

Copy link
Copy Markdown
Author

Ready for maintainer review whenever you have a moment 🙏

Quick status: CI is green, mergeStateStatus: CLEAN / MERGEABLE, Greptile reviewed at 5/5 "safe to merge," and all review threads are resolved (the cost-watchdog ordering and the --gracefully shorthand were both addressed). No outstanding changes on my end.

Scope is four dogfood-surfaced fixes to apify-pp-cli: correct token URL (Apify console vs MDN), run --wait no longer exceeds the client read timeout, a live --max-cost watchdog that protects first runs, and a directly-reachable actor-runs abort <runId>. Patch record + rationale are in .printing-press-patches/auth-url-wait-cost-watchdog-abort.json. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant