Fix/401 404 exit by alexphelps · Pull Request #33 · NextCommerceCo/theme-kit

alexphelps · 2026-07-24T10:47:23Z

New Features

Read the API key from the NTK_APIKEY environment variable.
Log the NEXT Theme Kit version on every command.

Improvements

Retry failed requests and space them to respect the rate limit.
Add a request timeout so commands never hang.
Log clear warnings when a request times out, fails, or is throttled.
Exit non-zero with a clear message when a command fails.

Bug Fixes

Show a clear error and exit non-zero on auth, connection, and throttle failures.
Keep ntk watch running on transient errors, stop it on permanent ones.

Closes #32
Closes #30
Closes #29

This reverts commit 30f3175.

codecov · 2026-07-24T10:47:54Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

kilo-code-bot · 2026-07-24T10:49:05Z

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Incremental review of commit 5e85fe49 (patch 10/10) on top of the prior review base 6424c9f6. Only ntk/conf.py and tests/test_config.py changed.

The previously reported WARNING on ntk/conf.py:135 (first env-only run writing apikey: None to config.yml) is fixed in this patch: 'apikey' is now only added to new_config when apikey is truthy, so a first-time NTK_APIKEY run no longer persists apikey: null. The new test test_write_config_omits_apikey_on_first_env_only_run directly covers this regression. No new issues introduced by the incremental change.

Files Reviewed (2 files changed since previous review)

ntk/conf.py - 0 issues (patch 10/10: guarded apikey write to avoid apikey: None on env-only first run — resolves previous WARNING on line 135)
tests/test_config.py - 0 issues (patch 10/10: new test for env-only first-run write)

Previous Review Findings (unchanged files, still active inline comments)

ntk/gateway.py — CRITICAL (unbounded recursion on persistent ConnectionError)
ntk/gateway.py — WARNING (narrow exception class)
ntk/gateway.py — WARNING (every request unconditionally waits 400ms; first request sleeps too)
ntk/decorator.py — WARNING (library code calling sys.exit)
ntk/decorator.py — WARNING (generic 404 message masks real cause)
ntk/decorator.py — SUGGESTION (order status-code checks consistently)
ntk/command.py — WARNING (NTKAuthError swallowed in ntk watch)
ntk/command.py — WARNING (push return False short-circuits remaining files with no diagnostic)
ntk/command.py — WARNING (generic NTKError discards the failing file path)

Fix these issues in Kilo Cloud

Previous Review Summaries (7 snapshots, latest commit 6424c9f)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit `6424c9f`)

Status: 2 Issues Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	0
WARNING	2
SUGGESTION	0

Issue Details (click to expand)

WARNING

File	Line	Issue
`ntk/command.py`	94	`return False` on first non-OK response short-circuits the rest of `parser.filenames` and surfaces no information about which file failed or how many already succeeded. No resume path for partial pushes.
`ntk/command.py`	194	`NTKError('Push failed, see the error above.')` is generic and discards the underlying file name; the decorator's "error above" also doesn't identify the failing file, so the user cannot tell which of `parser.filenames` triggered the failure.

Files Reviewed (2 files changed since previous review)

ntk/command.py - 2 issues (patch 9/9: error on failed file uploads)
tests/test_command.py - 0 issues (patch 9/9: new test for the push-failure path)
README.md - 0 issues (patch 8/9: docs-only NTK_APIKEY tip)

Previous Review Findings

The previously reported WARNINGs on ntk/gateway.py:12, ntk/gateway.py:36, and ntk/conf.py:135 are in unchanged files and remain active as the existing inline comments indicate. The ntk/command.py auth-error swallowing issue reported in earlier reviews is fixed in the current code (only NTKRequestError is now caught in _handle_files_change).

Fix these issues in Kilo Cloud

Previous review (commit `58b157d`)

Status: No Issues Found | Recommendation: Merge

Overview

Severity	Count
CRITICAL	0
WARNING	0
SUGGESTION	0

Files Reviewed (1 file)

tests/test_main.py - 0 issues (incremental diff since previous review covers only the theme_kit_version fallback test)

Previous Review Findings

The incremental diff (5e18c11..58b157d) only changes tests/test_main.py (8 added lines). The previously reported WARNINGs on ntk/gateway.py:12, ntk/gateway.py:36, and ntk/conf.py:135 are in unchanged files and remain active as the existing inline comments indicate. The ntk/command.py auth-error swallowing issue reported in earlier reviews is fixed in the current code.

Fix these issues in Kilo Cloud

Previous review (commit `5e18c11`)

Status: 3 Issues Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	0
WARNING	3
SUGGESTION	0

Issue Details (click to expand)

WARNING

