Skip to content

feat(codeagent): update Dockerfiles and refactor session management for improved container handling #20

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 8, 2025

Conversation

go-wyvern
Copy link
Contributor

No description provided.

@Copilot Copilot AI review requested due to automatic review settings July 7, 2025 11:18
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enhances session handling by adding a dedicated SessionPath, refactors in-memory session management to use string keys, and updates Docker integrations to run containers persistently rather than interactively.

  • Add SessionPath to Workspace and create a corresponding directory.
  • Change SessionManager map keys from int to string to support multi-field identifiers.
  • Update Gemini and Claude Docker runners to launch detached containers and adjust Dockerfiles to use tail -f /dev/null.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
pkg/models/workspace.go Added SessionPath field to track per-session directory.
internal/workspace/manager.go Initialize and create sessionPath, include it in returned struct.
internal/code/session.go Switched session map to map[string]Code and updated key logic.
internal/code/gemini_docker.go Removed interactive pipes, added container naming, and exec-based prompts.
internal/code/claude.go Mirror Gemini changes for Claude containers.
Dockerfile.gemini & Dockerfile.claude Changed CMD to keep containers alive by tailing /dev/null.
Comments suppressed due to low confidence (3)

internal/workspace/manager.go:170

  • New sessionPath creation logic should be covered by unit tests to ensure directory creation errors are handled correctly.
	sessionPath := filepath.Join(m.baseDir, session)

internal/code/session.go:26

  • The Workspace type has no PullRequest field, so referencing workspace.PullRequest will not compile. Consider using workspace.Issue.GetNumber() or adding a PullRequest field.
	key := fmt.Sprintf("%s-%d", workspace.Repository, workspace.PullRequest.GetNumber())

internal/code/gemini_docker.go:24

  • Using workspace.Repository directly in container names may introduce invalid characters (e.g., slashes). Sanitize or replace non-alphanumeric characters.
	containerName := fmt.Sprintf("gemini-%s-%d", workspace.Repository, workspace.PullRequest.GetNumber())

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
CarlJi and others added 2 commits July 7, 2025 19:25
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@CarlJi CarlJi merged commit b1f50bd into qiniu:main Jul 8, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants