[codex] fix Claude auth probe and state sync locking#441
Conversation
Adding .gitkeep for PR creation (default mode). This file will be removed when the task is complete. Issue: ProverCoderAI#439
docker-git auth claude login created and persisted the OAuth token, then ran a 'claude -p ping' probe and hard-failed (exit 1) on any non-zero probe exit, discarding an otherwise successful login. Transient probe failures (network, rate limit, token propagation delay) must not invalidate a saved token. The probe failure is now logged as a warning, mirroring authClaudeStatus. Adds a regression test asserting the token is persisted even when the probe returns non-zero. Fixes ProverCoderAI#439
Reject directory-valued docker-compose.gpu.yml before constructing docker compose arguments. Proof of fix: tests/docker-git/controller-compose.test.ts failed before the stat check and now passes with the new directory-as-overlay regression.
Retry transient Bun dependency installation failures in CI setup before failing the job. Proof of fix: CI job 84095018943 failed while downloading @effect/platform during bun install; local shell syntax and bun install --frozen-lockfile both pass after adding bounded retries.
📝 WalkthroughWalkthroughДобавлен пакет ChangesClaude OAuth login flow and supporting package
Claude auth status API and routing
Compose overlay validation in controller
Infrastructure and support changes
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant caller
participant runClaudeLoginFlow
participant token store
participant probe
caller->>runClaudeLoginFlow: capture token
runClaudeLoginFlow->>token store: persistToken()
runClaudeLoginFlow->>token store: normalizeStoredCredentials()
runClaudeLoginFlow->>probe: probeToken()
probe-->>runClaudeLoginFlow: exit code
runClaudeLoginFlow->>token store: syncState()
runClaudeLoginFlow-->>caller: result
sequenceDiagram
participant client
participant http.ts
participant readClaudeAuthStatus
participant controller storage
client->>http.ts: GET /auth/claude/status?label=...
http.ts->>readClaudeAuthStatus: label
readClaudeAuthStatus->>controller storage: inspect auth files
controller storage-->>readClaudeAuthStatus: status data
readClaudeAuthStatus-->>http.ts: ClaudeAuthStatus
http.ts-->>client: JSON response
Possibly related PRs
🚥 Pre-merge checks | ✅ 6 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/actions/setup/action.yml:
- Around line 61-88: The retry logic in run_bun_install is effectively
unreachable because the surrounding build job can time out before the first
20-minute watchdog finishes. Update the action’s timeout handling so it fits
within the job’s actual timeout in the check workflow, either by reducing the
bun install watchdog/retry window or by aligning the job timeout higher; keep
the fix centered around run_bun_install and the retry loop in this action.
- Around line 63-74: Update the timeout handling in the setup action so the
watchdog kills the entire process group, not just the `bun install` parent PID.
In the `bun install --frozen-lockfile` block, start the install in its own
process group and make the timeout path send the termination signal to that
group, including any child processes spawned by lifecycle scripts. Keep the
existing `install_pid`, `timeout_pid`, and `wait` flow in the action, but change
the termination logic to target the full group.
In `@packages/app/src/docker-git/controller-compose-files.ts`:
- Around line 20-21: The error text in mapComposePathError is too specific to
docker-compose.yml, but this mapper is also used for extra and GPU overlay
paths. Update the error mapping in controller-compose-files so the message
includes the actual path/context being resolved, or split the mapper into
separate helpers for base and overlay resolution; make sure callers from the
compose path resolution flow pass the relevant overlay name or file type into
the mapper.
In `@packages/app/src/docker-git/controller-compose.ts`:
- Around line 246-248: The runtime overlay path in
resolveControllerRuntimeOverlayPath() still accepts any existing path, so add
the same stat(...).type === "File" validation used for the other overlays to
reject directories for docker-compose.isolated.yml as well. Update the runtime
overlay resolution flow in controller-compose.ts to fail early with a clear
error before docker compose is invoked, and add a regression test covering a
directory passed as the isolated runtime overlay.
In `@packages/app/tests/docker-git/controller-compose-fixture.ts`:
- Around line 119-128: The controller build skiller fixture arbitrary does not
cover the full DOCKER_GIT_CONTROLLER_BUILD_SKILLER contract, so add the missing
alias values to controllerBuildSkillerFixtureModeArbitrary in
controller-compose-fixture.ts. Update the fc.constantFrom set to include "true"
and "false" alongside undefined, "0", and "1", so the parser normalization path
for env aliases is exercised and regressions in the build-skiller mode handling
are caught.
In `@packages/auth-oauth/src/claude-docker-oauth.ts`:
- Around line 389-399: Remove all token emission from the Claude OAuth CLI
output, including the `renderClaudeDockerOauthResult` path and the related
stdout branch in the result-printing flow. Keep `printToken` as a no-op for
credentials and only print non-sensitive status/probe information, so no full
credential can ever reach shell history, CI logs, or downstream scripts. Locate
the affected logic in `renderClaudeDockerOauthResult` and the token-printing
handling around the CLI result output, and ensure the final output path never
includes `result.token` or any equivalent secret value.
- Around line 216-223: The OAuth token is being exposed both in the docker argv
and in stdout. In the docker command setup inside claude-docker-oauth.ts, stop
passing CLAUDE_CODE_OAUTH_TOKEN directly via args.push and move it to a secure
temporary env file or mounted file with restrictive permissions, then load it
inside the container. Also update renderClaudeDockerOauthResult so it never
prints result.token, removing the --print-token path entirely or making it a
no-op for token output.
In `@packages/auth-oauth/src/claude-local-smoke.ts`:
- Around line 78-87: buildClaudeLocalOauthEnv currently spreads the entire
baseEnv, so inherited OAuth variables can override the fresh token used by the
probe. Update buildClaudeLocalOauthEnv in claude-local-smoke.ts to strip any
preexisting OAuth-related env keys from baseEnv before setting
CLAUDE_CODE_OAUTH_TOKEN, especially DOCKER_GIT_CLAUDE_OAUTH_TOKEN, then keep the
explicit CLAUDE_CONFIG_DIR and HOME overrides. Apply the same env-cleanup logic
anywhere the probe env is constructed in the related smoke/probe path so the
child process only sees the newly captured token.
In `@packages/auth-oauth/src/claude-oauth-token.ts`:
- Around line 37-38: `extractClaudeOauthToken()` is too permissive and can treat
diagnostic text after the marker as a token; tighten the parsing in
`oauthTokenRegex`/`compactSection` so only strings matching the real OAuth token
shape are accepted, using `oauthLiveOutputTokenPrefix` as part of the
validation. Update the token extraction logic in `claude-oauth-token.ts` to
reject marker-only or marker-plus-text output, and add a regression test
covering `setup-token` output that contains the marker but no actual token so
`claude-docker-oauth.ts` never persists a false credential.
In `@packages/auth-oauth/tests/claude-local-smoke.test.ts`:
- Around line 62-92: Add a failing regression test in claude-local-smoke.test.ts
that covers the probe env token precedence bug: start from a base env containing
both DOCKER_GIT_CLAUDE_OAUTH_TOKEN and other vars, then assert
buildClaudeLocalOauthEnv (and the local probe flow if applicable) only preserves
the fresh token by setting CLAUDE_CODE_OAUTH_TOKEN and not inheriting the stale
docker-git token. Update envArbitrary or add a dedicated case so the property
test models both keys, and reference buildClaudeLocalOauthEnv plus
renderClaudeLocalOauthSmokeResult if needed to keep the smoke coverage aligned.
- Around line 94-174: The smoke tests in claude-local-smoke.test.ts are still
using async/await and Promise-style helpers, so convert each case to it.effect
with Effect.gen to match the repository’s other usecase tests. Update the
affected test bodies around runClaudeLocalOauthSmoke,
persistClaudeLocalOauthToken, readFile, and renderClaudeLocalOauthSmokeResult so
the setup, assertions, and probe/setup-token stubs are expressed through
Effect-based test flows instead of plain async functions.
In `@packages/lib/src/usecases/auth-claude-local.ts`:
- Around line 38-57: The local Claude OAuth probe currently reuses account-level
config via buildClaudeLocalEnv, so runClaudeLocalPingProbeExitCode still
inherits settings.json and session state from accountPath. Change the probe flow
to use a separate temporary Claude config directory for the ping command,
matching the isolation already used in the docker path, and keep accountPath
only for the real account data. Update buildClaudeLocalEnv and
runClaudeLocalPingProbeExitCode accordingly, and add a regression test that
exercises ClaudeLoginProbeFailed behavior when accountPath contains an existing
settings.json with bypassPermissions or similar login-policy settings.
In `@packages/lib/src/usecases/state-repo.ts`:
- Around line 145-148: The warning in state-repo’s auto-sync path is leaking an
absolute filesystem path through the `Effect.logWarning` message that references
`indexLockPath`. Update the log in `state-repo` to avoid printing user-specific
absolute paths; keep the diagnostic context but replace the interpolated path
with a sanitized reference such as `.git/index.lock` or a generic
repository-root alias.
In `@packages/lib/src/usecases/state-repo/lock.ts`:
- Around line 75-79: The lock repository logic currently reads cwd directly via
process.cwd(), which should be removed from product code. Update the state lock
flow in lock.ts, especially the Effect.gen block that calls resolveStateRoot, to
obtain cwd/root through an Effect service or an explicit parameter instead of
the global process. Keep resolveStateLockPath and the surrounding repository
logic unchanged, but wire the cwd source through the existing Effect runtime
boundary so the code remains testable and SHELL-compliant.
- Around line 20-45: The state lock in acquireStateFileLock/releaseStateFileLock
can remain forever if the process crashes before cleanup, because the
<root>.lock directory has no owner or expiry handling. Update the lock flow in
acquireStateFileLock and the withStateGitLock path to detect and reclaim stale
locks, for example by storing owner/heartbeat metadata and checking age or
liveness before treating the lock as busy, or by switching to a real system file
lock. Keep releaseStateFileLock as best-effort cleanup, but make lock
acquisition resilient to orphaned lock directories.
In `@packages/lib/tests/usecases/auth-claude-login-flow.test.ts`:
- Around line 93-96: The current test only covers normalizeClaudeOauthToken
directly and bypasses runClaudeLoginFlow, so it will miss regressions in the
boundary handling. Update the test in auth-claude-login-flow.test.ts to exercise
runClaudeLoginFlow with a captureToken containing surrounding
whitespace/newlines, and assert that persistToken and probeToken receive the
trimmed token value. Keep the check at the flow boundary using the
runClaudeLoginFlow/ensureClaudeOauthToken path rather than testing
normalizeClaudeOauthToken in isolation, and follow the test style conventions by
expressing the invariant with fc.property where appropriate.
In `@packages/lib/tests/usecases/state-repo-auto-sync.test.ts`:
- Around line 102-179: Current tests only cover two fixed auto-sync cases; add a
property-based test around autoSyncState to vary whether .git/index.lock exists
and the git exit codes. Use fc.property with the existing helpers like
withTempDir, withPatchedEnv, makeFakeExecutor, and isMutatingSyncGitCommand to
assert the invariants that index.lock implies zero mutating git commands and
that any git failure still leaves the state .lock file absent. Keep the existing
logging assertions where relevant, but move the core expectations into the
generated property cases so the behavior is checked across many combinations.
In `@scripts/e2e/auth-claude-login.sh`:
- Around line 117-142: The e2e in auth-claude-login.sh currently exercises the
non-web login path and stops after login, so it misses the documented Claude
web-flow and the follow-up status redaction check. Update the flow in the auth
claude login test to invoke the web variant of the login command, then call auth
claude status to verify the post-login state instead of relying only on login
output. Keep the existing token persistence checks, and add/assert that status
output does not print the OAuth token or token marker. Use the existing auth
claude login, auth claude status, and OAUTH_TOKEN_MARKER references to locate
the test logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d6cc3051-c82d-4914-9684-6a35457babac
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (40)
.changeset/fix-claude-auth-login-probe.md.github/actions/setup/action.yml.github/workflows/check.ymlpackage.jsonpackages/api/Dockerfilepackages/app/CHANGELOG.mdpackages/app/package.jsonpackages/app/src/docker-git/controller-compose-files.tspackages/app/src/docker-git/controller-compose.tspackages/app/tests/docker-git/controller-compose-fixture.tspackages/app/tests/docker-git/controller-compose.test.tspackages/auth-oauth/package.jsonpackages/auth-oauth/src/claude-docker-oauth.tspackages/auth-oauth/src/claude-local-smoke.tspackages/auth-oauth/src/claude-oauth-token.tspackages/auth-oauth/src/index.tspackages/auth-oauth/tests/claude-docker-oauth.test.tspackages/auth-oauth/tests/claude-local-smoke.test.tspackages/auth-oauth/tests/claude-oauth-token.test.tspackages/auth-oauth/tsconfig.jsonpackages/container/src/core/templates/dockerfile-prelude.tspackages/container/tests/core/templates.test.tspackages/docker-git-session-sync/CHANGELOG.mdpackages/docker-git-session-sync/package.jsonpackages/lib/package.jsonpackages/lib/src/usecases/auth-claude-local.tspackages/lib/src/usecases/auth-claude-login-flow.tspackages/lib/src/usecases/auth-claude-oauth.tspackages/lib/src/usecases/auth-claude.tspackages/lib/src/usecases/auth.tspackages/lib/src/usecases/state-repo.tspackages/lib/src/usecases/state-repo/lock.tspackages/lib/tests/usecases/auth-claude-local.test.tspackages/lib/tests/usecases/auth-claude-login-flow.test.tspackages/lib/tests/usecases/auth-claude-login.test.tspackages/lib/tests/usecases/state-repo-auto-sync.test.tspnpm-workspace.yamlscripts/e2e/_lib.shscripts/e2e/auth-claude-login.shscripts/e2e/run-all.sh
📜 Review details
⏰ Context from checks skipped due to timeout. (10)
- GitHub Check: E2E (OpenCode)
- GitHub Check: E2E (Runtime volumes + SSH)
- GitHub Check: E2E (Clone auto-open SSH)
- GitHub Check: E2E (Clone cache)
- GitHub Check: E2E (Browser command)
- GitHub Check: E2E (Claude auth login)
- GitHub Check: E2E (Login context)
- GitHub Check: Lint
- GitHub Check: Test
- GitHub Check: Final build (windows-latest)
🧰 Additional context used
📓 Path-based instructions (17)
**/*.{sh,bash,py,js,ts,jsx,tsx,go,java,rb,php}
📄 CodeRabbit inference engine (Custom checks)
Fail if changed files introduce command injection or unsafe shell/process execution with user-controlled input
Files:
packages/auth-oauth/src/index.tsscripts/e2e/run-all.shpackages/container/tests/core/templates.test.tspackages/lib/src/usecases/auth.tspackages/lib/tests/usecases/auth-claude-local.test.tsscripts/e2e/_lib.shpackages/auth-oauth/tests/claude-oauth-token.test.tspackages/auth-oauth/tests/claude-docker-oauth.test.tspackages/lib/src/usecases/auth-claude-login-flow.tsscripts/e2e/auth-claude-login.shpackages/auth-oauth/tests/claude-local-smoke.test.tspackages/lib/tests/usecases/auth-claude-login-flow.test.tspackages/lib/src/usecases/state-repo/lock.tspackages/lib/tests/usecases/state-repo-auto-sync.test.tspackages/lib/src/usecases/auth-claude-local.tspackages/lib/tests/usecases/auth-claude-login.test.tspackages/app/src/docker-git/controller-compose-files.tspackages/auth-oauth/src/claude-docker-oauth.tspackages/app/tests/docker-git/controller-compose-fixture.tspackages/auth-oauth/src/claude-local-smoke.tspackages/lib/src/usecases/auth-claude-oauth.tspackages/app/tests/docker-git/controller-compose.test.tspackages/auth-oauth/src/claude-oauth-token.tspackages/lib/src/usecases/auth-claude.tspackages/container/src/core/templates/dockerfile-prelude.tspackages/app/src/docker-git/controller-compose.tspackages/lib/src/usecases/state-repo.ts
**/*.{py,js,ts,jsx,tsx,go,java,rb,php,sh,bash,c,cpp}
📄 CodeRabbit inference engine (Custom checks)
Fail if changed files introduce path traversal or writes outside intended project/container state directories
Files:
packages/auth-oauth/src/index.tsscripts/e2e/run-all.shpackages/container/tests/core/templates.test.tspackages/lib/src/usecases/auth.tspackages/lib/tests/usecases/auth-claude-local.test.tsscripts/e2e/_lib.shpackages/auth-oauth/tests/claude-oauth-token.test.tspackages/auth-oauth/tests/claude-docker-oauth.test.tspackages/lib/src/usecases/auth-claude-login-flow.tsscripts/e2e/auth-claude-login.shpackages/auth-oauth/tests/claude-local-smoke.test.tspackages/lib/tests/usecases/auth-claude-login-flow.test.tspackages/lib/src/usecases/state-repo/lock.tspackages/lib/tests/usecases/state-repo-auto-sync.test.tspackages/lib/src/usecases/auth-claude-local.tspackages/lib/tests/usecases/auth-claude-login.test.tspackages/app/src/docker-git/controller-compose-files.tspackages/auth-oauth/src/claude-docker-oauth.tspackages/app/tests/docker-git/controller-compose-fixture.tspackages/auth-oauth/src/claude-local-smoke.tspackages/lib/src/usecases/auth-claude-oauth.tspackages/app/tests/docker-git/controller-compose.test.tspackages/auth-oauth/src/claude-oauth-token.tspackages/lib/src/usecases/auth-claude.tspackages/container/src/core/templates/dockerfile-prelude.tspackages/app/src/docker-git/controller-compose.tspackages/lib/src/usecases/state-repo.ts
**/*.{js,ts,jsx,tsx,py,java,go,rb,php,sh,bash,yml,yaml,json,env*,toml,cfg,config,dockerfile,dockerignore}
📄 CodeRabbit inference engine (Custom checks)
Fail if changed files expose credentials, tokens, private-keys, or PII in source, generated config, logs, or CI output
Files:
packages/auth-oauth/src/index.tspnpm-workspace.yamlpackages/auth-oauth/tsconfig.jsonscripts/e2e/run-all.shpackages/docker-git-session-sync/package.jsonpackages/app/package.jsonpackages/container/tests/core/templates.test.tspackages/lib/src/usecases/auth.tspackages/lib/tests/usecases/auth-claude-local.test.tsscripts/e2e/_lib.shpackages/auth-oauth/tests/claude-oauth-token.test.tspackages/auth-oauth/tests/claude-docker-oauth.test.tspackages/lib/package.jsonpackage.jsonpackages/auth-oauth/package.jsonpackages/lib/src/usecases/auth-claude-login-flow.tsscripts/e2e/auth-claude-login.shpackages/auth-oauth/tests/claude-local-smoke.test.tspackages/lib/tests/usecases/auth-claude-login-flow.test.tspackages/lib/src/usecases/state-repo/lock.tspackages/lib/tests/usecases/state-repo-auto-sync.test.tspackages/lib/src/usecases/auth-claude-local.tspackages/lib/tests/usecases/auth-claude-login.test.tspackages/app/src/docker-git/controller-compose-files.tspackages/auth-oauth/src/claude-docker-oauth.tspackages/app/tests/docker-git/controller-compose-fixture.tspackages/auth-oauth/src/claude-local-smoke.tspackages/lib/src/usecases/auth-claude-oauth.tspackages/app/tests/docker-git/controller-compose.test.tspackages/auth-oauth/src/claude-oauth-token.tspackages/lib/src/usecases/auth-claude.tspackages/container/src/core/templates/dockerfile-prelude.tspackages/app/src/docker-git/controller-compose.tspackages/lib/src/usecases/state-repo.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: FUNCTIONAL CORE: Write only pure functions with immutable data and mathematical operations in core modules; no side effects, mutations, or external service calls
IMPERATIVE SHELL: Isolate all side effects (IO, network, database, environment/process) in a thin SHELL layer; CORE never calls SHELL, only SHELL → CORE
Never useanytype annotation in TypeScript; useunknownonly at SHELL boundaries for decoding, never exportunknownoutside boundary modules
Never useastype assertions in normal code; only permitasin a single 'axiomatic' module (brands, constructors, constants) after which types flow safely without casts
Always use exhaustive pattern matching for union types through.exhaustive()orMatch.exhaustive()from effect-ts; never use switch statements or unhandled type branches
Use Effect<Success, Error, Requirements> monad from effect-ts for all effects; compose through pipe() and Effect.flatMap(); never use async/await, raw Promise chains (then/catch), or Promise.all in product code
Interoperate with Promise/exceptions only in SHELL through Effect.try/Effect.tryPromise with typed error mapping; never leave raw exceptions or untyped errors in the domain
Use Effect.acquireRelease + Effect.scoped for resource management with guaranteed finalization; never manage resources with try/finally or manual cleanup
All external services (database, HTTP, environment) must be accessed through Effect-based interfaces and Layer-based dependency injection; never call external APIs directly
Provide comprehensive TSDoc comments with mathematical notation:@pure,@effect,@invariant,@precondition,@postcondition,@complexity,@throws, and CHANGE/WHY/REF/SOURCE/FORMAT THEOREM functional comment markers
No console.*, process direct calls, or untyped environment access in product code; all such operations must be abstracted through Layer-based services in SHELL
Boundary data from external sources (HTTP, database, environment) must be decoded/valida...
Files:
packages/auth-oauth/src/index.tspackages/container/tests/core/templates.test.tspackages/lib/src/usecases/auth.tspackages/lib/tests/usecases/auth-claude-local.test.tspackages/auth-oauth/tests/claude-oauth-token.test.tspackages/auth-oauth/tests/claude-docker-oauth.test.tspackages/lib/src/usecases/auth-claude-login-flow.tspackages/auth-oauth/tests/claude-local-smoke.test.tspackages/lib/tests/usecases/auth-claude-login-flow.test.tspackages/lib/src/usecases/state-repo/lock.tspackages/lib/tests/usecases/state-repo-auto-sync.test.tspackages/lib/src/usecases/auth-claude-local.tspackages/lib/tests/usecases/auth-claude-login.test.tspackages/app/src/docker-git/controller-compose-files.tspackages/auth-oauth/src/claude-docker-oauth.tspackages/app/tests/docker-git/controller-compose-fixture.tspackages/auth-oauth/src/claude-local-smoke.tspackages/lib/src/usecases/auth-claude-oauth.tspackages/app/tests/docker-git/controller-compose.test.tspackages/auth-oauth/src/claude-oauth-token.tspackages/lib/src/usecases/auth-claude.tspackages/container/src/core/templates/dockerfile-prelude.tspackages/app/src/docker-git/controller-compose.tspackages/lib/src/usecases/state-repo.ts
**/{browser*,server*,app*,*.ts,*.js}
📄 CodeRabbit inference engine (README.md)
Web version must listen on 0.0.0.0 by default for accessibility across LAN devices
Files:
packages/auth-oauth/src/index.tspackages/container/tests/core/templates.test.tspackages/lib/src/usecases/auth.tspackages/lib/tests/usecases/auth-claude-local.test.tspackages/auth-oauth/tests/claude-oauth-token.test.tspackages/auth-oauth/tests/claude-docker-oauth.test.tspackages/lib/src/usecases/auth-claude-login-flow.tspackages/auth-oauth/tests/claude-local-smoke.test.tspackages/lib/tests/usecases/auth-claude-login-flow.test.tspackages/lib/src/usecases/state-repo/lock.tspackages/lib/tests/usecases/state-repo-auto-sync.test.tspackages/lib/src/usecases/auth-claude-local.tspackages/lib/tests/usecases/auth-claude-login.test.tspackages/app/src/docker-git/controller-compose-files.tspackages/auth-oauth/src/claude-docker-oauth.tspackages/app/tests/docker-git/controller-compose-fixture.tspackages/auth-oauth/src/claude-local-smoke.tspackages/lib/src/usecases/auth-claude-oauth.tspackages/app/tests/docker-git/controller-compose.test.tspackages/auth-oauth/src/claude-oauth-token.tspackages/lib/src/usecases/auth-claude.tspackages/container/src/core/templates/dockerfile-prelude.tspackages/app/src/docker-git/controller-compose.tspackages/lib/src/usecases/state-repo.ts
**/{cli*,command*,auto*,*.ts,*.tsx}
📄 CodeRabbit inference engine (README.md)
Implement auto-mode agent selection logic to choose Claude, Codex, Gemini, or Grok randomly from available authorized providers, or allow forced selection with --auto=
Files:
packages/auth-oauth/src/index.tspackages/container/tests/core/templates.test.tspackages/lib/src/usecases/auth.tspackages/lib/tests/usecases/auth-claude-local.test.tspackages/auth-oauth/tests/claude-oauth-token.test.tspackages/auth-oauth/tests/claude-docker-oauth.test.tspackages/lib/src/usecases/auth-claude-login-flow.tspackages/auth-oauth/tests/claude-local-smoke.test.tspackages/lib/tests/usecases/auth-claude-login-flow.test.tspackages/lib/src/usecases/state-repo/lock.tspackages/lib/tests/usecases/state-repo-auto-sync.test.tspackages/lib/src/usecases/auth-claude-local.tspackages/lib/tests/usecases/auth-claude-login.test.tspackages/app/src/docker-git/controller-compose-files.tspackages/auth-oauth/src/claude-docker-oauth.tspackages/app/tests/docker-git/controller-compose-fixture.tspackages/auth-oauth/src/claude-local-smoke.tspackages/lib/src/usecases/auth-claude-oauth.tspackages/app/tests/docker-git/controller-compose.test.tspackages/auth-oauth/src/claude-oauth-token.tspackages/lib/src/usecases/auth-claude.tspackages/container/src/core/templates/dockerfile-prelude.tspackages/app/src/docker-git/controller-compose.tspackages/lib/src/usecases/state-repo.ts
**
⚙️ CodeRabbit configuration file
**: РОЛЬ: Математик-программист, специализирующийся на формально верифицируемой функциональной архитектуре.ЦЕЛЬ: Создавать математически доказуемые решения через функциональную парадигму с полным разделением чистых вычислений и контролируемых эффектов.
МОДЕЛЬ РАССУЖДЕНИЯ:
- Не выдавать “личные мнения”. Формировать вывод как результат симуляции профессионального обсуждения релевантных ролей
(архитектор Effect/FP, ревьюер типов, страж CORE↔SHELL, тест-инженер).- Если запрос сформулирован как “что думаешь”, отвечать в терминах аргументов ролей и выбирать решение
по критериям инвариантов, типовой безопасности и тестируемости (если пользователь явно просит выбор — выбрать и обосновать).ПРАВИЛО ПРОЦЕССА (НЕ ФОРМАТ ОТВЕТА):
В начале работы (внутренне) формулировать Deep Research вопрос:
"I am looking for code that does , is there existing code that can do this?"
Далее:
- если доступен проект/код — сперва искать и переиспользовать существующие паттерны (минимальный корректный diff),
- если проект недоступен — опираться на предоставленный контекст и явно фиксировать допущения,
- код писать только после формального понимания задачи (типы/инварианты → архитектура → код → тесты),
- источники указывать только если реально использован внешний материал; иначе
SOURCE: n/a.ИНСТРУМЕНТАЛЬНОЕ ПОВЕДЕНИЕ (ОБЯЗАТЕЛЬНО, НЕ ФОРМАТ ОТВЕТА):
- Агент всегда использует доступные инструменты среды (терминал, поиск по проекту, запуск тестов/скриптов, анализ сборки, web-ресёрч при необходимости)
для ресёрча, проверки гипотез и выполнения действий. Приоритет: проверяемость, воспроизводимость, минимальный риск.- Агент не предлагает “гайд” как замену действия. Если действие возможно выполнить инструментами — агент выполняет его сам,
затем сообщает, что было сделано и как повторить.- Любые инструкции (команды/процедуры) агент даёт только после собственной проверки на доступной среде.
Если проверить невозможно — явно фиксирует ограничение и перечисляе...
Files:
packages/auth-oauth/src/index.tspnpm-workspace.yamlpackages/auth-oauth/tsconfig.jsonscripts/e2e/run-all.shpackages/docker-git-session-sync/package.jsonpackages/docker-git-session-sync/CHANGELOG.mdpackages/app/package.jsonpackages/app/CHANGELOG.mdpackages/container/tests/core/templates.test.tspackages/lib/src/usecases/auth.tspackages/lib/tests/usecases/auth-claude-local.test.tsscripts/e2e/_lib.shpackages/auth-oauth/tests/claude-oauth-token.test.tspackages/auth-oauth/tests/claude-docker-oauth.test.tspackages/lib/package.jsonpackage.jsonpackages/auth-oauth/package.jsonpackages/lib/src/usecases/auth-claude-login-flow.tsscripts/e2e/auth-claude-login.shpackages/auth-oauth/tests/claude-local-smoke.test.tspackages/lib/tests/usecases/auth-claude-login-flow.test.tspackages/api/Dockerfilepackages/lib/src/usecases/state-repo/lock.tspackages/lib/tests/usecases/state-repo-auto-sync.test.tspackages/lib/src/usecases/auth-claude-local.tspackages/lib/tests/usecases/auth-claude-login.test.tspackages/app/src/docker-git/controller-compose-files.tspackages/auth-oauth/src/claude-docker-oauth.tspackages/app/tests/docker-git/controller-compose-fixture.tspackages/auth-oauth/src/claude-local-smoke.tspackages/lib/src/usecases/auth-claude-oauth.tspackages/app/tests/docker-git/controller-compose.test.tspackages/auth-oauth/src/claude-oauth-token.tspackages/lib/src/usecases/auth-claude.tspackages/container/src/core/templates/dockerfile-prelude.tspackages/app/src/docker-git/controller-compose.tspackages/lib/src/usecases/state-repo.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Ты строгий ревьюер SPEC DRIVEN DEVELOPMENT.Перед выводами изучи README.md, другие *.md файлы, linked issues,
PR description, PR comments/discussion и релевантную кодовую базу.Сверь изменения с исходным ТЗ/спекой и обсуждением. Флагай любой уход
от спеки, недокументированное изменение поведения, отсутствие тестов
для заявленного поведения и security-риск. Если спека не видна,
попроси автора добавить ее в issue или PR description.Проверь решение с точки зрения формальной верификации: какие инварианты,
предусловия и постусловия можно доказать математически, а где доказуемость
слабая. Оцени решение с точки зрения теории игр: устойчивы ли стимулы,
нет ли выгодного обхода правил, и какое решение было бы сильнее.
Files:
packages/auth-oauth/src/index.tspnpm-workspace.yamlpackages/auth-oauth/tsconfig.jsonscripts/e2e/run-all.shpackages/docker-git-session-sync/package.jsonpackages/docker-git-session-sync/CHANGELOG.mdpackages/app/package.jsonpackages/app/CHANGELOG.mdpackages/container/tests/core/templates.test.tspackages/lib/src/usecases/auth.tspackages/lib/tests/usecases/auth-claude-local.test.tsscripts/e2e/_lib.shpackages/auth-oauth/tests/claude-oauth-token.test.tspackages/auth-oauth/tests/claude-docker-oauth.test.tspackages/lib/package.jsonpackage.jsonpackages/auth-oauth/package.jsonpackages/lib/src/usecases/auth-claude-login-flow.tsscripts/e2e/auth-claude-login.shpackages/auth-oauth/tests/claude-local-smoke.test.tspackages/lib/tests/usecases/auth-claude-login-flow.test.tspackages/api/Dockerfilepackages/lib/src/usecases/state-repo/lock.tspackages/lib/tests/usecases/state-repo-auto-sync.test.tspackages/lib/src/usecases/auth-claude-local.tspackages/lib/tests/usecases/auth-claude-login.test.tspackages/app/src/docker-git/controller-compose-files.tspackages/auth-oauth/src/claude-docker-oauth.tspackages/app/tests/docker-git/controller-compose-fixture.tspackages/auth-oauth/src/claude-local-smoke.tspackages/lib/src/usecases/auth-claude-oauth.tspackages/app/tests/docker-git/controller-compose.test.tspackages/auth-oauth/src/claude-oauth-token.tspackages/lib/src/usecases/auth-claude.tspackages/container/src/core/templates/dockerfile-prelude.tspackages/app/src/docker-git/controller-compose.tspackages/lib/src/usecases/state-repo.ts
**/{setup,install,config,*.sh,*.md}
📄 CodeRabbit inference engine (README.md)
Ensure default projects directory is ~/.docker-git
Files:
scripts/e2e/run-all.shpackages/docker-git-session-sync/CHANGELOG.mdpackages/app/CHANGELOG.mdscripts/e2e/_lib.shscripts/e2e/auth-claude-login.sh
**/{.git*,config*,*.sh,docker-compose*}
📄 CodeRabbit inference engine (README.md)
Use git credential helper to automatically select correct token by host for HTTPS clone/push operations
Files:
scripts/e2e/run-all.shscripts/e2e/_lib.shscripts/e2e/auth-claude-login.sh
**/{browser*,server*,.env*,*.sh}
📄 CodeRabbit inference engine (README.md)
Allow DOCKER_GIT_WEB_HOST environment variable to restrict web access to 127.0.0.1
Files:
scripts/e2e/run-all.shscripts/e2e/_lib.shscripts/e2e/auth-claude-login.sh
**/{package*.json,requirements*.txt,setup.py,setup.cfg,Pipfile,Pipfile.lock,pyproject.toml,pom.xml,build.gradle,Gemfile,Gemfile.lock,go.mod,go.sum,composer.json,Cargo.toml,Cargo.lock}
📄 CodeRabbit inference engine (Custom checks)
Fail if dependency or package-manager changes materially increase supply-chain risk without justification
Files:
packages/docker-git-session-sync/package.jsonpackages/app/package.jsonpackages/lib/package.jsonpackage.jsonpackages/auth-oauth/package.json
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.test.{ts,tsx}: Write property-based tests using fast-check (fc.property) to verify mathematical invariants; unit tests must use Effect test utilities without async/await
Every bug fix must be accompanied by a reproducing test case; the test must fail before the fix and pass after; document the Proof of Fix with root cause and solution
Files:
packages/container/tests/core/templates.test.tspackages/lib/tests/usecases/auth-claude-local.test.tspackages/auth-oauth/tests/claude-oauth-token.test.tspackages/auth-oauth/tests/claude-docker-oauth.test.tspackages/auth-oauth/tests/claude-local-smoke.test.tspackages/lib/tests/usecases/auth-claude-login-flow.test.tspackages/lib/tests/usecases/state-repo-auto-sync.test.tspackages/lib/tests/usecases/auth-claude-login.test.tspackages/app/tests/docker-git/controller-compose.test.ts
**/{.env*,auth*,config*,setup*}
📄 CodeRabbit inference engine (README.md)
Store authentication tokens in env-file as GIT_AUTH_TOKEN__ / GIT_AUTH_USER__
Files:
packages/lib/src/usecases/auth.tspackages/lib/tests/usecases/auth-claude-local.test.tspackages/lib/src/usecases/auth-claude-login-flow.tsscripts/e2e/auth-claude-login.shpackages/lib/tests/usecases/auth-claude-login-flow.test.tspackages/lib/src/usecases/auth-claude-local.tspackages/lib/tests/usecases/auth-claude-login.test.tspackages/lib/src/usecases/auth-claude-oauth.tspackages/lib/src/usecases/auth-claude.ts
**/.github/workflows/*.{yml,yaml}
📄 CodeRabbit inference engine (Custom checks)
Fail if changed files introduce unsafe GitHub Actions configuration such as unsafe
pull_request_target, or unnecessary write permissions
Files:
.github/workflows/check.yml
package.json
📄 CodeRabbit inference engine (AGENTS.md)
Dependencies must include effect ^3.x and
@effect/schema^0.x; prohibit downgrading these versions or introducing incompatible alternatives (async-only libraries without Effect support)Require Effect and
@effect/schemaas mandatory dependencies for type-safe effects and validation
Files:
package.json
**/{Dockerfile*,docker-compose*.{yml,yaml},.dockerignore}
📄 CodeRabbit inference engine (Custom checks)
Fail if changed files introduce unsafe Docker configuration such as privileged containers, broad host mounts, unbounded Docker socket access, or unnecessary write permissions
Files:
packages/api/Dockerfile
🧠 Learnings (2)
📚 Learning: 2026-05-22T21:08:18.083Z
Learnt from: skulidropek
Repo: ProverCoderAI/docker-git PR: 344
File: packages/app/src/docker-git/controller-compose.ts:34-40
Timestamp: 2026-05-22T21:08:18.083Z
Learning: In this repo’s docker-git controller compose generation, `${DOCKER_GIT_CONTROLLER_BUILD_SKILLER:-1}` should be treated as standard bash parameter expansion: when `DOCKER_GIT_CONTROLLER_BUILD_SKILLER` is unset, it defaults to the string "1". There is no "-1" mode. The runtime contract enforced by `packages/app/src/docker-git/controller-compose.ts` is: unset / "1" / "true" => output "1"; "0" / "false" => output "0". If review code shows branching/behavior for "-1" or any numeric value other than this 0/1 contract, flag it. Also ensure the Dockerfile ARG `DOCKER_GIT_CONTROLLER_BUILD_SKILLER` stays consistent with default `1`.
Applied to files:
packages/app/src/docker-git/controller-compose-files.tspackages/app/src/docker-git/controller-compose.ts
📚 Learning: 2026-06-10T10:21:51.286Z
Learnt from: konard
Repo: ProverCoderAI/docker-git PR: 386
File: packages/app/tests/docker-git/gridland-react-singleton.test.ts:0-0
Timestamp: 2026-06-10T10:21:51.286Z
Learning: In `packages/app/tests/docker-git` tests that inspect the `packages/terminal` workspace (e.g., for React-related versions), remember that `react-dom` is intentionally declared as a `devDependency` in `packages/terminal` (the terminal exports components but does not render to the DOM itself). Therefore, test assertions should not require `react-dom` to appear in `dependencies`; when resolving versions, accept either `dependencies` or `devDependencies` (as `resolveDepVersion` does in `gridland-react-singleton.test.ts`).
Applied to files:
packages/app/tests/docker-git/controller-compose.test.ts
🪛 ast-grep (0.44.0)
scripts/e2e/auth-claude-login.sh
[warning] 116-116: set +e (or set +o errexit) disables the shell's errexit option, so the script keeps running after a command fails. This masks failures of security-critical operations (downloads, signature/checksum verification, permission changes, cleanup of secrets), letting the script proceed with a bad or insecure state. Leave errexit enabled (set -e / set -euo pipefail), or handle failures explicitly with if/|| and an explicit exit instead of globally turning off failure detection.
Context: set +e
Note: [CWE-754] Improper Check for Unusual or Exceptional Conditions.
(set-plus-e-error-masking-bash)
[warning] 17-17: A credential-bearing variable (e.g. PASSWORD, PASSWD, SECRET, TOKEN, API_KEY) is assigned a hardcoded string literal. Secrets committed to a script are exposed in source control, process listings, and shell history, and cannot be rotated without a code change. Read the value from a secrets manager or an injected environment variable at runtime instead (e.g. PASSWORD="${DB_PASSWORD:?must be set}"), and never commit the literal.
Context: OAUTH_TOKEN_MARKER="sk-ant-oat01-docker-git-e2e-oauth-token-marker"
Note: [CWE-798] Use of Hard-coded Credentials.
(hardcoded-password-assignment-bash)
packages/auth-oauth/src/claude-docker-oauth.ts
[warning] 4-4: Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawn } from "node:child_process"
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
packages/auth-oauth/src/claude-local-smoke.ts
[warning] 4-4: Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawn } from "node:child_process"
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🪛 Checkov (3.3.2)
packages/auth-oauth/package.json
[low] 15-16: Base64 High Entropy String
(CKV_SECRET_6)
🪛 Hadolint (2.14.0)
packages/api/Dockerfile
[info] 120-120: Multiple consecutive RUN instructions. Consider consolidation.
(DL3059)
[info] 121-121: Multiple consecutive RUN instructions. Consider consolidation.
(DL3059)
🪛 markdownlint-cli2 (0.22.1)
.changeset/fix-claude-auth-login-probe.md
[warning] 5-5: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🪛 OpenGrep (1.23.0)
packages/auth-oauth/src/claude-oauth-token.ts
[ERROR] 204-204: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
[ERROR] 209-209: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🪛 Shellcheck (0.11.0)
scripts/e2e/auth-claude-login.sh
[info] 118-118: Expressions don't expand in single quotes, use double quotes for that.
(SC2016)
🔇 Additional comments (10)
packages/app/CHANGELOG.md (1)
3-10: LGTM!packages/app/package.json (1)
3-3: LGTM!packages/docker-git-session-sync/CHANGELOG.md (1)
3-7: LGTM!packages/docker-git-session-sync/package.json (1)
3-3: LGTM!.github/workflows/check.yml (1)
252-271: LGTM!scripts/e2e/run-all.sh (1)
8-8: LGTM!scripts/e2e/_lib.sh (1)
21-47: LGTM!packages/container/src/core/templates/dockerfile-prelude.ts (1)
87-87: LGTM!Also applies to: 97-101
packages/container/tests/core/templates.test.ts (1)
211-211: LGTM!packages/auth-oauth/package.json (1)
34-41: 📐 Maintainability & Code QualityНе переносить
effectв runtime dependencies
packages/auth-oauth/src/*не импортирует ниeffect, ни@effect/schema; эти зависимости используются только в тестах. Для этого пакетаdevDependenciesдостаточно, а@effect/schemaздесь не требуется.> Likely an incorrect or invalid review comment.
| run_bun_install() { | ||
| local timeout_seconds=$((20 * 60)) | ||
| bun install --frozen-lockfile & | ||
| local install_pid="$!" | ||
| ( | ||
| sleep "$timeout_seconds" | ||
| echo "bun install exceeded 20 minutes; terminating" >&2 | ||
| kill "$install_pid" 2>/dev/null || true | ||
| ) & | ||
| local timeout_pid="$!" | ||
| local status=0 | ||
| wait "$install_pid" || status="$?" | ||
| kill "$timeout_pid" 2>/dev/null || true | ||
| wait "$timeout_pid" 2>/dev/null || true | ||
| return "$status" | ||
| } | ||
|
|
||
| for attempt in 1 2 3; do | ||
| if run_bun_install; then | ||
| exit 0 | ||
| fi | ||
| if [[ "$attempt" == "3" ]]; then | ||
| echo "bun install failed after retries" >&2 | ||
| exit 1 | ||
| fi | ||
| echo "bun install attempt ${attempt} failed; retrying..." >&2 | ||
| sleep $((attempt * 2)) | ||
| done |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Ретраи здесь недостижимы в текущем workflow timeout.
На Line 62 одна попытка может ждать 20 минут, но .github/workflows/check.yml уже использует этот action в build job с timeout-minutes: 10. В этом job GitHub завершит весь job раньше первого watchdog/retry, так что новая логика не даст заявленного восстановления после transient install failures. Сведите watchdog ниже минимального job timeout или поднимите timeout job.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/actions/setup/action.yml around lines 61 - 88, The retry logic in
run_bun_install is effectively unreachable because the surrounding build job can
time out before the first 20-minute watchdog finishes. Update the action’s
timeout handling so it fits within the job’s actual timeout in the check
workflow, either by reducing the bun install watchdog/retry window or by
aligning the job timeout higher; keep the fix centered around run_bun_install
and the retry loop in this action.
| bun install --frozen-lockfile & | ||
| local install_pid="$!" | ||
| ( | ||
| sleep "$timeout_seconds" | ||
| echo "bun install exceeded 20 minutes; terminating" >&2 | ||
| kill "$install_pid" 2>/dev/null || true | ||
| ) & | ||
| local timeout_pid="$!" | ||
| local status=0 | ||
| wait "$install_pid" || status="$?" | ||
| kill "$timeout_pid" 2>/dev/null || true | ||
| wait "$timeout_pid" 2>/dev/null || true |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
На timeout нужно завершать всю process group, а не только родительский PID.
На Lines 63-74 watchdog шлёт kill только в bun install. Если postinstall или другой lifecycle-скрипт породит дочерние процессы, они переживут retry и смогут оставить lock/сокеты/IO, а следующая попытка стартует поверх них. Здесь нужен запуск в отдельной process group и сигнал по всей группе.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/actions/setup/action.yml around lines 63 - 74, Update the timeout
handling in the setup action so the watchdog kills the entire process group, not
just the `bun install` parent PID. In the `bun install --frozen-lockfile` block,
start the install in its own process group and make the timeout path send the
termination signal to that group, including any child processes spawned by
lifecycle scripts. Keep the existing `install_pid`, `timeout_pid`, and `wait`
flow in the action, but change the termination logic to target the full group.
| const mapComposePathError = (error: PlatformError): ControllerBootstrapError => | ||
| controllerBootstrapError(`Failed to resolve docker-compose.yml path.\nDetails: ${String(error)}`) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Сообщение об ошибке здесь стало вводить в заблуждение.
mapComposePathError() теперь используется не только для базового docker-compose.yml, но и для extra/GPU overlay-путей. Если exists/stat упадёт на overlay-файле, пользователь всё равно увидит “Failed to resolve docker-compose.yml path”, хотя сломан уже другой вход. Лучше передавать контекст в mapper или разделить mapper'ы по типу overlay.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/app/src/docker-git/controller-compose-files.ts` around lines 20 -
21, The error text in mapComposePathError is too specific to docker-compose.yml,
but this mapper is also used for extra and GPU overlay paths. Update the error
mapping in controller-compose-files so the message includes the actual
path/context being resolved, or split the mapper into separate helpers for base
and overlay resolution; make sure callers from the compose path resolution flow
pass the relevant overlay name or file type into the mapper.
| const extraOverlayPath = yield* _(loadControllerComposeExtraPath()) | ||
| const runtimeOverlayPath = yield* _(resolveControllerRuntimeOverlayPath(composePath, dockerRuntime)) | ||
| return { ...composeFiles, runtimeOverlayPath } | ||
| return { ...composeFiles, extraOverlayPath, runtimeOverlayPath } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Проверка regular-file всё ещё не покрывает isolated runtime overlay.
resolveControllerComposeFiles() теперь валидирует extra/GPU overlays, но resolveControllerRuntimeOverlayPath() по-прежнему принимает любой существующий путь. Если docker-compose.isolated.yml окажется директорией, этот путь всё равно дойдёт до docker compose -f ... и свалится уже после bootstrap — ровно тот класс ошибки, который этот PR устраняет для остальных overlay-файлов. Добавьте ту же stat(...).type === "File" проверку для runtime overlay и отдельный регрессионный тест.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/app/src/docker-git/controller-compose.ts` around lines 246 - 248,
The runtime overlay path in resolveControllerRuntimeOverlayPath() still accepts
any existing path, so add the same stat(...).type === "File" validation used for
the other overlays to reject directories for docker-compose.isolated.yml as
well. Update the runtime overlay resolution flow in controller-compose.ts to
fail early with a clear error before docker compose is invoked, and add a
regression test covering a directory passed as the isolated runtime overlay.
| const controllerBuildSkillerFixtureModeArbitrary = fc.constantFrom<ControllerBuildSkillerFixtureMode>( | ||
| undefined, | ||
| "0", | ||
| "1" | ||
| ) | ||
| export const controllerDockerRuntimeEnvFixtureModeArbitrary = fc.constantFrom<ControllerDockerRuntimeEnvFixtureMode>( | ||
| undefined, | ||
| "host", | ||
| "isolated" | ||
| ) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Арбитрар не покрывает полный контракт DOCKER_GIT_CONTROLLER_BUILD_SKILLER.
Сейчас fixture генерирует только undefined | "0" | "1", поэтому ветки "true" и "false" больше не проверяются, хотя парсер режима должен принимать их как алиасы "1" и "0". Добавьте эти значения в controllerBuildSkillerFixtureModeArbitrary, иначе регрессия в нормализации env останется незамеченной. Based on learnings, поддерживаемый контракт здесь — unset / "1" / "true" → "1" и "0" / "false" → "0".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/app/tests/docker-git/controller-compose-fixture.ts` around lines 119
- 128, The controller build skiller fixture arbitrary does not cover the full
DOCKER_GIT_CONTROLLER_BUILD_SKILLER contract, so add the missing alias values to
controllerBuildSkillerFixtureModeArbitrary in controller-compose-fixture.ts.
Update the fc.constantFrom set to include "true" and "false" alongside
undefined, "0", and "1", so the parser normalization path for env aliases is
exercised and regressions in the build-skiller mode handling are caught.
Source: Learnings
| const acquireStateFileLock = ( | ||
| fs: FileSystem.FileSystem, | ||
| lockPath: string, | ||
| attempt: number = 0 | ||
| ): Effect.Effect<string, CommandFailedError | PlatformError> => | ||
| fs.makeDirectory(lockPath).pipe( | ||
| Effect.as(lockPath), | ||
| Effect.catchTag("SystemError", (error) => { | ||
| if (!isStateFileLockBusy(error)) { | ||
| return Effect.fail(error) | ||
| } | ||
| if (attempt >= stateGitLockMaxAttempts) { | ||
| return Effect.fail(new CommandFailedError({ command: "state git lock", exitCode: stateGitLockBusyExitCode })) | ||
| } | ||
| return Effect.sleep(stateGitLockRetryDelay).pipe( | ||
| Effect.zipRight(acquireStateFileLock(fs, lockPath, attempt + 1)) | ||
| ) | ||
| }) | ||
| ) | ||
|
|
||
| const releaseStateFileLock = ( | ||
| fs: FileSystem.FileSystem, | ||
| lockPath: string | ||
| ): Effect.Effect<void> => | ||
| fs.remove(lockPath, { recursive: true, force: true }).pipe( | ||
| Effect.orElseSucceed(() => void 0) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Каталоговый lock может застрять навсегда после crash.
На Line 25 создаётся постоянный <root>.lock, а на Lines 40-45 он снимается только в release-path. Если процесс/контейнер завершится между acquire и release, каталог останется на диске без owner/ttl, и все следующие withStateGitLock будут лишь ретраить 5 секунд и падать. Это превращает единичный crash в бессрочный отказ autoSyncState/autoPullState, пока кто-то вручную не удалит lock. Нужен reclaim stale lock (например, owner metadata + age/liveness check) или системный file lock.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/lib/src/usecases/state-repo/lock.ts` around lines 20 - 45, The state
lock in acquireStateFileLock/releaseStateFileLock can remain forever if the
process crashes before cleanup, because the <root>.lock directory has no owner
or expiry handling. Update the lock flow in acquireStateFileLock and the
withStateGitLock path to detect and reclaim stale locks, for example by storing
owner/heartbeat metadata and checking age or liveness before treating the lock
as busy, or by switching to a real system file lock. Keep releaseStateFileLock
as best-effort cleanup, but make lock acquisition resilient to orphaned lock
directories.
| Effect.gen(function*(_) { | ||
| const fs = yield* _(FileSystem.FileSystem) | ||
| const path = yield* _(Path.Path) | ||
| const root = resolveStateRoot(path, process.cwd()) | ||
| const lockPath = resolveStateLockPath(root) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Уберите прямой process.cwd() из продукт-кода.
На Line 78 ключ блокировки зависит от глобального состояния процесса, хотя этот слой уже живёт в Effect runtime и может получать cwd/root через service или явный параметр. Так код останется тестируемым и будет соответствовать границе SHELL. As per coding guidelines, "No console.*, process direct calls, or untyped environment access in product code; all such operations must be abstracted through Layer-based services in SHELL".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/lib/src/usecases/state-repo/lock.ts` around lines 75 - 79, The lock
repository logic currently reads cwd directly via process.cwd(), which should be
removed from product code. Update the state lock flow in lock.ts, especially the
Effect.gen block that calls resolveStateRoot, to obtain cwd/root through an
Effect service or an explicit parameter instead of the global process. Keep
resolveStateLockPath and the surrounding repository logic unchanged, but wire
the cwd source through the existing Effect runtime boundary so the code remains
testable and SHELL-compliant.
Source: Coding guidelines
| it.effect("normalizes token whitespace at the flow boundary", () => | ||
| Effect.sync(() => { | ||
| expect(normalizeClaudeOauthToken(`\n${oauthToken}\n`)).toBe(oauthToken) | ||
| })) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Этот тест обходит сам flow и не доказывает boundary-нормализацию.
Сейчас проверяется только normalizeClaudeOauthToken(), поэтому регресс в runClaudeLoginFlow() на Line 69 (например, если убрать ensureClaudeOauthToken) этот кейс не поймает. Для proof-of-fix здесь нужен flow-level тест: передать captureToken с обрамляющими пробелами/переводами строки и проверить, что в persistToken и probeToken приходит уже trim-значение. As per coding guidelines, "*.test.ts" должны проверять инварианты через fc.property, и "Every bug fix must be accompanied by a reproducing test case...".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/lib/tests/usecases/auth-claude-login-flow.test.ts` around lines 93 -
96, The current test only covers normalizeClaudeOauthToken directly and bypasses
runClaudeLoginFlow, so it will miss regressions in the boundary handling. Update
the test in auth-claude-login-flow.test.ts to exercise runClaudeLoginFlow with a
captureToken containing surrounding whitespace/newlines, and assert that
persistToken and probeToken receive the trimmed token value. Keep the check at
the flow boundary using the runClaudeLoginFlow/ensureClaudeOauthToken path
rather than testing normalizeClaudeOauthToken in isolation, and follow the test
style conventions by expressing the invariant with fc.property where
appropriate.
Source: Coding guidelines
| describe("state repo auto sync", () => { | ||
| it.effect("skips non-strict auto-sync before git add when the git index is locked", () => | ||
| withTempDir((home) => | ||
| withPatchedEnv( | ||
| { | ||
| HOME: home, | ||
| DOCKER_GIT_PROJECTS_ROOT: undefined, | ||
| DOCKER_GIT_STATE_AUTO_SYNC: "1", | ||
| DOCKER_GIT_STATE_AUTO_SYNC_STRICT: undefined | ||
| }, | ||
| Effect.gen(function*(_) { | ||
| const fs = yield* _(FileSystem.FileSystem) | ||
| const path = yield* _(Path.Path) | ||
| const root = path.join(home, ".docker-git") | ||
| const indexLockPath = path.join(root, ".git", "index.lock") | ||
| const recorded: Array<RecordedCommand> = [] | ||
| const logs: Array<string> = [] | ||
| const logger = Logger.make(({ message }) => { | ||
| logs.push(String(message)) | ||
| }) | ||
|
|
||
| yield* _(fs.makeDirectory(path.join(root, ".git"), { recursive: true })) | ||
| yield* _(fs.writeFileString(indexLockPath, "locked\n")) | ||
|
|
||
| yield* _( | ||
| autoSyncState("chore(state): test").pipe( | ||
| Effect.provideService(CommandExecutor.CommandExecutor, makeFakeExecutor(recorded)), | ||
| Effect.provide(Logger.replace(Logger.defaultLogger, logger)) | ||
| ) | ||
| ) | ||
|
|
||
| expect(recorded.filter(isMutatingSyncGitCommand)).toEqual([]) | ||
| expect(logs.some((message) => message.includes("State auto-sync skipped: git index lock exists"))).toBe(true) | ||
| expect(yield* _(fs.exists(`${root}.lock`))).toBe(false) | ||
| }) | ||
| ) | ||
| ).pipe(Effect.provide(NodeContext.layer))) | ||
|
|
||
| it.effect("releases the state lock when non-strict auto-sync logs a git failure", () => | ||
| withTempDir((home) => | ||
| withPatchedEnv( | ||
| { | ||
| HOME: home, | ||
| DOCKER_GIT_PROJECTS_ROOT: undefined, | ||
| DOCKER_GIT_STATE_AUTO_SYNC: "1", | ||
| DOCKER_GIT_STATE_AUTO_SYNC_STRICT: undefined | ||
| }, | ||
| Effect.gen(function*(_) { | ||
| const fs = yield* _(FileSystem.FileSystem) | ||
| const path = yield* _(Path.Path) | ||
| const root = path.join(home, ".docker-git") | ||
| const recorded: Array<RecordedCommand> = [] | ||
| const logs: Array<string> = [] | ||
| const logger = Logger.make(({ message }) => { | ||
| logs.push(String(message)) | ||
| }) | ||
|
|
||
| yield* _(fs.makeDirectory(path.join(root, ".git"), { recursive: true })) | ||
|
|
||
| yield* _( | ||
| autoSyncState("chore(state): test").pipe( | ||
| Effect.provideService( | ||
| CommandExecutor.CommandExecutor, | ||
| makeFakeExecutor(recorded, (command) => | ||
| command.command === "git" && command.args[0] === "rm" ? 23 : 0 | ||
| ) | ||
| ), | ||
| Effect.provide(Logger.replace(Logger.defaultLogger, logger)) | ||
| ) | ||
| ) | ||
|
|
||
| expect(recorded.some((command) => command.command === "git" && command.args[0] === "rm")).toBe(true) | ||
| expect(logs.some((message) => message.includes("State auto-sync failed: git rm (exit 23)"))).toBe(true) | ||
| expect(yield* _(fs.exists(`${root}.lock`))).toBe(false) | ||
| }) | ||
| ) | ||
| ).pipe(Effect.provide(NodeContext.layer))) | ||
| }) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect repository test patterns and the target file context.
git ls-files | sed -n '1,200p'
printf '\n--- target file ---\n'
cat -n packages/lib/tests/usecases/state-repo-auto-sync.test.ts | sed -n '1,260p'
printf '\n--- search for fast-check usage in tests ---\n'
rg -n --glob '*.test.ts' --glob '*.test.tsx' 'fc\.property|fast-check|property-based' packages | sed -n '1,240p'
printf '\n--- search for this specific behavior elsewhere ---\n'
rg -n 'State auto-sync skipped: git index lock exists|State auto-sync failed: git rm \(exit 23\)|index\.lock|state auto-sync' packages | sed -n '1,240p'Repository: ProverCoderAI/docker-git
Length of output: 36931
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Focus on nearby code if the file is long.
wc -l packages/lib/tests/usecases/state-repo-auto-sync.test.ts
sed -n '1,220p' packages/lib/tests/usecases/state-repo-auto-sync.test.tsRepository: ProverCoderAI/docker-git
Length of output: 6973
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read surrounding test conventions if the target file alone is insufficient.
fd -a 'docs' . || true
rg -n --glob 'docs/**/*.md' --glob '*.md' 'fast-check|fc\.property|property-based|bug fix|reproducing test' . | sed -n '1,240p'Repository: ProverCoderAI/docker-git
Length of output: 802
Добавь property-based покрытие инвариантов auto-sync
packages/lib/tests/usecases/state-repo-auto-sync.test.ts:102-179 — сейчас здесь только два фиксированных сценария; добавь fc.property, который варьирует наличие index.lock и коды выхода git-команд, чтобы закрепить index.lock ⇒ mutating git commands = 0 и git failure ⇒ .lock отсутствует.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/lib/tests/usecases/state-repo-auto-sync.test.ts` around lines 102 -
179, Current tests only cover two fixed auto-sync cases; add a property-based
test around autoSyncState to vary whether .git/index.lock exists and the git
exit codes. Use fc.property with the existing helpers like withTempDir,
withPatchedEnv, makeFakeExecutor, and isMutatingSyncGitCommand to assert the
invariants that index.lock implies zero mutating git commands and that any git
failure still leaves the state .lock file absent. Keep the existing logging
assertions where relevant, but move the core expectations into the generated
property cases so the behavior is checked across many combinations.
Source: Coding guidelines
| set +e | ||
| timeout "${LOGIN_TIMEOUT_SECONDS}s" bash -lc 'cd "$1" && bun packages/app/dist/src/docker-git/main.js auth claude login' bash "$REPO_ROOT" \ | ||
| >"$LOG_FILE" 2>&1 | ||
| login_exit=$? | ||
| set -e | ||
|
|
||
| if [[ "$login_exit" -ne 0 ]]; then | ||
| cat "$LOG_FILE" >&2 || true | ||
| fail "docker-git auth claude login failed (exit: $login_exit)" | ||
| fi | ||
|
|
||
| if grep -Fq -- "$OAUTH_TOKEN_MARKER" "$LOG_FILE"; then | ||
| fail "expected OAuth token marker to be absent from auth claude login output" | ||
| fi | ||
|
|
||
| grep -Fq -- "Claude OAuth token saved" "$LOG_FILE" \ | ||
| || fail "expected saved-token warning in auth claude login output" | ||
|
|
||
| grep -Fq -- "live Claude API access is not yet verified" "$LOG_FILE" \ | ||
| || fail "expected diagnostic API probe warning in auth claude login output" | ||
|
|
||
| docker exec "$DOCKER_GIT_API_CONTAINER_NAME" \ | ||
| test -s "$ROOT/.orch/auth/claude/default/.oauth-token" \ | ||
| || fail "expected persisted Claude OAuth token in controller state" | ||
|
|
||
| echo "e2e/auth-claude-login: docker-backed Claude login warning path verified" >&2 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Покройте документированный web-flow и повторную проверку через status.
Сейчас e2e запускает auth claude login без --web и на этом останавливается, поэтому не проверяет контракт из README: web-вход должен идти через auth claude login --web, а состояние после warning нужно перепроверять auth claude status с проверкой, что токен не попадает в вывод. В текущем виде регрессия в CLI-маршрутизации --web или в redaction/status останется незамеченной.
🧪 Предлагаемый фикс
set +e
-timeout "${LOGIN_TIMEOUT_SECONDS}s" bash -lc 'cd "$1" && bun packages/app/dist/src/docker-git/main.js auth claude login' bash "$REPO_ROOT" \
+timeout "${LOGIN_TIMEOUT_SECONDS}s" bash -lc 'cd "$1" && bun packages/app/dist/src/docker-git/main.js auth claude login --web' bash "$REPO_ROOT" \
>"$LOG_FILE" 2>&1
login_exit=$?
set -e
@@
grep -Fq -- "live Claude API access is not yet verified" "$LOG_FILE" \
|| fail "expected diagnostic API probe warning in auth claude login output"
+
+STATUS_LOG_FILE="/tmp/docker-git-auth-claude-status-$RUN_ID.log"
+
+set +e
+timeout "${LOGIN_TIMEOUT_SECONDS}s" bash -lc 'cd "$1" && bun packages/app/dist/src/docker-git/main.js auth claude status' bash "$REPO_ROOT" \
+ >"$STATUS_LOG_FILE" 2>&1
+status_exit=$?
+set -e
+
+if [[ "$status_exit" -ne 0 ]]; then
+ cat "$STATUS_LOG_FILE" >&2 || true
+ fail "docker-git auth claude status failed (exit: $status_exit)"
+fi
+
+if grep -Fq -- "$OAUTH_TOKEN_MARKER" "$STATUS_LOG_FILE"; then
+ fail "expected OAuth token marker to be absent from auth claude status output"
+fiAs per path instructions, "follow the documented CLI usage: bun run docker-git auth claude login --web (web flow) and use docker-git auth claude status to re-check issues instead of expecting the login command to fail hard on transient probe problems. Also ensure any test/log assertions align with the documented guarantee that status does not print token values."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| set +e | |
| timeout "${LOGIN_TIMEOUT_SECONDS}s" bash -lc 'cd "$1" && bun packages/app/dist/src/docker-git/main.js auth claude login' bash "$REPO_ROOT" \ | |
| >"$LOG_FILE" 2>&1 | |
| login_exit=$? | |
| set -e | |
| if [[ "$login_exit" -ne 0 ]]; then | |
| cat "$LOG_FILE" >&2 || true | |
| fail "docker-git auth claude login failed (exit: $login_exit)" | |
| fi | |
| if grep -Fq -- "$OAUTH_TOKEN_MARKER" "$LOG_FILE"; then | |
| fail "expected OAuth token marker to be absent from auth claude login output" | |
| fi | |
| grep -Fq -- "Claude OAuth token saved" "$LOG_FILE" \ | |
| || fail "expected saved-token warning in auth claude login output" | |
| grep -Fq -- "live Claude API access is not yet verified" "$LOG_FILE" \ | |
| || fail "expected diagnostic API probe warning in auth claude login output" | |
| docker exec "$DOCKER_GIT_API_CONTAINER_NAME" \ | |
| test -s "$ROOT/.orch/auth/claude/default/.oauth-token" \ | |
| || fail "expected persisted Claude OAuth token in controller state" | |
| echo "e2e/auth-claude-login: docker-backed Claude login warning path verified" >&2 | |
| set +e | |
| timeout "${LOGIN_TIMEOUT_SECONDS}s" bash -lc 'cd "$1" && bun packages/app/dist/src/docker-git/main.js auth claude login --web' bash "$REPO_ROOT" \ | |
| >"$LOG_FILE" 2>&1 | |
| login_exit=$? | |
| set -e | |
| if [[ "$login_exit" -ne 0 ]]; then | |
| cat "$LOG_FILE" >&2 || true | |
| fail "docker-git auth claude login failed (exit: $login_exit)" | |
| fi | |
| if grep -Fq -- "$OAUTH_TOKEN_MARKER" "$LOG_FILE"; then | |
| fail "expected OAuth token marker to be absent from auth claude login output" | |
| fi | |
| grep -Fq -- "Claude OAuth token saved" "$LOG_FILE" \ | |
| || fail "expected saved-token warning in auth claude login output" | |
| grep -Fq -- "live Claude API access is not yet verified" "$LOG_FILE" \ | |
| || fail "expected diagnostic API probe warning in auth claude login output" | |
| STATUS_LOG_FILE="/tmp/docker-git-auth-claude-status-$RUN_ID.log" | |
| set +e | |
| timeout "${LOGIN_TIMEOUT_SECONDS}s" bash -lc 'cd "$1" && bun packages/app/dist/src/docker-git/main.js auth claude status' bash "$REPO_ROOT" \ | |
| >"$STATUS_LOG_FILE" 2>&1 | |
| status_exit=$? | |
| set -e | |
| if [[ "$status_exit" -ne 0 ]]; then | |
| cat "$STATUS_LOG_FILE" >&2 || true | |
| fail "docker-git auth claude status failed (exit: $status_exit)" | |
| fi | |
| if grep -Fq -- "$OAUTH_TOKEN_MARKER" "$STATUS_LOG_FILE"; then | |
| fail "expected OAuth token marker to be absent from auth claude status output" | |
| fi | |
| docker exec "$DOCKER_GIT_API_CONTAINER_NAME" \ | |
| test -s "$ROOT/.orch/auth/claude/default/.oauth-token" \ | |
| || fail "expected persisted Claude OAuth token in controller state" | |
| echo "e2e/auth-claude-login: docker-backed Claude login warning path verified" >&2 |
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 118-118: Expressions don't expand in single quotes, use double quotes for that.
(SC2016)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/e2e/auth-claude-login.sh` around lines 117 - 142, The e2e in
auth-claude-login.sh currently exercises the non-web login path and stops after
login, so it misses the documented Claude web-flow and the follow-up status
redaction check. Update the flow in the auth claude login test to invoke the web
variant of the login command, then call auth claude status to verify the
post-login state instead of relying only on login output. Keep the existing
token persistence checks, and add/assert that status output does not print the
OAuth token or token marker. Use the existing auth claude login, auth claude
status, and OAUTH_TOKEN_MARKER references to locate the test logic.
Source: Path instructions
Proof of fix:\n- Причина: AuthClaudeStatus парсился, но host API mode считал его unsupported и не имел /auth/claude/status endpoint.\n- Решение: добавлен controller/API status endpoint, app routing/client dispatch и OpenAPI контракт; статус читает controller state без запуска Claude CLI и без утечки секретов.\n- Доказательство: bun run --cwd packages/api test -- auth.test.ts openapi.test.ts; bun run --cwd packages/app test -- program-auth.test.ts parser-auth.test.ts; typecheck packages api/app/openapi.
|
Старая инфа: #440 |
Expose a nullable Claude account field in the controller status response and render it in the status message when safe identity metadata is available. Proof of fix: - Cause: Claude status reported only label/method, so a connected account could not be identified like Codex status. - Solution: read non-secret account metadata from persisted Claude config/credentials, keep token bytes out of the response, and update OpenAPI/contracts/tests. - Verification: bun run --cwd packages/api test -- auth.test.ts openapi.test.ts; bun run --cwd packages/app test -- program-auth.test.ts parser-auth.test.ts; bun run --cwd packages/lib lint.
AI Session BackupCommit: ae12482
|
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/lib/src/usecases/state-repo.ts (1)
159-159: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winИспользуйте sanitized failure logger здесь тоже.
Line 159 снова пишет
String(error), обходя новыйrenderStateSyncFailure; так в warning могут попасть абсолютные paths или command context. Замените на общий helper.Минимальный фикс
- onFailure: (error) => Effect.logWarning(`State auto-sync failed: ${String(error)}`), + onFailure: logStateAutoSyncFailure,As per coding guidelines, "Fail if changed files expose credentials, tokens, private-keys, or PII in source, generated config, logs, or CI output".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/lib/src/usecases/state-repo.ts` at line 159, The warning in the state auto-sync failure path is still formatting the raw error with String(error), which can leak sensitive details. Update the onFailure handler in state-repo to use the shared sanitized helper renderStateSyncFailure instead of logging the raw error, so the State auto-sync failed message is consistent and safe.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/api/src/http.ts`:
- Around line 1142-1150: The `/auth/claude/status` handler currently exposes
auth status without a guard, and `readClaudeAuthStatus()` can leak
`emailAddress`/`displayName` from local credentials. Protect this route with the
same auth check used for other sensitive `/auth/*` endpoints in
`HttpRouter.get`, or remove `account` fields from the payload returned via
`jsonResponse`. Use `readClaudeAuthStatus` and the existing route registration
in `packages/api/src/http.ts` to keep the fix localized.
In `@packages/api/src/services/auth.ts`:
- Around line 565-583: `resolveClaudeAuthMethod` in
`packages/api/src/services/auth.ts` duplicates the Claude auth detection logic
already present in `packages/lib/src/usecases/auth-claude-credentials.ts`, so
the source of truth is split. Extract a shared pure detector (for example,
`detectClaudeAuthMethod(fs, accountPath)`) in the lib usecase and have both the
API-side `resolveClaudeAuthMethod` and the existing lib
`resolveClaudeAuthMethod` call it, keeping the side-effectful sync/clear
behavior only in the lib wrapper.
In `@packages/api/tests/auth.test.ts`:
- Line 22: Add property-based coverage for readClaudeAuthStatus using
fast-check, since the current tests only cover fixed branches and serialization.
Extend the auth.test.ts suite with fc.property-based tests around the
readClaudeAuthStatus flow to validate invariants like connected matching method
!== "none" and label normalization being idempotent. Use the existing
readClaudeAuthStatus test setup and keep the new properties focused on the
returned status shape so they complement the current branch tests without
duplicating them.
In `@packages/app/tests/docker-git/program-auth.test.ts`:
- Around line 1-15: The new program auth test is using plain vitest `it` instead
of the required Effect test utilities. Update `program-auth.test.ts` to follow
the same pattern as `parser-auth.test.ts` by using `it.effect` with
`Effect.sync` (and the matching Effect imports) around the `isRoutedAuthCommand`
assertion, while keeping the test synchronous and free of async/await.
In `@packages/auth-oauth/src/claude-docker-oauth.ts`:
- Line 21: The claudeDockerOauthProbeConfigDir constant currently points to a
root-level path that may not be writable for non-root Docker runs, causing the
probe to fail when Claude tries to create HOME or CLAUDE_CONFIG_DIR. Update
claudeDockerOauthProbeConfigDir in claude-docker-oauth.ts to use a writable
location such as a /tmp-based directory, or ensure the directory is created and
owned appropriately before the probe runs in the claudeDockerOauth flow.
In `@packages/lib/src/usecases/auth-claude-credentials.ts`:
- Around line 58-95: The naming of resolveClaudeAuthMethod is misleading because
it performs filesystem side effects via syncClaudeCredentialsFile and
clearClaudeSessionCredentials instead of only resolving state. Rename the
function to reflect the normalize-and-resolve behavior, and add functional
comments marking syncClaudeCredentialsFile, clearClaudeSessionCredentials, and
the renamed resolver as PURITY: SHELL with a short EFFECT/invariant note,
consistent with persistClaudeOauthToken. Keep the behavior unchanged; just make
the side effects explicit in the symbol names and comments.
- Around line 96-124: `hasNonEmptyOauthToken` duplicates the same token lookup
and normalization logic already implemented in `readOauthToken`. Refactor
`hasNonEmptyOauthToken` to delegate to `readOauthToken(fs, accountPath)` and
return a boolean based on whether it yields a non-null value, keeping the
filesystem/file checks and trimming behavior centralized in `readOauthToken`.
In `@packages/lib/src/usecases/auth-claude.ts`:
- Line 49: The hardcoded claudeProbeConfigDir in auth-claude.ts should not point
to a top-level path that may be unwritable for non-root Docker runs. Update the
probe config directory setup to match claude-docker-oauth.ts by using a writable
/tmp-based location or by explicitly creating and preparing the directory with
the right ownership/permissions before login/status probes use it.
In `@packages/lib/src/usecases/state-repo/init.ts`:
- Line 114: The init usecase is reading the current working directory directly
via process.cwd(), which should be removed from Effect orchestration. Update the
init flow around resolveStateRoot so cwd is injected from the shell boundary or
an environment service passed into the usecase, and use that value instead of
calling process.cwd() inside product code. Keep the dependency explicit through
the existing init/usecase entrypoint and related service boundary so the logic
remains testable and Layer-based.
- Around line 122-123: The logging in the state repo initialization leaks
sensitive information by printing the absolute state directory and the raw
remote URL. Update the logging around the init flow in the state-repo usecase
(the Effect.log calls in init) to use a non-identifying alias for the state
location and a redacted remote string that removes credentials/userinfo from
input.repoUrl. Keep the log context useful, but avoid exposing root or any
embedded tokens in the remote output.
---
Outside diff comments:
In `@packages/lib/src/usecases/state-repo.ts`:
- Line 159: The warning in the state auto-sync failure path is still formatting
the raw error with String(error), which can leak sensitive details. Update the
onFailure handler in state-repo to use the shared sanitized helper
renderStateSyncFailure instead of logging the raw error, so the State auto-sync
failed message is consistent and safe.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8dfd9eba-02ec-45c2-b4cd-5cf7cfdd8c43
📒 Files selected for processing (21)
packages/api/src/api/contracts.tspackages/api/src/api/openapi.tspackages/api/src/http.tspackages/api/src/services/auth.tspackages/api/tests/auth.test.tspackages/api/tests/openapi.test.tspackages/app/src/docker-git/api-client-auth.tspackages/app/src/docker-git/api-client.tspackages/app/src/docker-git/program-auth.tspackages/app/src/docker-git/program-unsupported.tspackages/app/tests/docker-git/parser-auth.test.tspackages/app/tests/docker-git/program-auth.test.tspackages/auth-oauth/src/claude-docker-oauth.tspackages/auth-oauth/tests/claude-docker-oauth.test.tspackages/lib/src/usecases/auth-claude-credentials.tspackages/lib/src/usecases/auth-claude.tspackages/lib/src/usecases/state-repo.tspackages/lib/src/usecases/state-repo/init.tspackages/lib/tests/usecases/auth-claude-login.test.tspackages/openapi/openapi.jsonpackages/openapi/src/openapi-paths.ts
💤 Files with no reviewable changes (1)
- packages/app/src/docker-git/program-unsupported.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (10)
- GitHub Check: Lint
- GitHub Check: E2E (Browser command)
- GitHub Check: E2E (Login context)
- GitHub Check: E2E (Claude auth login)
- GitHub Check: E2E (Clone auto-open SSH)
- GitHub Check: E2E (OpenCode)
- GitHub Check: E2E (Clone cache)
- GitHub Check: E2E (Runtime volumes + SSH)
- GitHub Check: Test
- GitHub Check: Final build (windows-latest)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{sh,bash,py,js,ts,jsx,tsx,go,java,rb,php}
📄 CodeRabbit inference engine (Custom checks)
Fail if changed files introduce command injection or unsafe shell/process execution with user-controlled input
Files:
packages/app/src/docker-git/api-client.tspackages/api/src/api/contracts.tspackages/app/tests/docker-git/parser-auth.test.tspackages/api/tests/openapi.test.tspackages/app/tests/docker-git/program-auth.test.tspackages/openapi/src/openapi-paths.tspackages/app/src/docker-git/api-client-auth.tspackages/app/src/docker-git/program-auth.tspackages/api/tests/auth.test.tspackages/api/src/http.tspackages/api/src/api/openapi.tspackages/lib/src/usecases/state-repo/init.tspackages/lib/src/usecases/auth-claude-credentials.tspackages/api/src/services/auth.tspackages/lib/tests/usecases/auth-claude-login.test.tspackages/auth-oauth/tests/claude-docker-oauth.test.tspackages/auth-oauth/src/claude-docker-oauth.tspackages/lib/src/usecases/state-repo.tspackages/lib/src/usecases/auth-claude.ts
**/*.{py,js,ts,jsx,tsx,go,java,rb,php,sh,bash,c,cpp}
📄 CodeRabbit inference engine (Custom checks)
Fail if changed files introduce path traversal or writes outside intended project/container state directories
Files:
packages/app/src/docker-git/api-client.tspackages/api/src/api/contracts.tspackages/app/tests/docker-git/parser-auth.test.tspackages/api/tests/openapi.test.tspackages/app/tests/docker-git/program-auth.test.tspackages/openapi/src/openapi-paths.tspackages/app/src/docker-git/api-client-auth.tspackages/app/src/docker-git/program-auth.tspackages/api/tests/auth.test.tspackages/api/src/http.tspackages/api/src/api/openapi.tspackages/lib/src/usecases/state-repo/init.tspackages/lib/src/usecases/auth-claude-credentials.tspackages/api/src/services/auth.tspackages/lib/tests/usecases/auth-claude-login.test.tspackages/auth-oauth/tests/claude-docker-oauth.test.tspackages/auth-oauth/src/claude-docker-oauth.tspackages/lib/src/usecases/state-repo.tspackages/lib/src/usecases/auth-claude.ts
**/*.{js,ts,jsx,tsx,py,java,go,rb,php,sh,bash,yml,yaml,json,env*,toml,cfg,config,dockerfile,dockerignore}
📄 CodeRabbit inference engine (Custom checks)
Fail if changed files expose credentials, tokens, private-keys, or PII in source, generated config, logs, or CI output
Files:
packages/app/src/docker-git/api-client.tspackages/api/src/api/contracts.tspackages/app/tests/docker-git/parser-auth.test.tspackages/api/tests/openapi.test.tspackages/app/tests/docker-git/program-auth.test.tspackages/openapi/src/openapi-paths.tspackages/openapi/openapi.jsonpackages/app/src/docker-git/api-client-auth.tspackages/app/src/docker-git/program-auth.tspackages/api/tests/auth.test.tspackages/api/src/http.tspackages/api/src/api/openapi.tspackages/lib/src/usecases/state-repo/init.tspackages/lib/src/usecases/auth-claude-credentials.tspackages/api/src/services/auth.tspackages/lib/tests/usecases/auth-claude-login.test.tspackages/auth-oauth/tests/claude-docker-oauth.test.tspackages/auth-oauth/src/claude-docker-oauth.tspackages/lib/src/usecases/state-repo.tspackages/lib/src/usecases/auth-claude.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: FUNCTIONAL CORE: Write only pure functions with immutable data and mathematical operations in core modules; no side effects, mutations, or external service calls
IMPERATIVE SHELL: Isolate all side effects (IO, network, database, environment/process) in a thin SHELL layer; CORE never calls SHELL, only SHELL → CORE
Never useanytype annotation in TypeScript; useunknownonly at SHELL boundaries for decoding, never exportunknownoutside boundary modules
Never useastype assertions in normal code; only permitasin a single 'axiomatic' module (brands, constructors, constants) after which types flow safely without casts
Always use exhaustive pattern matching for union types through.exhaustive()orMatch.exhaustive()from effect-ts; never use switch statements or unhandled type branches
Use Effect<Success, Error, Requirements> monad from effect-ts for all effects; compose through pipe() and Effect.flatMap(); never use async/await, raw Promise chains (then/catch), or Promise.all in product code
Interoperate with Promise/exceptions only in SHELL through Effect.try/Effect.tryPromise with typed error mapping; never leave raw exceptions or untyped errors in the domain
Use Effect.acquireRelease + Effect.scoped for resource management with guaranteed finalization; never manage resources with try/finally or manual cleanup
All external services (database, HTTP, environment) must be accessed through Effect-based interfaces and Layer-based dependency injection; never call external APIs directly
Provide comprehensive TSDoc comments with mathematical notation:@pure,@effect,@invariant,@precondition,@postcondition,@complexity,@throws, and CHANGE/WHY/REF/SOURCE/FORMAT THEOREM functional comment markers
No console.*, process direct calls, or untyped environment access in product code; all such operations must be abstracted through Layer-based services in SHELL
Boundary data from external sources (HTTP, database, environment) must be decoded/valida...
Files:
packages/app/src/docker-git/api-client.tspackages/api/src/api/contracts.tspackages/app/tests/docker-git/parser-auth.test.tspackages/api/tests/openapi.test.tspackages/app/tests/docker-git/program-auth.test.tspackages/openapi/src/openapi-paths.tspackages/app/src/docker-git/api-client-auth.tspackages/app/src/docker-git/program-auth.tspackages/api/tests/auth.test.tspackages/api/src/http.tspackages/api/src/api/openapi.tspackages/lib/src/usecases/state-repo/init.tspackages/lib/src/usecases/auth-claude-credentials.tspackages/api/src/services/auth.tspackages/lib/tests/usecases/auth-claude-login.test.tspackages/auth-oauth/tests/claude-docker-oauth.test.tspackages/auth-oauth/src/claude-docker-oauth.tspackages/lib/src/usecases/state-repo.tspackages/lib/src/usecases/auth-claude.ts
**/{browser*,server*,app*,*.ts,*.js}
📄 CodeRabbit inference engine (README.md)
Web version must listen on 0.0.0.0 by default for accessibility across LAN devices
Files:
packages/app/src/docker-git/api-client.tspackages/api/src/api/contracts.tspackages/app/tests/docker-git/parser-auth.test.tspackages/api/tests/openapi.test.tspackages/app/tests/docker-git/program-auth.test.tspackages/openapi/src/openapi-paths.tspackages/app/src/docker-git/api-client-auth.tspackages/app/src/docker-git/program-auth.tspackages/api/tests/auth.test.tspackages/api/src/http.tspackages/api/src/api/openapi.tspackages/lib/src/usecases/state-repo/init.tspackages/lib/src/usecases/auth-claude-credentials.tspackages/api/src/services/auth.tspackages/lib/tests/usecases/auth-claude-login.test.tspackages/auth-oauth/tests/claude-docker-oauth.test.tspackages/auth-oauth/src/claude-docker-oauth.tspackages/lib/src/usecases/state-repo.tspackages/lib/src/usecases/auth-claude.ts
**/{cli*,command*,auto*,*.ts,*.tsx}
📄 CodeRabbit inference engine (README.md)
Implement auto-mode agent selection logic to choose Claude, Codex, Gemini, or Grok randomly from available authorized providers, or allow forced selection with --auto=
Files:
packages/app/src/docker-git/api-client.tspackages/api/src/api/contracts.tspackages/app/tests/docker-git/parser-auth.test.tspackages/api/tests/openapi.test.tspackages/app/tests/docker-git/program-auth.test.tspackages/openapi/src/openapi-paths.tspackages/app/src/docker-git/api-client-auth.tspackages/app/src/docker-git/program-auth.tspackages/api/tests/auth.test.tspackages/api/src/http.tspackages/api/src/api/openapi.tspackages/lib/src/usecases/state-repo/init.tspackages/lib/src/usecases/auth-claude-credentials.tspackages/api/src/services/auth.tspackages/lib/tests/usecases/auth-claude-login.test.tspackages/auth-oauth/tests/claude-docker-oauth.test.tspackages/auth-oauth/src/claude-docker-oauth.tspackages/lib/src/usecases/state-repo.tspackages/lib/src/usecases/auth-claude.ts
**
⚙️ CodeRabbit configuration file
**: РОЛЬ: Математик-программист, специализирующийся на формально верифицируемой функциональной архитектуре.ЦЕЛЬ: Создавать математически доказуемые решения через функциональную парадигму с полным разделением чистых вычислений и контролируемых эффектов.
МОДЕЛЬ РАССУЖДЕНИЯ:
- Не выдавать “личные мнения”. Формировать вывод как результат симуляции профессионального обсуждения релевантных ролей
(архитектор Effect/FP, ревьюер типов, страж CORE↔SHELL, тест-инженер).- Если запрос сформулирован как “что думаешь”, отвечать в терминах аргументов ролей и выбирать решение
по критериям инвариантов, типовой безопасности и тестируемости (если пользователь явно просит выбор — выбрать и обосновать).ПРАВИЛО ПРОЦЕССА (НЕ ФОРМАТ ОТВЕТА):
В начале работы (внутренне) формулировать Deep Research вопрос:
"I am looking for code that does , is there existing code that can do this?"
Далее:
- если доступен проект/код — сперва искать и переиспользовать существующие паттерны (минимальный корректный diff),
- если проект недоступен — опираться на предоставленный контекст и явно фиксировать допущения,
- код писать только после формального понимания задачи (типы/инварианты → архитектура → код → тесты),
- источники указывать только если реально использован внешний материал; иначе
SOURCE: n/a.ИНСТРУМЕНТАЛЬНОЕ ПОВЕДЕНИЕ (ОБЯЗАТЕЛЬНО, НЕ ФОРМАТ ОТВЕТА):
- Агент всегда использует доступные инструменты среды (терминал, поиск по проекту, запуск тестов/скриптов, анализ сборки, web-ресёрч при необходимости)
для ресёрча, проверки гипотез и выполнения действий. Приоритет: проверяемость, воспроизводимость, минимальный риск.- Агент не предлагает “гайд” как замену действия. Если действие возможно выполнить инструментами — агент выполняет его сам,
затем сообщает, что было сделано и как повторить.- Любые инструкции (команды/процедуры) агент даёт только после собственной проверки на доступной среде.
Если проверить невозможно — явно фиксирует ограничение и перечисляе...
Files:
packages/app/src/docker-git/api-client.tspackages/api/src/api/contracts.tspackages/app/tests/docker-git/parser-auth.test.tspackages/api/tests/openapi.test.tspackages/app/tests/docker-git/program-auth.test.tspackages/openapi/src/openapi-paths.tspackages/openapi/openapi.jsonpackages/app/src/docker-git/api-client-auth.tspackages/app/src/docker-git/program-auth.tspackages/api/tests/auth.test.tspackages/api/src/http.tspackages/api/src/api/openapi.tspackages/lib/src/usecases/state-repo/init.tspackages/lib/src/usecases/auth-claude-credentials.tspackages/api/src/services/auth.tspackages/lib/tests/usecases/auth-claude-login.test.tspackages/auth-oauth/tests/claude-docker-oauth.test.tspackages/auth-oauth/src/claude-docker-oauth.tspackages/lib/src/usecases/state-repo.tspackages/lib/src/usecases/auth-claude.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Ты строгий ревьюер SPEC DRIVEN DEVELOPMENT.Перед выводами изучи README.md, другие *.md файлы, linked issues,
PR description, PR comments/discussion и релевантную кодовую базу.Сверь изменения с исходным ТЗ/спекой и обсуждением. Флагай любой уход
от спеки, недокументированное изменение поведения, отсутствие тестов
для заявленного поведения и security-риск. Если спека не видна,
попроси автора добавить ее в issue или PR description.Проверь решение с точки зрения формальной верификации: какие инварианты,
предусловия и постусловия можно доказать математически, а где доказуемость
слабая. Оцени решение с точки зрения теории игр: устойчивы ли стимулы,
нет ли выгодного обхода правил, и какое решение было бы сильнее.
Files:
packages/app/src/docker-git/api-client.tspackages/api/src/api/contracts.tspackages/app/tests/docker-git/parser-auth.test.tspackages/api/tests/openapi.test.tspackages/app/tests/docker-git/program-auth.test.tspackages/openapi/src/openapi-paths.tspackages/openapi/openapi.jsonpackages/app/src/docker-git/api-client-auth.tspackages/app/src/docker-git/program-auth.tspackages/api/tests/auth.test.tspackages/api/src/http.tspackages/api/src/api/openapi.tspackages/lib/src/usecases/state-repo/init.tspackages/lib/src/usecases/auth-claude-credentials.tspackages/api/src/services/auth.tspackages/lib/tests/usecases/auth-claude-login.test.tspackages/auth-oauth/tests/claude-docker-oauth.test.tspackages/auth-oauth/src/claude-docker-oauth.tspackages/lib/src/usecases/state-repo.tspackages/lib/src/usecases/auth-claude.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.test.{ts,tsx}: Write property-based tests using fast-check (fc.property) to verify mathematical invariants; unit tests must use Effect test utilities without async/await
Every bug fix must be accompanied by a reproducing test case; the test must fail before the fix and pass after; document the Proof of Fix with root cause and solution
Files:
packages/app/tests/docker-git/parser-auth.test.tspackages/api/tests/openapi.test.tspackages/app/tests/docker-git/program-auth.test.tspackages/api/tests/auth.test.tspackages/lib/tests/usecases/auth-claude-login.test.tspackages/auth-oauth/tests/claude-docker-oauth.test.ts
**/{.env*,auth*,config*,setup*}
📄 CodeRabbit inference engine (README.md)
Store authentication tokens in env-file as GIT_AUTH_TOKEN__ / GIT_AUTH_USER__
Files:
packages/api/tests/auth.test.tspackages/lib/src/usecases/auth-claude-credentials.tspackages/api/src/services/auth.tspackages/lib/tests/usecases/auth-claude-login.test.tspackages/lib/src/usecases/auth-claude.ts
🧠 Learnings (3)
📚 Learning: 2026-05-22T21:08:18.083Z
Learnt from: skulidropek
Repo: ProverCoderAI/docker-git PR: 344
File: packages/app/src/docker-git/controller-compose.ts:34-40
Timestamp: 2026-05-22T21:08:18.083Z
Learning: In this repo’s docker-git controller compose generation, `${DOCKER_GIT_CONTROLLER_BUILD_SKILLER:-1}` should be treated as standard bash parameter expansion: when `DOCKER_GIT_CONTROLLER_BUILD_SKILLER` is unset, it defaults to the string "1". There is no "-1" mode. The runtime contract enforced by `packages/app/src/docker-git/controller-compose.ts` is: unset / "1" / "true" => output "1"; "0" / "false" => output "0". If review code shows branching/behavior for "-1" or any numeric value other than this 0/1 contract, flag it. Also ensure the Dockerfile ARG `DOCKER_GIT_CONTROLLER_BUILD_SKILLER` stays consistent with default `1`.
Applied to files:
packages/app/src/docker-git/api-client.tspackages/app/src/docker-git/api-client-auth.tspackages/app/src/docker-git/program-auth.ts
📚 Learning: 2026-06-10T10:21:51.286Z
Learnt from: konard
Repo: ProverCoderAI/docker-git PR: 386
File: packages/app/tests/docker-git/gridland-react-singleton.test.ts:0-0
Timestamp: 2026-06-10T10:21:51.286Z
Learning: In `packages/app/tests/docker-git` tests that inspect the `packages/terminal` workspace (e.g., for React-related versions), remember that `react-dom` is intentionally declared as a `devDependency` in `packages/terminal` (the terminal exports components but does not render to the DOM itself). Therefore, test assertions should not require `react-dom` to appear in `dependencies`; when resolving versions, accept either `dependencies` or `devDependencies` (as `resolveDepVersion` does in `gridland-react-singleton.test.ts`).
Applied to files:
packages/app/tests/docker-git/parser-auth.test.tspackages/app/tests/docker-git/program-auth.test.ts
📚 Learning: 2026-05-18T07:43:38.131Z
Learnt from: skulidropek
Repo: ProverCoderAI/docker-git PR: 313
File: packages/api/src/services/panel-cloudflare-tunnel.ts:123-135
Timestamp: 2026-05-18T07:43:38.131Z
Learning: In this repo’s ProverCoderAI/docker-git architecture, it is intentional for service modules under packages/api/src/services/ to call Node.js APIs directly (e.g., fs, child_process, fetch). Do not treat direct Node API usage in these service modules as a dependency-injection (DI) violation. This codebase provides the NodeContext at the program boundary (entry point) rather than injecting Context.Tag/Layer services into individual service modules; reviewers should only flag DI/context issues if the expected boundary setup is missing.
Applied to files:
packages/api/src/services/auth.ts
🔇 Additional comments (17)
packages/app/src/docker-git/api-client.ts (1)
31-31: LGTM!packages/app/tests/docker-git/parser-auth.test.ts (1)
53-63: LGTM!packages/openapi/src/openapi-paths.ts (1)
455-470: LGTM!Also applies to: 5163-5285
packages/openapi/openapi.json (1)
10458-10758: LGTM!packages/app/src/docker-git/api-client-auth.ts (1)
25-25: LGTM!Also applies to: 207-211
packages/app/src/docker-git/program-auth.ts (1)
9-9: LGTM!Also applies to: 34-58, 82-82, 162-165, 224-224
packages/api/src/api/contracts.ts (1)
302-309: LGTM!packages/api/tests/openapi.test.ts (1)
35-40: LGTM!packages/api/src/http.ts (1)
62-62: LGTM!Also applies to: 1142-1150
packages/api/src/api/openapi.ts (1)
254-267: LGTM!Also applies to: 618-622
packages/lib/src/usecases/auth-claude-credentials.ts (2)
1-24: LGTM!
35-56: 🩺 Stability & AvailabilityКомментарий неактуален
accountPathсоздаётся вwithClaudeAuthчерезfs.makeDirectory(accountPath, { recursive: true })до вызоваpersistClaudeOauthToken, поэтомуfs.makeTempDirectory({ directory: accountPath, ... })не падает из‑за отсутствующей директории.> Likely an incorrect or invalid review comment.packages/api/src/services/auth.ts (1)
37-41: LGTM!Also applies to: 68-68, 85-85, 523-563, 585-719
packages/lib/tests/usecases/auth-claude-login.test.ts (1)
311-312: LGTM!packages/auth-oauth/tests/claude-docker-oauth.test.ts (1)
105-106: LGTM!packages/lib/src/usecases/state-repo.ts (1)
8-8: LGTM!Also applies to: 33-33, 46-52, 219-219, 258-258
packages/lib/src/usecases/auth-claude.ts (1)
6-24: LGTM!Also applies to: 33-44, 179-197
| HttpRouter.get( | ||
| "/auth/claude/status", | ||
| Effect.gen(function*(_) { | ||
| const request = yield* _(HttpServerRequest.HttpServerRequest) | ||
| const label = new URL(request.url, "http://localhost").searchParams.get("label") | ||
| const status = yield* _(readClaudeAuthStatus(label)) | ||
| return yield* _(jsonResponse({ status }, 200)) | ||
| }).pipe(Effect.catchAll(errorResponse)) | ||
| ), |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Проверить, есть ли middleware авторизации, оборачивающий /auth маршруты
rg -n "HttpMiddleware|Authorization|authenticate" packages/api/src/http.ts packages/api/src -g '!**/*.test.ts'Repository: ProverCoderAI/docker-git
Length of output: 1743
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== program.ts outline ==\n'
ast-grep outline packages/api/src/program.ts --view expanded || true
printf '\n== http.ts outline (around auth routes) ==\n'
ast-grep outline packages/api/src/http.ts --view expanded | sed -n '1,220p' || true
printf '\n== auth status routes and middleware references ==\n'
rg -n '"/auth/[^"]+/status"|HttpMiddleware|withMiddlewares|middleware|Auth' packages/api/src/http.ts packages/api/src/program.tsRepository: ProverCoderAI/docker-git
Length of output: 16514
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== program.ts (lines 1-120) ==\n'
sed -n '1,120p' packages/api/src/program.ts
printf '\n== http.ts (routes around /auth and wrapper) ==\n'
sed -n '1100,1338p' packages/api/src/http.ts
printf '\n== readClaudeAuthStatus definition ==\n'
rg -n "readClaudeAuthStatus|type .*AuthStatus|account" packages/api/src/services packages/api/src -g '!**/*.test.ts'Repository: ProverCoderAI/docker-git
Length of output: 40037
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== packages/api/src/services/auth.ts (480-540, 708-720) ==\n'
sed -n '480,540p' packages/api/src/services/auth.ts
printf '\n---\n'
sed -n '708,720p' packages/api/src/services/auth.ts
printf '\n== packages/api/src/api/contracts.ts (290-315) ==\n'
sed -n '290,315p' packages/api/src/api/contracts.ts
printf '\n== route/app middleware search ==\n'
rg -n 'HttpMiddleware|serve\\(|withMiddleware|withMiddlewares|HttpServer\\.serve|auth.*middleware|require.*auth|ApiAuthRequiredError' packages/api/srcRepository: ProverCoderAI/docker-git
Length of output: 3284
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== auth / middleware search ==\n'
rg -n 'HttpMiddleware|HttpRouter\.(use|mount|group|prefix|prefixPath)|withMiddlewares|withMiddleware|authorize|authenticated|ApiAuthRequiredError' packages/api/src
printf '\n== makeRouter head and app assembly around withCoreRoutes ==\n'
sed -n '1040,1360p' packages/api/src/http.ts
printf '\n== Claude account extraction ==\n'
sed -n '600,705p' packages/api/src/services/auth.tsRepository: ProverCoderAI/docker-git
Length of output: 22509
Закрыть /auth/*/status от публичного доступа или убрать account из ответа. В packages/api/src/http.ts:1142-1150 эти маршруты живут без отдельного auth-guard, а readClaudeAuthStatus() может вернуть emailAddress/displayName из локальных credential-файлов; при веб-старте на 0.0.0.0 это раскрывает PII по LAN.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/api/src/http.ts` around lines 1142 - 1150, The `/auth/claude/status`
handler currently exposes auth status without a guard, and
`readClaudeAuthStatus()` can leak `emailAddress`/`displayName` from local
credentials. Protect this route with the same auth check used for other
sensitive `/auth/*` endpoints in `HttpRouter.get`, or remove `account` fields
from the payload returned via `jsonResponse`. Use `readClaudeAuthStatus` and the
existing route registration in `packages/api/src/http.ts` to keep the fix
localized.
Source: Path instructions
| const resolveClaudeAuthMethod = ( | ||
| fs: FileSystem.FileSystem, | ||
| path: Path.Path, | ||
| accountPath: string | ||
| ): Effect.Effect<ClaudeAuthMethod, PlatformError> => | ||
| Effect.gen(function*(_) { | ||
| const hasOauthToken = yield* _(readNonEmptyFile(fs, path.join(accountPath, ".oauth-token"))) | ||
| if (hasOauthToken) { | ||
| return "oauth-token" | ||
| } | ||
|
|
||
| const hasRootCredentials = yield* _(readNonEmptyFile(fs, path.join(accountPath, ".credentials.json"))) | ||
| if (hasRootCredentials) { | ||
| return "claude-ai-session" | ||
| } | ||
|
|
||
| const hasNestedCredentials = yield* _(readNonEmptyFile(fs, path.join(accountPath, ".claude", ".credentials.json"))) | ||
| return hasNestedCredentials ? "claude-ai-session" : "none" | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Дублирование resolveClaudeAuthMethod между packages/api/src/services/auth.ts и packages/lib/src/usecases/auth-claude-credentials.ts.
Здесь реализована собственная (read-only) версия определения метода Claude-аутентификации, при том что packages/lib/src/usecases/auth-claude-credentials.ts уже содержит функцию с тем же именем и почти той же логикой (.oauth-token → root .credentials.json → nested .claude/.credentials.json), но с побочными эффектами (clearClaudeSessionCredentials, syncClaudeCredentialsFile). Поскольку packages/api архитектурно может зависеть от packages/lib (а не наоборот — согласно README: "API работает только с LIB"), знание о наборе путей-кандидатов для проверки метода дублируется в двух местах без единого источника истины: если в lib добавится новый fallback-путь или изменится порядок проверки, services/auth.ts можно легко забыть обновить, и статус в API начнёт расходиться с реальным поведением CLI.
Предлагается вынести в auth-claude-credentials.ts чистую (без побочных эффектов) функцию вида detectClaudeAuthMethod(fs, accountPath): Effect<ClaudeAuthMethod, PlatformError>, которую использовали бы обе стороны: resolveClaudeAuthMethod (SHELL, с sync/clear) — вызывая её и затем нормализуя хранение, и readClaudeAuthStatus здесь — вызывая её напрямую без побочных эффектов.
Как указано в coding guidelines: "Дублирование кода и смысла недопустимо: перед новым кодом — Deep Research по существующим реализациям, одна единица смысла — одно место определения."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/api/src/services/auth.ts` around lines 565 - 583,
`resolveClaudeAuthMethod` in `packages/api/src/services/auth.ts` duplicates the
Claude auth detection logic already present in
`packages/lib/src/usecases/auth-claude-credentials.ts`, so the source of truth
is split. Extract a shared pure detector (for example,
`detectClaudeAuthMethod(fs, accountPath)`) in the lib usecase and have both the
API-side `resolveClaudeAuthMethod` and the existing lib
`resolveClaudeAuthMethod` call it, keeping the side-effectful sync/clear
behavior only in the lib wrapper.
Source: Coding guidelines
| logoutCodexAuth, | ||
| logoutGitAuth, | ||
| logoutGrokAuth, | ||
| readClaudeAuthStatus, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Тесты логически верны, но отсутствуют property-based тесты.
Новые тесты для readClaudeAuthStatus корректно проверяют все ветки (oauth-token, root/nested claude-ai-session, none), а также отсутствие секрета токена в сериализованном статусе (строка 548). Однако не добавлено ни одного property-based теста через fc.property для инвариантов вида connected === (method !== "none") или идемпотентности нормализации label.
Как указано в coding guidelines: "Write property-based tests using fast-check (fc.property) to verify mathematical invariants".
Also applies to: 518-634
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/api/tests/auth.test.ts` at line 22, Add property-based coverage for
readClaudeAuthStatus using fast-check, since the current tests only cover fixed
branches and serialization. Extend the auth.test.ts suite with fc.property-based
tests around the readClaudeAuthStatus flow to validate invariants like connected
matching method !== "none" and label normalization being idempotent. Use the
existing readClaudeAuthStatus test setup and keep the new properties focused on
the returned status shape so they complement the current branch tests without
duplicating them.
Source: Coding guidelines
| import { describe, expect, it } from "vitest" | ||
|
|
||
| import { isRoutedAuthCommand } from "../../src/docker-git/program-auth.js" | ||
|
|
||
| describe("program auth routing", () => { | ||
| it("routes Claude status through the controller in host API mode", () => { | ||
| expect( | ||
| isRoutedAuthCommand({ | ||
| _tag: "AuthClaudeStatus", | ||
| label: null, | ||
| claudeAuthPath: ".docker-git/.orch/auth/claude" | ||
| }) | ||
| ).toBe(true) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Тест написан в обычном стиле vitest, а не через Effect test utilities.
Файл целиком новый и тестирует isRoutedAuthCommand, используя it напрямую из vitest, без Effect-обёртки — в отличие от соседнего parser-auth.test.ts, где аналогичные тесты оформлены через it.effect + Effect.sync.
As per coding guidelines, **/*.test.{ts,tsx}: "unit tests must use Effect test utilities without async/await".
♻️ Пример приведения к стилю Effect (уточните точный импорт `it.effect`, используемый в parser-auth.test.ts)
-import { describe, expect, it } from "vitest"
+import { describe, expect } from "vitest"
+import { Effect } from "effect"
+// используйте тот же источник `it.effect`, что и в parser-auth.test.ts
+import { it } from "`@effect/vitest`"
import { isRoutedAuthCommand } from "../../src/docker-git/program-auth.js"
describe("program auth routing", () => {
- it("routes Claude status through the controller in host API mode", () => {
- expect(
- isRoutedAuthCommand({
- _tag: "AuthClaudeStatus",
- label: null,
- claudeAuthPath: ".docker-git/.orch/auth/claude"
- })
- ).toBe(true)
- })
+ it.effect("routes Claude status through the controller in host API mode", () =>
+ Effect.sync(() => {
+ expect(
+ isRoutedAuthCommand({
+ _tag: "AuthClaudeStatus",
+ label: null,
+ claudeAuthPath: ".docker-git/.orch/auth/claude"
+ })
+ ).toBe(true)
+ }))
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { describe, expect, it } from "vitest" | |
| import { isRoutedAuthCommand } from "../../src/docker-git/program-auth.js" | |
| describe("program auth routing", () => { | |
| it("routes Claude status through the controller in host API mode", () => { | |
| expect( | |
| isRoutedAuthCommand({ | |
| _tag: "AuthClaudeStatus", | |
| label: null, | |
| claudeAuthPath: ".docker-git/.orch/auth/claude" | |
| }) | |
| ).toBe(true) | |
| }) | |
| }) | |
| import { describe, expect } from "vitest" | |
| import { Effect } from "effect" | |
| import { it } from "`@effect/vitest`" | |
| import { isRoutedAuthCommand } from "../../src/docker-git/program-auth.js" | |
| describe("program auth routing", () => { | |
| it.effect("routes Claude status through the controller in host API mode", () => | |
| Effect.sync(() => { | |
| expect( | |
| isRoutedAuthCommand({ | |
| _tag: "AuthClaudeStatus", | |
| label: null, | |
| claudeAuthPath: ".docker-git/.orch/auth/claude" | |
| }) | |
| ).toBe(true) | |
| })) | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/app/tests/docker-git/program-auth.test.ts` around lines 1 - 15, The
new program auth test is using plain vitest `it` instead of the required Effect
test utilities. Update `program-auth.test.ts` to follow the same pattern as
`parser-auth.test.ts` by using `it.effect` with `Effect.sync` (and the matching
Effect imports) around the `isRoutedAuthCommand` assertion, while keeping the
test synchronous and free of async/await.
Source: Coding guidelines
|
|
||
| export const defaultClaudeDockerOauthImage = "docker-git-auth-claude:latest" | ||
| export const defaultClaudeDockerOauthContainerHome = "/claude-home" | ||
| const claudeDockerOauthProbeConfigDir = "/claude-probe-home" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Сделайте probe HOME writable для non-root Docker runs.
/claude-probe-home находится в корне FS; при запуске с поддерживаемым --user Claude не сможет создать HOME/CLAUDE_CONFIG_DIR, и probe даст ложный auth failure. Верните путь в writable область вроде /tmp/... либо создавайте/chown-ьте каталог через tmpfs/entrypoint перед probe.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/auth-oauth/src/claude-docker-oauth.ts` at line 21, The
claudeDockerOauthProbeConfigDir constant currently points to a root-level path
that may not be writable for non-root Docker runs, causing the probe to fail
when Claude tries to create HOME or CLAUDE_CONFIG_DIR. Update
claudeDockerOauthProbeConfigDir in claude-docker-oauth.ts to use a writable
location such as a /tmp-based directory, or ensure the directory is created and
owned appropriately before the probe runs in the claudeDockerOauth flow.
| const syncClaudeCredentialsFile = ( | ||
| fs: FileSystem.FileSystem, | ||
| path: Path.Path, | ||
| accountPath: string | ||
| ): Effect.Effect<void, PlatformError> => | ||
| Effect.gen(function*(_) { | ||
| const nestedPath = claudeNestedCredentialsPath(accountPath) | ||
| const rootPath = claudeCredentialsPath(accountPath) | ||
| const isNestedExists = yield* _(isRegularFile(fs, nestedPath)) | ||
| if (isNestedExists) { | ||
| const nestedText = yield* _(readFileStringIfPresent(fs, nestedPath)) | ||
| if (nestedText !== null) { | ||
| yield* _(writeFileStringEnsuringParent(fs, path, rootPath, nestedText)) | ||
| yield* _(fs.chmod(rootPath, 0o600), Effect.orElseSucceed(() => void 0)) | ||
| } | ||
| return | ||
| } | ||
|
|
||
| const isRootExists = yield* _(isRegularFile(fs, rootPath)) | ||
| if (isRootExists) { | ||
| const rootText = yield* _(readFileStringIfPresent(fs, rootPath)) | ||
| if (rootText === null) { | ||
| return | ||
| } | ||
| yield* _(writeFileStringEnsuringParent(fs, path, nestedPath, rootText)) | ||
| yield* _(fs.chmod(nestedPath, 0o600), Effect.orElseSucceed(() => void 0)) | ||
| } | ||
| }) | ||
|
|
||
| const clearClaudeSessionCredentials = ( | ||
| fs: FileSystem.FileSystem, | ||
| accountPath: string | ||
| ): Effect.Effect<void, PlatformError> => | ||
| Effect.gen(function*(_) { | ||
| yield* _(fs.remove(claudeCredentialsPath(accountPath), { force: true })) | ||
| yield* _(fs.remove(claudeNestedCredentialsPath(accountPath), { force: true })) | ||
| }) | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
resolveClaudeAuthMethod — misleading имя: функция "resolve" имеет побочные эффекты.
resolveClaudeAuthMethod (строки 126-141) не просто читает состояние — она вызывает clearClaudeSessionCredentials и syncClaudeCredentialsFile, которые удаляют/переписывают файлы credentials на диске. Именно из-за этих побочных эффектов пришлось писать отдельную read-only копию логики в packages/api/src/services/auth.ts (см. комментарий там) — это прямое следствие того, что имя функции не отражает её реального поведения (SHELL-эффект скрыт за именем в стиле "resolve"/query).
Предлагается переименовать в что-то вида normalizeAndResolveClaudeAuthMethod, а также добавить функциональные комментарии (PURITY: SHELL, EFFECT, INVARIANT) для syncClaudeCredentialsFile, clearClaudeSessionCredentials и resolveClaudeAuthMethod, как это уже сделано для persistClaudeOauthToken (строки 25-34).
Как указано в coding guidelines: функции SHELL-слоя должны иметь маркировку PURITY: SHELL и явное описание эффекта; имя должно однозначно отражать поведение.
Also applies to: 126-141
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/lib/src/usecases/auth-claude-credentials.ts` around lines 58 - 95,
The naming of resolveClaudeAuthMethod is misleading because it performs
filesystem side effects via syncClaudeCredentialsFile and
clearClaudeSessionCredentials instead of only resolving state. Rename the
function to reflect the normalize-and-resolve behavior, and add functional
comments marking syncClaudeCredentialsFile, clearClaudeSessionCredentials, and
the renamed resolver as PURITY: SHELL with a short EFFECT/invariant note,
consistent with persistClaudeOauthToken. Keep the behavior unchanged; just make
the side effects explicit in the symbol names and comments.
Source: Coding guidelines
| const hasNonEmptyOauthToken = ( | ||
| fs: FileSystem.FileSystem, | ||
| accountPath: string | ||
| ): Effect.Effect<boolean, PlatformError> => | ||
| Effect.gen(function*(_) { | ||
| const tokenPath = claudeOauthTokenPath(accountPath) | ||
| const hasToken = yield* _(isRegularFile(fs, tokenPath)) | ||
| if (!hasToken) { | ||
| return false | ||
| } | ||
| const tokenText = yield* _(fs.readFileString(tokenPath), Effect.orElseSucceed(() => "")) | ||
| return tokenText.trim().length > 0 | ||
| }) | ||
|
|
||
| export const readOauthToken = ( | ||
| fs: FileSystem.FileSystem, | ||
| accountPath: string | ||
| ): Effect.Effect<string | null, PlatformError> => | ||
| Effect.gen(function*(_) { | ||
| const tokenPath = claudeOauthTokenPath(accountPath) | ||
| const hasToken = yield* _(isRegularFile(fs, tokenPath)) | ||
| if (!hasToken) { | ||
| return null | ||
| } | ||
|
|
||
| const tokenText = yield* _(fs.readFileString(tokenPath), Effect.orElseSucceed(() => "")) | ||
| const token = tokenText.trim() | ||
| return token.length > 0 ? token : null | ||
| }) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Дублирование логики между hasNonEmptyOauthToken и readOauthToken.
Обе функции читают claudeOauthTokenPath(accountPath), проверяют isRegularFile, читают файл и делают trim(). hasNonEmptyOauthToken можно выразить через readOauthToken.
♻️ Устранение дублирования
-const hasNonEmptyOauthToken = (
- fs: FileSystem.FileSystem,
- accountPath: string
-): Effect.Effect<boolean, PlatformError> =>
- Effect.gen(function*(_) {
- const tokenPath = claudeOauthTokenPath(accountPath)
- const hasToken = yield* _(isRegularFile(fs, tokenPath))
- if (!hasToken) {
- return false
- }
- const tokenText = yield* _(fs.readFileString(tokenPath), Effect.orElseSucceed(() => ""))
- return tokenText.trim().length > 0
- })
-
-export const readOauthToken = (
+export const readOauthToken = (
fs: FileSystem.FileSystem,
accountPath: string
): Effect.Effect<string | null, PlatformError> =>
Effect.gen(function*(_) {
const tokenPath = claudeOauthTokenPath(accountPath)
const hasToken = yield* _(isRegularFile(fs, tokenPath))
if (!hasToken) {
return null
}
const tokenText = yield* _(fs.readFileString(tokenPath), Effect.orElseSucceed(() => ""))
const token = tokenText.trim()
return token.length > 0 ? token : null
})
+
+const hasNonEmptyOauthToken = (
+ fs: FileSystem.FileSystem,
+ accountPath: string
+): Effect.Effect<boolean, PlatformError> =>
+ readOauthToken(fs, accountPath).pipe(Effect.map((token) => token !== null))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const hasNonEmptyOauthToken = ( | |
| fs: FileSystem.FileSystem, | |
| accountPath: string | |
| ): Effect.Effect<boolean, PlatformError> => | |
| Effect.gen(function*(_) { | |
| const tokenPath = claudeOauthTokenPath(accountPath) | |
| const hasToken = yield* _(isRegularFile(fs, tokenPath)) | |
| if (!hasToken) { | |
| return false | |
| } | |
| const tokenText = yield* _(fs.readFileString(tokenPath), Effect.orElseSucceed(() => "")) | |
| return tokenText.trim().length > 0 | |
| }) | |
| export const readOauthToken = ( | |
| fs: FileSystem.FileSystem, | |
| accountPath: string | |
| ): Effect.Effect<string | null, PlatformError> => | |
| Effect.gen(function*(_) { | |
| const tokenPath = claudeOauthTokenPath(accountPath) | |
| const hasToken = yield* _(isRegularFile(fs, tokenPath)) | |
| if (!hasToken) { | |
| return null | |
| } | |
| const tokenText = yield* _(fs.readFileString(tokenPath), Effect.orElseSucceed(() => "")) | |
| const token = tokenText.trim() | |
| return token.length > 0 ? token : null | |
| }) | |
| export const readOauthToken = ( | |
| fs: FileSystem.FileSystem, | |
| accountPath: string | |
| ): Effect.Effect<string | null, PlatformError> => | |
| Effect.gen(function*(_) { | |
| const tokenPath = claudeOauthTokenPath(accountPath) | |
| const hasToken = yield* _(isRegularFile(fs, tokenPath)) | |
| if (!hasToken) { | |
| return null | |
| } | |
| const tokenText = yield* _(fs.readFileString(tokenPath), Effect.orElseSucceed(() => "")) | |
| const token = tokenText.trim() | |
| return token.length > 0 ? token : null | |
| }) | |
| const hasNonEmptyOauthToken = ( | |
| fs: FileSystem.FileSystem, | |
| accountPath: string | |
| ): Effect.Effect<boolean, PlatformError> => | |
| readOauthToken(fs, accountPath).pipe(Effect.map((token) => token !== null)) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/lib/src/usecases/auth-claude-credentials.ts` around lines 96 - 124,
`hasNonEmptyOauthToken` duplicates the same token lookup and normalization logic
already implemented in `readOauthToken`. Refactor `hasNonEmptyOauthToken` to
delegate to `readOauthToken(fs, accountPath)` and return a boolean based on
whether it yields a non-null value, keeping the filesystem/file checks and
trimming behavior centralized in `readOauthToken`.
| const hasCredentials = yield* _(isRegularFile(fs, claudeCredentialsPath(accountPath))) | ||
| return hasCredentials ? "claude-ai-session" : "none" | ||
| }) | ||
| const claudeProbeConfigDir = "/claude-probe-home" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Не используйте top-level probe config dir без подготовки прав.
Этот путь используется login/status probe; если Docker запускается non-root, /claude-probe-home нельзя создать/записать, и статус OAuth станет ложно negative. Синхронизируйте фикс с packages/auth-oauth/src/claude-docker-oauth.ts: writable /tmp/... либо явное создание/chown/tmpfs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/lib/src/usecases/auth-claude.ts` at line 49, The hardcoded
claudeProbeConfigDir in auth-claude.ts should not point to a top-level path that
may be unwritable for non-root Docker runs. Update the probe config directory
setup to match claude-docker-oauth.ts by using a writable /tmp-based location or
by explicitly creating and preparing the directory with the right
ownership/permissions before login/status probes use it.
| Effect.gen(function*(_) { | ||
| const fs = yield* _(FileSystem.FileSystem) | ||
| const path = yield* _(Path.Path) | ||
| const root = resolveStateRoot(path, process.cwd()) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Уберите прямой process.cwd() из Effect-оркестрации.
Line 114 читает процесс напрямую внутри usecase-слоя. Передавайте cwd из shell-boundary/сервиса окружения, чтобы эффект оставался управляемым и тестируемым. As per coding guidelines, "No console.*, process direct calls, or untyped environment access in product code; all such operations must be abstracted through Layer-based services in SHELL".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/lib/src/usecases/state-repo/init.ts` at line 114, The init usecase
is reading the current working directory directly via process.cwd(), which
should be removed from Effect orchestration. Update the init flow around
resolveStateRoot so cwd is injected from the shell boundary or an environment
service passed into the usecase, and use that value instead of calling
process.cwd() inside product code. Keep the dependency explicit through the
existing init/usecase entrypoint and related service boundary so the logic
remains testable and Layer-based.
Source: Coding guidelines
| yield* _(Effect.log(`State dir ready: ${root}`)) | ||
| yield* _(Effect.log(`Remote: ${input.repoUrl}`)) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Не логируйте абсолютный state path и raw remote URL.
root обычно содержит пользовательский home path, а input.repoUrl может включать embedded token/userinfo. Логируйте обезличенный alias и редактированный remote без credentials.
Возможный минимальный фикс
- yield* _(Effect.log(`State dir ready: ${root}`))
- yield* _(Effect.log(`Remote: ${input.repoUrl}`))
+ yield* _(Effect.log("State dir ready"))
+ yield* _(Effect.log("Remote configured"))As per coding guidelines, "Fail if changed files expose credentials, tokens, private-keys, or PII in source, generated config, logs, or CI output".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| yield* _(Effect.log(`State dir ready: ${root}`)) | |
| yield* _(Effect.log(`Remote: ${input.repoUrl}`)) | |
| yield* _(Effect.log("State dir ready")) | |
| yield* _(Effect.log("Remote configured")) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/lib/src/usecases/state-repo/init.ts` around lines 122 - 123, The
logging in the state repo initialization leaks sensitive information by printing
the absolute state directory and the raw remote URL. Update the logging around
the init flow in the state-repo usecase (the Effect.log calls in init) to use a
non-identifying alias for the state location and a redacted remote string that
removes credentials/userinfo from input.repoUrl. Keep the log context useful,
but avoid exposing root or any embedded tokens in the remote output.
Source: Coding guidelines
2 participants
Summary
--env-file, never via stdout or Docker argv; add writable tmpfs probe HOMEsetup-tokentoken extraction to realsk-ant-*tokens, and isolate local probe HOME from account settings/auth/*/statusaccount/path payloads to loopback clientstrue/falsefixture coverage, and Effect-style testsauth claude login --webparser/e2e coverage and follow-upauth claude statusverification without token disclosureValidation
bun run --cwd packages/auth-oauth test -- claude-docker-oauth.test.ts claude-local-smoke.test.ts claude-oauth-token.test.tsbun run --cwd packages/lib test -- auth-claude-login.test.ts auth-claude-local.test.ts state-repo-auto-sync.test.tsbun run --cwd packages/api test -- auth.test.ts auth-github-login-stream.test.tsbun run --cwd packages/app test -- controller-compose.test.ts parser-auth.test.ts program-auth.test.ts actions-github-oauth.test.tsbun run --cwd packages/auth-oauth typecheckbun run --cwd packages/lib typecheckbun run --cwd packages/api typecheckbun run --cwd packages/app typecheckbun run --cwd packages/openapi typecheckbun run --cwd packages/lib lint:effectbun run --cwd packages/api lint:effect(exit 0; existing warnings remain)bun run --cwd packages/app lint:effectgit diff --checkProof of fix
auth claude login --web/statuscoverage was incomplete, API status duplicated auth-method logic and exposed account metadata without locality checks, state repo locks could remain forever after a crash, compose validation missed isolated runtime overlays, and CI Bun retries could not run inside the job timeout.sk-ant-*; local and Docker probe envs are isolated; API status uses a shared read-only Claude detector plus loopback guard; state locks include reclaimable TTL metadata and sanitized logs; compose/runtime overlays are validated as regular files; CI retries fit the job timeout and terminate process groups; e2e/parser tests coverauth claude login --webfollowed byauth claude status.