File	Line	Issue
`ntk/gateway.py`	12	`REQUEST_INTERVAL_SECONDS` (400ms) is still applied unconditionally at the top of the retry loop — adds ~`0.4 * N` seconds of artificial delay to bulk `ntk push` and 400ms of latency to every other gateway call. (Re-verified — issue from previous review remains.)
`ntk/gateway.py`	36	`time.sleep(REQUEST_INTERVAL_SECONDS)` still runs on attempt 0 inside the `for attempt in range(MAX_RETRIES)` loop, so every gateway call — including the very first attempt — pays a 400ms tax. (Re-verified — issue from previous review remains.)
`ntk/conf.py`	135	First-run with `NTK_APIKEY` and no prior `config.yml` writes `apikey: None` to disk, silently breaking subsequent runs once the env var is unset. The existing env-key test only covers the case where a config.yml with a key already exists. (New finding from patch 6/6.)

Issue Resolved in This Update

ntk/command.py — _handle_files_change now catches only NTKRequestError, so permanent NTKAuthError / NTKNotFoundError propagate and stop the watcher as intended. The previous WARNING about swallowing NTKAuthError is no longer applicable.

Files Reviewed (6 files changed since previous review)

ntk/__main__.py - 0 new issues
ntk/command.py - 0 new issues (previous auth-error swallowing issue is fixed)
ntk/conf.py - 1 new issue
ntk/exceptions.py - 0 issues
ntk/gateway.py - 0 new issues (previous REQUEST_INTERVAL_SECONDS issues re-verified, still present)
tests/test_command.py, tests/test_config.py, tests/test_gateway.py, tests/test_main.py - 0 new issues

Fix these issues in Kilo Cloud

Previous review (commit `5e5e1b9`)

Status: 3 Issues Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	0
WARNING	3
SUGGESTION	0

Issue Details (click to expand)

WARNING

File	Line	Issue
`ntk/gateway.py`	12	400ms `REQUEST_INTERVAL_SECONDS` is applied unconditionally, adding ~`0.4 * N` seconds of artificial delay to bulk `ntk push` and 400ms of latency to every other gateway call.
`ntk/command.py`	59	`_handle_files_change` swallows `NTKAuthError` (and `NTKNotFoundError`) as if they were transient — the watcher will keep logging the same permanent error on every subsequent file change.
`ntk/gateway.py`	36	`time.sleep(REQUEST_INTERVAL_SECONDS)` runs on attempt 0, so even the first request pays a 400ms tax.

Files Reviewed (8 files)

ntk/__main__.py - 0 new issues
ntk/command.py - 1 issue
ntk/decorator.py - 0 new issues (previous 401/404, sys.exit, and ordering issues are resolved)
ntk/exceptions.py - 0 issues (new file)
ntk/gateway.py - 2 issues
tests/test_command.py - 0 issues
tests/test_gateway.py - 0 issues
tests/test_main.py - 0 issues (new file)

Fix these issues in Kilo Cloud

Previous review (commit `1314048`)

Status: 5 Issues Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	1
WARNING	3
SUGGESTION	1

Issue Details (click to expand)

CRITICAL

File	Line	Issue
`ntk/gateway.py`	20	Unbounded recursion on persistent `ConnectionError` — will stack-overflow instead of reporting a network failure cleanly.

WARNING

File	Line	Issue
`ntk/gateway.py`	19	Only `ConnectionError` is caught; `Timeout`/`SSLError`/`ChunkedEncodingError` still propagate.
`ntk/decorator.py`	42	`sys.exit(1)` from library decorator kills callers (notably `ntk watch`) and prevents unit-testing.
`ntk/decorator.py`	46	Generic 404 message can't distinguish missing-theme from missing-endpoint / wrong store URL.

SUGGESTION

File	Line	Issue
`ntk/decorator.py`	40	Fold 401/404 into the existing error-format branch instead of short-circuiting with `sys.exit`.

Files Reviewed (3 files)

ntk/gateway.py - 2 issues
ntk/decorator.py - 3 issues
README.md - 0 issues (doc-only revert, no code defects)

Fix these issues in Kilo Cloud

Previous review (commit `cd3bf7d`)

Status: 5 Issues Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	1
WARNING	3
SUGGESTION	1

Issue Details (click to expand)

CRITICAL

File	Line	Issue
`ntk/gateway.py`	20	Unbounded recursion on persistent `ConnectionError` — will stack-overflow instead of reporting a network failure cleanly.

WARNING

