-
Notifications
You must be signed in to change notification settings - Fork 0
feat(m1): skill export + cloud sync fixes #144
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 | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -201,11 +201,16 @@ def _post(self, path: str, payload: dict, timeout: float = 10.0) -> dict | None: | |||||||||||||||||||
| with urllib.request.urlopen(req, timeout=timeout) as resp: | ||||||||||||||||||||
| body = resp.read().decode() | ||||||||||||||||||||
| return json.loads(body) if body else {} | ||||||||||||||||||||
| except (urllib.error.URLError, urllib.error.HTTPError, OSError) as e: | ||||||||||||||||||||
| log.debug("cloud POST %s failed: %s", path, e) | ||||||||||||||||||||
| except urllib.error.HTTPError as e: | ||||||||||||||||||||
| # Surface HTTP errors at WARNING — silent 4xx/5xx is how the | ||||||||||||||||||||
| # 'last_sync never updates' bug hid for months. | ||||||||||||||||||||
| log.warning("cloud POST %s failed: HTTP %s %s", path, e.code, e.reason) | ||||||||||||||||||||
| return None | ||||||||||||||||||||
| except (urllib.error.URLError, OSError) as e: | ||||||||||||||||||||
| log.warning("cloud POST %s failed (network): %s", path, e) | ||||||||||||||||||||
| return None | ||||||||||||||||||||
| except json.JSONDecodeError: | ||||||||||||||||||||
| log.debug("cloud response non-JSON for %s", path) | ||||||||||||||||||||
| log.warning("cloud response non-JSON for %s", path) | ||||||||||||||||||||
| return {} | ||||||||||||||||||||
|
|
||||||||||||||||||||
| def sync_metrics(self, payload: TelemetryPayload) -> bool: | ||||||||||||||||||||
|
|
@@ -215,7 +220,10 @@ def sync_metrics(self, payload: TelemetryPayload) -> bool: | |||||||||||||||||||
| """ | ||||||||||||||||||||
| if not self.enabled: | ||||||||||||||||||||
| return False | ||||||||||||||||||||
| result = self._post("/telemetry/metrics", asdict(payload)) | ||||||||||||||||||||
| # Backend mounts the metrics router under /api/v1 (see | ||||||||||||||||||||
| # cloud/app/main.py → app.include_router(router, prefix="/api/v1") | ||||||||||||||||||||
| # and cloud/app/routes/metrics.py → @router.post("/telemetry/metrics")). | ||||||||||||||||||||
| result = self._post("/api/v1/telemetry/metrics", asdict(payload)) | ||||||||||||||||||||
|
Comment on lines
+223
to
+226
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. Critical: Double The The path should be 🐛 Proposed fix- # Backend mounts the metrics router under /api/v1 (see
- # cloud/app/main.py → app.include_router(router, prefix="/api/v1")
- # and cloud/app/routes/metrics.py → `@router.post`("/telemetry/metrics")).
- result = self._post("/api/v1/telemetry/metrics", asdict(payload))
+ # Backend mounts the metrics router under /api/v1 (see
+ # cloud/app/main.py → app.include_router(router, prefix="/api/v1")
+ # and cloud/app/routes/metrics.py → `@router.post`("/telemetry/metrics")).
+ # api_base already includes /api/v1, so we only append the route path.
+ result = self._post("/telemetry/metrics", asdict(payload))📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||
| if result is not None: | ||||||||||||||||||||
| self.config.last_sync_at = payload.sent_at | ||||||||||||||||||||
| save_config(self.brain_dir, self.config) | ||||||||||||||||||||
|
|
@@ -231,7 +239,9 @@ def contribute_corpus(self, anonymized_patterns: list[dict]) -> bool: | |||||||||||||||||||
| """ | ||||||||||||||||||||
| if not self.enabled or not self.config.contribute_corpus: | ||||||||||||||||||||
| return False | ||||||||||||||||||||
| result = self._post("/corpus/contribute", {"patterns": anonymized_patterns}) | ||||||||||||||||||||
| # Backend mounts the corpus router under /api/v1 (same prefix as | ||||||||||||||||||||
| # telemetry — see cloud/app/main.py). | ||||||||||||||||||||
| result = self._post("/api/v1/corpus/contribute", {"patterns": anonymized_patterns}) | ||||||||||||||||||||
|
Comment on lines
+242
to
+244
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. Same double-prefix issue for corpus contribution path. 🐛 Proposed fix- # Backend mounts the corpus router under /api/v1 (same prefix as
- # telemetry — see cloud/app/main.py).
- result = self._post("/api/v1/corpus/contribute", {"patterns": anonymized_patterns})
+ # Backend mounts the corpus router under /api/v1 (same prefix as
+ # telemetry — see cloud/app/main.py).
+ # api_base already includes /api/v1, so we only append the route path.
+ result = self._post("/corpus/contribute", {"patterns": anonymized_patterns})📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||
| return result is not None | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
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.
Resolve brain root once to avoid cross-brain exports
Line 1034 resolves
brain_rootvia_resolve_brain_root()(default./brain), but Lines 1037-1038 resolvelessons_pathvia_get_brain()(defaultcwd). That can export rules from one brain while loading meta-principles from another.Suggested fix
Based on learnings: cli.py brain resolution precedence should be
GRADATA_BRAIN > --brain-dir > cwd.📝 Committable suggestion
🤖 Prompt for AI Agents