Repository navigation
Codex/feat wework local devices #1268
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ | |
| from sqlalchemy.orm import Session | ||
|
|
||
| from app.models.project import Project | ||
| from app.schemas.device import DeviceType | ||
| from app.schemas.project import ( | ||
| ProjectConfig, | ||
| ProjectDeviceSessionResponse, | ||
|
|
@@ -21,6 +22,7 @@ | |
| DeviceSessionNotFoundError, | ||
| local_device_session_service, | ||
| ) | ||
| from app.services.device_service import device_service | ||
|
|
||
| ProjectSessionType = Literal["terminal", "code_server"] | ||
|
|
||
|
|
@@ -50,6 +52,7 @@ async def start_project_device_session( | |
|
|
||
| project_config = _parse_project_config(project.config) | ||
| device_id = _get_bound_device_id(project_config) | ||
| _ensure_device_supports_project_sessions(db, user_id, device_id) | ||
| path, create_if_missing = _get_project_path( | ||
| project_config, project.config, project_id | ||
| ) | ||
|
|
@@ -126,6 +129,27 @@ def _get_bound_device_id(config: ProjectConfig) -> str: | |
| return config.execution.deviceId | ||
|
|
||
|
|
||
| def _ensure_device_supports_project_sessions( | ||
| db: Session, | ||
| user_id: int, | ||
| device_id: str, | ||
| ) -> None: | ||
| """Reject project terminal and code-server sessions for local devices.""" | ||
|
|
||
| device_kind = device_service.get_device_by_device_id(db, user_id, device_id) | ||
| if not device_kind: | ||
| return | ||
|
|
||
| spec = getattr(device_kind, "json", None) | ||
| spec = spec.get("spec", {}) if isinstance(spec, dict) else {} | ||
| device_type = spec.get("deviceType") | ||
| if device_type == DeviceType.LOCAL.value: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_400_BAD_REQUEST, | ||
| detail="Local devices do not support terminal or code-server sessions", | ||
| ) | ||
|
Comment on lines
+145
to
+150
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fail closed on This guard currently rejects only 🔧 Proposed fix- device_type = spec.get("deviceType")
- if device_type == DeviceType.LOCAL.value:
+ device_type = spec.get("deviceType")
+ if device_type != DeviceType.CLOUD.value:
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST,
- detail="Local devices do not support terminal or code-server sessions",
+ detail="Only cloud devices support terminal or code-server sessions",
)🤖 Prompt for AI Agents |
||
|
|
||
|
|
||
| def _get_project_path( | ||
| config: ProjectConfig, | ||
| raw_config: object, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add tracing on the new session-capability guard.
This new decision point should be instrumented with
@trace_syncand span attributes/events (e.g.,device_id, resolveddeviceType, rejection reason) for production diagnostics and policy auditing. As per coding guidelines, "Use OpenTelemetry tracing decorators:@trace_asyncor@trace_syncfor entire functions, andadd_span_event()orset_span_attribute()for adding details to spans".🤖 Prompt for AI Agents