File	Line	Issue
`ntk/gateway.py`	19	Only `ConnectionError` is caught; `Timeout`/`SSLError`/`ChunkedEncodingError` still propagate.
`ntk/decorator.py`	42	`sys.exit(1)` from library decorator kills callers (notably `ntk watch`) and prevents unit-testing.
`ntk/decorator.py`	46	Generic 404 message can't distinguish missing-theme from missing-endpoint / wrong store URL.

SUGGESTION

File	Line	Issue
`ntk/decorator.py`	40	Fold 401/404 into the existing error-format branch instead of short-circuiting with `sys.exit`.

Files Reviewed (3 files)

ntk/gateway.py - 2 issues
ntk/decorator.py - 3 issues
README.md - 0 issues (doc-only revert, no code defects)

Fix these issues in Kilo Cloud

Previous review (commit `55f4eff`)

Status: 5 Issues Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	1
WARNING	3
SUGGESTION	1

Issue Details (click to expand)

CRITICAL

File	Line	Issue
`ntk/gateway.py`	20	Unbounded recursion on persistent `ConnectionError` — will stack-overflow instead of reporting a network failure cleanly.

WARNING

File	Line	Issue
`ntk/gateway.py`	19	Only `ConnectionError` is caught; `Timeout`/`SSLError`/`ChunkedEncodingError` still propagate.
`ntk/decorator.py`	42	`sys.exit(1)` from library decorator kills callers (notably `ntk watch`) and prevents unit-testing.
`ntk/decorator.py`	46	Generic 404 message can't distinguish missing-theme from missing-endpoint / wrong store URL.

SUGGESTION

File	Line	Issue
`ntk/decorator.py`	40	Fold 401/404 into the existing error-format branch instead of short-circuiting with `sys.exit`.

Files Reviewed (3 files)

ntk/gateway.py - 2 issues
ntk/decorator.py - 3 issues
README.md - 0 issues (doc-only revert, no code defects)

Fix these issues in Kilo Cloud

_{Reviewed by minimax-m3 · Input: 32.7K · Output: 2.1K · Cached: 200.8K}

alexphelps added 2 commits July 24, 2026 16:14

Revert "improve readme"

5de4d61

This reverts commit 30f3175.

improve error handling

55f4eff

kilo-code-bot Bot reviewed Jul 24, 2026

View reviewed changes

Comment thread ntk/gateway.py Outdated

Comment thread ntk/gateway.py Outdated

Comment thread ntk/decorator.py Outdated

Comment thread ntk/decorator.py Outdated

Comment thread ntk/decorator.py

alexphelps added 2 commits July 24, 2026 17:52

revert readme changes

cd3bf7d

improve approach

1314048

next-devin mentioned this pull request Jul 29, 2026

Add JSON output, validation, and screenshot capture #34

Closed

improve request timeout and error handling

5e5e1b9

kilo-code-bot Bot reviewed Jul 29, 2026

View reviewed changes

Comment thread ntk/gateway.py

Comment thread ntk/command.py Outdated

Comment thread ntk/gateway.py

add support for api key as env variable

5e18c11

kilo-code-bot Bot reviewed Jul 29, 2026

View reviewed changes

Comment thread ntk/conf.py

alexphelps added 3 commits July 29, 2026 16:03

improve test coverage

58b157d

add tip for NTK_APIKEY support

1fe2f4a

error on failed file uploads

6424c9f

kilo-code-bot Bot reviewed Jul 29, 2026

View reviewed changes

Comment thread ntk/command.py

Comment thread ntk/command.py

improve api key handling in config.yml

5e85fe4

alexphelps merged commit 25f60ab into master Jul 29, 2026
16 checks passed

Uh oh!

Conversation

alexphelps commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

New Features

Improvements

Bug Fixes

Uh oh!

codecov Bot commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kilo-code-bot Bot commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Summary

Previous review (commit 6424c9f)

Overview

WARNING

Previous review (commit 58b157d)

Overview

Previous review (commit 5e18c11)

Overview

WARNING

Previous review (commit 5e5e1b9)

Overview

WARNING

Previous review (commit 1314048)

Overview

CRITICAL

WARNING

SUGGESTION

Previous review (commit cd3bf7d)

Overview

CRITICAL

WARNING

SUGGESTION

Previous review (commit 55f4eff)

Overview

CRITICAL

WARNING

SUGGESTION

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

alexphelps commented Jul 24, 2026 •

edited

Loading

codecov Bot commented Jul 24, 2026 •

edited

Loading

kilo-code-bot Bot commented Jul 24, 2026 •

edited

Loading

Previous review (commit `6424c9f`)

Previous review (commit `58b157d`)

Previous review (commit `5e18c11`)

Previous review (commit `5e5e1b9`)

Previous review (commit `1314048`)

Previous review (commit `cd3bf7d`)

Previous review (commit `55f4eff`)