Skip to content

[CI debug for #542] Stream Cloud Build logs to diagnose integration-test failure - #586

Closed
theo-michel wants to merge 14 commits into
mainfrom
claude/pr542-review-comments-w34h9y
Closed

[CI debug for #542] Stream Cloud Build logs to diagnose integration-test failure#586
theo-michel wants to merge 14 commits into
mainfrom
claude/pr542-review-comments-w34h9y

Conversation

@theo-michel

Copy link
Copy Markdown
Contributor

Summary

  • Debugging vehicle for the integration-test failure on feat(skills): Pick Anything Skill + Other changes #542 — not meant to merge as-is. This branch is feat(skills): Pick Anything Skill + Other changes #542's head (598f853) plus one CI change: the workflow uses gcloud beta builds submit, which streams Cloud Build logs (including run_integration_tests.sh output) into the Actions console. The stock gcloud builds submit only reports "build step 2 failed" because build logs go to Cloud Logging, invisible from GitHub.
  • Context: integration-test on feat(skills): Pick Anything Skill + Other changes #542 went red on 598f853 (previous commit 9d9911f was green 12 minutes earlier) and failed identically on a rerun, so it's deterministic. The stages of run_integration_tests.sh that run without ROS (exec-bit guard, brain_client/manipulation unit tests, webapp proxy tests) all pass locally on the same code, pointing at the ROS launch tests — this PR's CI run will show the actual failing output.
  • If the streamed logs identify a fix, it will be committed here for feat(skills): Pick Anything Skill + Other changes #542 to pick up.

Validation

  • I ran the relevant local validation, or explained why it was skipped: the purpose of this PR is the CI run; local no-ROS test stages pass (67+17 tests, exec-bit guard).
  • No simulator behavior, config, or asset changes (the one non-feat(skills): Pick Anything Skill + Other changes #542 commit touches only .github/workflows/integration-test.yml).
  • No simulator asset files or asset references changed.

https://claude.ai/code/session_01TUaxM36BuAa9upYeC3cVcR


Generated by Claude Code

theo-michel and others added 14 commits July 29, 2026 12:19
…es by annotation

One rule for all robot state, hardware, and composition: annotate it.
`mobility: Mobility`, `odom: Odometry`, `image: MainImage`, a sub-skill
(`gripper_open: GripperOpen`), or a trained policy
(`pick_socks = PhysicalSkill("pick_socks")`). Plain annotation = required —
the server waits (bounded per feed: cameras 3 s, battery 6 s, else 2 s) and
fails the run before execute() if nothing arrives, so skills need no
`if self.x is None` guards. `| None` (or a `= None` default) = best effort.
Reading an undeclared feed raises with the annotation to add.

The base Skill supplies what every file used to repeat: the class name is the
skill name (snake_cased), the docstring is guidelines(), plus self.fail(),
self.cancelled / check_cancelled(), per-run on_cancel() hooks dispatched on
the cancelling thread, self.feedback(), self.wait_for(), self.storage, and
None/str returns treated as success. Instances are per-run: constructed for
the run, disposed at its end — @resource declares expensive per-run objects
with generator-style teardown that runs while interfaces are still live.

Robot state arrives as typed, ROS-free values instead of raw dicts:
Odometry, Pose (map frame), Battery, Lidar (+ min_range sector helper), Arm,
Map (lazy .grid), JointStates, HeadState, Image/MainImage/WristImage (the
value is the b64 string, .jpeg gives bytes), DepthMap (numpy). Legacy dict
access keeps working via dictcompat.LegacyMapping, soft-deprecated with a
warning. Main/wrist frames are memoized per frame in the 50 Hz injection
path, and CameraProvider.start(feeds) subscribes only the declared feeds, so
raw depth no longer streams for skills that never asked for it.

Sub-skills are constructed per run, wired like a root skill, share the
parent's cancel latch, and sit on the attribute as a callable that raises
SkillFailed/SkillCancelled instead of returning a status; PhysicalSkill moves
an unknown-id error to run start. Cancel dispatch stays outside the execution
lock so on_cancel hooks can't deadlock the server, and cancels landing
between slot claim and run wiring are latched and applied.

Shared helpers move into the platform: innate.gemini (cancel-aware retries),
innate.geometry (verified bit-identical to the math it replaces),
innate.vision, and the Arm* exceptions on the manipulation interface.

pyrightconfig.json scopes pyright to the skills tree, which is standard-mode
clean — catching move_to_cartesian_pose(duration: int) while every caller
passed fractional seconds. common/logging.py defers the `launch` import that
swapped the global logger class as a side effect (breaking stdlib propagation
and pytest's caplog for later loggers).

Covered by test_ambient_robot_state.py (declaration → injection → gating,
optional vs required, legacy dict compat, resource lifecycle, the 50 Hz
update thread, per-feed grace timing, on_cancel dispatch, invoker
Stop-vs-watchdog race), test_skill_composition.py, and test_cancel_latch.py.
…n drop-in packages

Skills directories under workspace/ are ordinary Python packages, imported
rather than scanned: workspace/ goes on sys.path, every module is imported,
and defining a Skill subclass *is* the registration (like a PyTorch
nn.Module). The class is the identity, so files organize freely — several
skills per file, a skill split across a subpackage (innate_skills/wave/,
pick_socks/), helpers next to the skills — and packages import each other by
bare name (from innate_skills import arm_utils). Skill-id collision behavior
is unchanged from main.

innate_skills/ (shipped) and custom_skills/ (yours, gitignored) are just the
two built-in packs; any other folder dropped into workspace/ — or symlinked
in (ln -s /opt/team/skills workspace/team_skills) — is a pack, ids namespaced
by folder name (<pack>/<name>). This replaces the 0.6.x extra_skill_dirs /
extra_agent_dirs setting (settings knob, webapp catalog entry, and the
media-route serving lanes), and 0.7 no longer scans ~/skills / ~/agents in
place — migrate_user_data.sh moves both home lanes into workspace/custom_*
so nothing silently stops loading (sudo-safe via ACTUAL_HOME).

A module that fails to import no longer vanishes: it shows up in the web-app
skills menu as a disabled row carrying its load error (SkillInfo.load_error),
is never registered with the cloud agent, and clears when fixed. Hot reload
(/brain/reload_primitives or the save-watcher) covers helper modules too.

The old scan-based skill loader, the string-based composition path, and the
legacy import shims (agent_types, input_types, skill_types, logging_config)
are deleted along with test_backwards_compat.py, which guarded them.
dynamic_loader stays for agents and inputs only.

Covered by test_skill_packages.py, test_broken_skills_catalog.py,
test_workspace_hot_reload.py, and test_dynamic_loader_helpers.py, all wired
into the no-ROS CI bucket.
Each skill now declares what it consumes as annotations, drops its name/
guidelines/cancel boilerplate into the base class (docstring = guidelines),
and reads typed state (odom.x, arm.gripper) instead of raw dicts. Sub-skill
calls go through declared composition — new gripper_open / gripper_close
skills replace the stringly-typed gripper routines, and arm_rest_position
joins arm_zero_position as a plain declared skill.

Skill-side helpers live next to the skills as ordinary modules
(innate_skills/arm_utils.py), imported by bare package name.
The first skill written against the new API at full size. After the base
parks the object in the head-camera pick box:

1. Search pose — arm to an operator-posed position (up high, elbow ~90°,
   wrist camera looking down), captured joints in WRIST_SEARCH_ARM.
2. Seed — Gemini returns a tight box around the object; its center is the
   mark.
3. Servo down — center-in-box (nudge x/y), descend one step, repeat, until
   wrist_stop_z, where the existing blind ladder finishes the grasp, then
   close/twist/lift. Persistent tracking loss buys a Gemini re-seed
   (budget = wrist_steps − 1).

Tracking is HSV color segmentation + CamShift, not optical flow: during the
descent the object grows ~2.5× in the wrist image and fabric deforms, which
slides LK patches onto the carpet (failed repeatedly in live testing). A
likelihood-ratio color model (object vs the floor immediately around it)
back-projected each frame is scale- and deformation-proof, and its score
honestly reports occlusion/loss instead of confidently tracking carpet. An
object whose color ≈ the floor gives a flat model → early "lost track" →
blind-descent fallback.

Cancel semantics: a Stop cancels the search-pose move too, and once the
fingers commit the skill never releases a grasped object — it folds with the
grip kept. The grasp-verify prompt labels images by the camera that actually
supplied them.

Tuning is by editing PARAMS + hot-reload; the observable surface is the log
lines (localization fixes, wrist-stage exit reasons, the grasp verdict).
wrist_kx/wrist_ky signs are unverified against the physical wrist-cam mount;
divergence is bounded by wrist_step_max and the reach clamp, and
wrist_steps=0 falls back to the proven blind grasp.
Closing uses a goal_current cap instead of software force loops, so grip
strength is a hardware limit rather than a control loop; strength/percent
are clamped to hardware-safe ranges. arm_control's gain_mode_ is atomic with
a per-waypoint re-assert (idle-decay TOCTOU).
…party isort

- post_update.sh: enforce root:<user> 640 on an existing /etc/innate.env — a
  hand-created 600 root:root file is unreadable to the non-root launch
  readers, silently dropping the service key (proxy "not configured").
- Webapp: press j to hide/show the on-screen joystick (releases any latched
  command on hide).
- ruff isort now knows workspace, innate, innate_proxy as first-party;
  re-sort the four files whose import blocks the old config had misfiled.
… state/

SkillOutput becomes a proper result object instead of a str subclass with
.data bolted on: .message, .status (SkillResult enum, never a bare string),
.data for structured payloads, and an .ok shorthand. str(output) keeps the
old text so existing call sites read the same.

The robot-state value types move out of skills/ into their own state/
package -- arm, battery, head, image, joint_states, lidar, map, odometry,
pose, dictcompat. They were never skill machinery, only the values skills
consume.

Workspace skills are regrouped into packages (arm/, chess/, email/) now
that a folder is a namespace.

Drops the six test modules added earlier on this branch and their entries
in ci/run_integration_tests.sh. The no-ROS unit bucket is back to
test_fake_cloud_selftest.py and test_config_validation.py.
Runs the pinned pre-commit formatter (ruff 0.15.15) over the files the
refactor touched. Line wrapping and import ordering only, no behaviour
change.
Agents can now list skill classes directly (from innate_skills.x import Y)
instead of id strings — typed, so a rename or missing skill is an editor
error, not a silent registration miss. Id strings still work and remain
the only form for physical skills, which are data with no class.

Normalization happens in one place, Agent.skill_ids(), via the same
skill_id_for_class derivation the catalog uses — so the cloud registration
payload, webapp JSON, and active-skill filtering are byte-identical to
before; classes never cross the wire. AgentLoader now puts workspace
packages on sys.path so agent files can import skill classes in the
brain process.
…s too

Physical skills are data (metadata.json + checkpoint) with no class to
import, so agents were stuck writing slug-derived id strings for exactly
the skills whose ids are hardest to guess. The catalog now generates
workspace/physical_skills/ on every roster publish: one TrainedSkill
subclass per physical skill, docstring carrying its guidelines, exact id
embedded (slugs aren't invertible). Agents and skills import them like
any other skill:

    from physical_skills import PickSocks
    get_skills(): return [NavigateToPosition, PickSocks]
    pick: PickSocks            # skill declaration, same as a code sub-skill

A module __getattr__ in the generated file turns a deleted/renamed skill
into a did-you-mean error listing what exists. Writes are content-compared
(no watcher loops) and atomic; generation failure logs and never blocks
skill loading. local/ wins class-name collisions, mirroring the plain-
display-name precedence. The package is gitignored (robot-generated) and
excluded from skill-package scanning.
…ring

All eight innate_agents now list skill classes: code skills import from
innate_skills.*, and demo_agent's recorded wave comes from the generated
physical_skills package. Verified every agent still loads and produces
byte-identical skill ids to the strings it replaced.

Referencing a generated package from a *shipped* agent needs the package
to exist, so agent init now regenerates it from the roster it already
receives before importing any agent file — previously demo_agent would
silently fail to load if the skills server hadn't published yet. Both
writers content-compare, so whichever runs second is a no-op; the tmp
file is pid-suffixed so the two processes can't interleave.
…ping fixes

get_inputs() now accepts InputDevice classes alongside name strings
(InputRef, mirroring SkillRef), normalized by Agent.input_names() before
the input manager consumes them. Resolution goes through the new
input_name_for_class() in inputs/types.py, which the InputLoader now uses
too, so a class reference and the loader's registered name cannot drift.

Also riding along:
- Agent.source / Skill.source annotated with the existing Source Literal
- display_icon_data declared on Agent instead of stamped dynamically
- lowercase-`any` annotations in the agent loader fixed to dict[str, dict],
  and initialize_agents() typed as returning Agent instances
integration-test fails on 598f853 with only 'build step 2 failed' visible —
the real test output goes to Cloud Logging, which the Actions log doesn't
show. gcloud beta builds submit streams those logs, making the failing
stage of run_integration_tests.sh readable from GitHub.

Claude-Session: https://claude.ai/code/session_01TUaxM36BuAa9upYeC3cVcR
The runner's gcloud has no beta group; the interactive component install
prompt fails in CI. Preinstall it via setup-gcloud.

Claude-Session: https://claude.ai/code/session_01TUaxM36BuAa9upYeC3cVcR

Copy link
Copy Markdown
Contributor Author

Closing — this debug PR did its job. Findings:

One thing worth keeping from this branch: the workflow change to gcloud beta builds submit (+ install_components: beta). Today a Cloud Build test failure surfaces in the Actions log as just build step 2 failed because the real output goes to Cloud Logging; the beta command streams those logs into the Actions console, which would have made this whole investigation a two-minute log read. Happy to open it as its own small PR if wanted.


Generated by Claude Code

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.

3 participants