Skip to content

Fix stale timeout message and deduplicate duration parsing#1

Open
TadiMadz wants to merge 1 commit into
b-nnett:mainfrom
TadiMadz:fix/small-maintainability-improvements
Open

Fix stale timeout message and deduplicate duration parsing#1
TadiMadz wants to merge 1 commit into
b-nnett:mainfrom
TadiMadz:fix/small-maintainability-improvements

Conversation

@TadiMadz

@TadiMadz TadiMadz commented Jun 2, 2026

Copy link
Copy Markdown

What

Two small maintainability fixes, no behaviour changes.

1. CodexEmbeddedAuth.swift — timeout message now derived from its constant

The error thrown when device-code login times out hardcoded the string "15 minutes" rather than reading from maxDeviceCodeWaitSeconds. If the constant were ever bumped the message would silently show the wrong value.

Before:

throw CodexSelfContainedAuthError.invalidResponse("Device code login timed out after 15 minutes.")

After:

throw CodexSelfContainedAuthError.invalidResponse("Device code login timed out after \(maxDeviceCodeWaitSeconds / 60) minutes.")

2. GooseAppModel.swift — deduplicate env-var / CLI-flag duration parsing

The same ProcessInfo lookup pattern (check env var → check CLI flag → return default) was copy-pasted verbatim four times for the four auto-start capture durations. A fix or edge case would have to be applied to all four independently.

Extracted into a private nonisolated static func durationFromEnvironment(envVar:cliPrefix:fallback:). Each call site is now a single self-documenting expression. Behaviour is identical.

Test plan

  • Build succeeds (no API changes, pure refactor + string fix)
  • Device-code login timeout surfaces the correct duration in the error message if maxDeviceCodeWaitSeconds is changed
  • Auto-start capture durations resolve correctly from env vars and CLI flags as before

CodexEmbeddedAuth: the device-code timeout error message hardcoded "15
minutes" instead of deriving the value from maxDeviceCodeWaitSeconds.
If the constant were ever changed the message would silently lie.

GooseAppModel: the environment-variable + CLI-flag parsing pattern for
capture durations was copy-pasted four times. Extract it into a private
nonisolated static helper (durationFromEnvironment) so the logic lives
in one place and each call site reads as self-documenting data.
@KingPsychopath

Copy link
Copy Markdown

+1 works for me

goamorim added a commit to goamorim/goose that referenced this pull request Jun 3, 2026
tigercraft4 referenced this pull request in tigercraft4/goose Jun 3, 2026
tigercraft4 referenced this pull request in tigercraft4/goose Jun 3, 2026
…osure

- main.py: remove user-supplied body.device from log entry (log injection)
- main.py: return type(exc).__name__ instead of str(exc) (stack trace exposure)
- daily.py: remove device_id from warning log entry (log injection)

Fixes CodeQL alerts #1, #2, #3
tigercraft4 referenced this pull request in tigercraft4/goose Jun 5, 2026
tigercraft4 referenced this pull request in tigercraft4/goose Jun 5, 2026
…osure

- main.py: remove user-supplied body.device from log entry (log injection)
- main.py: return type(exc).__name__ instead of str(exc) (stack trace exposure)
- daily.py: remove device_id from warning log entry (log injection)

Fixes CodeQL alerts #1, #2, #3
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.

2 participants