fix: resolve lint errors from message-improvements#253
Conversation
…s and permissions integer
…oogleCloudPlatform#403) * Add `scion build` command for local harness-config image builds Introduces a top-level `scion build <harness-config-name>` CLI command that builds a container image from a Dockerfile bundled in a harness-config directory. Supports --tag, --base-image, --push, --platform, and --dry-run flags. After a successful build, updates the harness-config's config.yaml image field to reference the built image. Also fixes Dockerfile content hashing: Dockerfiles were previously excluded from ComputeHarnessConfigRevision, so changes to them did not trigger re-sync to the Hub. Removes Dockerfile from the skipBasenames exclusion list. Extracts detectContainerRuntime() from pkg/hub/maintenance_executors.go into a shared pkg/runtime/container.go so both the new build command and the Hub executor can use it. * Address PR review: yaml.Node config update, error handling, dedup settings H1: Replace destructive yaml.Unmarshal/Marshal round-trip with targeted yaml.Node edit for config.yaml image update. Preserves comments, field order, and unknown fields. M1: Handle all os.Stat errors on Dockerfile, not just IsNotExist. M2: Load settings once instead of twice when both --base-image is unset and --push is set. L3: Remove extra blank line in maintenance_executors.go. * Guard against empty harness-config path and non-mapping YAML nodes Add empty-path check after FindHarnessConfigDir to prevent synthetic harness-configs (e.g. 'generic') from resolving Dockerfile against CWD. Verify yaml.MappingNode kind before manipulating doc.Content[0] to handle malformed config.yaml gracefully. --------- Co-authored-by: Scion Agent (harness-local-build-p1) <agent@scion.dev>
* fix: test-login hardening, agent CLI access, and UI nits - M1: Distinguish store.ErrNotFound from transient DB errors in GetUserByEmail — return 500 for unexpected failures instead of silently creating duplicate users. - L1: Add http.MaxBytesReader(4096) body size limit. - L2: Validate email contains "@" before processing. - L3: Track whether displayName was explicitly provided so the default (email) doesn't overwrite an existing user's display name. - Add harness-config subcommands to the agentAllowed map so agents can manage harness configs via the CLI. Fixes 3 (file-browser badge) and 4 (page title) were already resolved in the current codebase. * style: fix gofmt alignment in cli_mode.go * ci: retry after flaky TestPipeline_LogHandlerRegistered port conflict * test: update cli_mode tests to expect harness-config in agentAllowed --------- Co-authored-by: Scion Agent (dev-followup-pr) <agent@scion.dev>
…GoogleCloudPlatform#401) * feat: wire hub OTel metric recorders to Cloud Monitoring The hub's dbmetrics and dispatchmetrics recorders were created with NewDisabled() — all OTel instruments recorded to no-op sinks. This wires them to a real GCP Cloud Monitoring MeterProvider during server startup, lighting up ~20 existing instruments (pg LISTEN/NOTIFY latency, notifications published/delivered/dropped, subscriber lag, dispatch lifecycle, pool stats). New package pkg/observability/hubmetrics provides NewMeterProvider() which creates a PeriodicReader (60s) with the GCP metric exporter. Metric groups (db-notify, db-pool, dispatch, hub-auth, hub-gcp) can be independently disabled via env vars using OTel View Drop(). Graceful degradation: when GCPProjectID is empty or the exporter fails, the hub logs a warning and continues with disabled recorders — identical to the previous behavior. * test: clean up TestIsGroupDisabled table-driven test Use the table's 'want' field directly instead of re-deriving the expected value in the assertion body. * fix: add 10s timeout to hub MeterProvider shutdown Prevents indefinite hang if the GCP metric exporter is unresponsive during server shutdown. * feat: replace hub in-memory counters with OTel instruments (Phase 2) Add OTelMetricsRecorder and OTelGCPTokenMetrics that implement the existing MetricsRecorder and new GCPTokenMetricsRecorder interfaces using OTel instruments for Cloud Monitoring export. Both use a dual-write pattern — OTel instruments for cloud export plus embedded atomic structs for the /api/metrics JSON snapshot endpoint. New metrics: scion.hub.auth.*, scion.hub.registration.count, scion.hub.join.*, scion.hub.rotation.count, scion.hub.dispatch.*, scion.hub.brokers.connected, scion.hub.gcp.token.*, scion.hub.gcp.iam.duration. Closes #240 * fix: address Phase 2 review findings (M1, M2, M3) M1: Add operation attribute to RecordDispatch OTel instruments so dispatch failures can be broken down by operation type. M2: Expand hub-auth metric group to cover all broker lifecycle instruments (registration, join, rotation, brokers, dispatch) — not just scion.hub.auth.*. M3: Gate SetMetrics(otelMetrics) on broker auth being enabled, preventing /api/metrics from showing an all-zeros broker block when auth is disabled. * feat: add pipeline health gauge, export error counter, and structured logging Phase 3 of metrics-delivery: add observability to the agent-side telemetry pipeline so we can confirm it's working end-to-end in production. - Add scion.telemetry.pipeline.status gauge (Int64, value=1) that self-reports via the cloud exporter on a 60s ticker, confirming the pipeline is alive - Add scion.telemetry.export.errors counter with signal and error_type attributes, incrementing on metric/span/log export failures - Add classifyError() to bucket errors into timeout/auth/quota/other - Upgrade credential logging at pipeline startup from log.Info format strings to structured slog.Info with credentials_file, source, project_id, provider, and cloud_configured fields - Add structured slog.Warn when cloud export is not configured, including the env var and well-known path that were checked - Fix slog handler in pkg/sciontool/log to render key=value attributes instead of silently dropping them - Add pipeline_health_test.go with tests for health gauge lifecycle, nil-safe export error recording, and classifyError bucketing * fix: shut down unused TracerProvider and LoggerProvider in initSelfMetrics initSelfMetrics() creates providers via NewProviders() but only needs the MeterProvider. The TracerProvider and LoggerProvider were never shut down, leaking goroutines. Shut them down immediately after creation. * fix(metrics-delivery): address errcheck and staticcheck CI failures - Replace os.Setenv+cleanup with t.Setenv in hubmetrics tests - Wrap standalone os.Unsetenv calls with error checks - Handle Shutdown errors on TracerProvider, LoggerProvider, MeterProvider - Check pipeline.Stop error in test cleanup - Convert switch to tagged form (staticcheck QF1002) - Fix MeterProvider leak on early return in startHealthGauge --------- Co-authored-by: Scion Agent (metrics-phase1-dev) <agent@scion.dev>
…ogleCloudPlatform#405) * fix: suppress commentary (TypeAssistantReply) messages in Discord Add unconditional early return in broker.Publish() to filter TypeAssistantReply before per-channel logic, covering all channels including those with ShowAssistantReply=true from the buggy setup default. Fix the setup default to false for new channel links. Clean up now-dead per-channel isAssistantReply check. * style: run gofmt on skill-related files inherited from main Fix formatting issues in files that were merged from main without gofmt, causing CI Build & Test check to fail. * fix: move commentary filter before webhook routing, remove dead branch Move the TypeAssistantReply early return before webhook routing and message formatting to avoid computing text that gets immediately discarded. Remove the now-unreachable TypeAssistantReply branch from the useWebhook condition. --------- Co-authored-by: Scion Agent (discord-commentary-filter-dev) <agent@scion.dev>
* Add hub build executor and web UI for harness-config images Phase 2: adds BuildHarnessConfigImageExecutor to build container images from a harness-config's bundled Dockerfile, with web UI for triggering builds, monitoring progress, and viewing logs. Includes all review fixes: - Path traversal containment check in file materialization - Uses shared scionruntime.DetectContainerRuntime() from Phase 1 - Uses hc.Slug (identifier-safe) for image names with Name fallback - Consecutive-error limit (5) in build status polling - Reactive checkbox binding (?checked) in build dialog - Property binding (.value) for tag input - Auto-scroll build log to bottom on new output * fix: address PR GoogleCloudPlatform#406 review comments --------- Co-authored-by: Scion Agent (harness-local-build-p2-fix) <agent@scion.dev>
…metrics) (GoogleCloudPlatform#407) Clarify the two distinct metric families in Scion: - Infrastructure metrics (scion.hub.*, scion.db.*, scion.dispatch.*) for platform health, produced by the Hub process - Agent metrics (gen_ai.*, agent.*) for harness/model telemetry, produced inside agent containers via the telemetry pipeline Also defines the Telemetry pipeline term. Co-authored-by: Scion Agent (metrics-architect) <agent@scion.dev>
…tform#410) Co-authored-by: Scion Agent (harness-build-blocker-fix) <agent@scion.dev>
* skill-bank M5a: add RoutingSkillResolver and scheme detection Introduce RoutingSkillResolver that groups SkillReferences by URI scheme and dispatches each group to a registered scheme-specific resolver. The hub resolver serves as the fallback for skill:// URIs and bare names. Includes detectScheme() which routes gh://, gcp-skill://, and GitHub full URLs to their respective resolvers, with comprehensive tests covering fallback routing, scheme dispatch, mixed batches, unsupported schemes, nil fallback safety, and error propagation. * skill-bank M5a: wire routing resolver at CLI and broker call sites Replace direct HubSkillResolver construction with RoutingSkillResolver wrapping the hub resolver at both CLI (cmd/create.go) and broker (pkg/runtimebroker/handlers.go) call sites. CachingSkillResolver wraps the routing resolver so content-hash caching applies to all source types. Add SkillURIScheme() utility to pkg/api/skill_uri.go for extracting the scheme portion of a skill URI without full parsing. * skill-bank M5c: add SkillRegistry schema, store, and models Add the SkillRegistry Ent schema with fields for name, endpoint, type (hub/gcp), trust_level (trusted/pinned), auth_token, resolve_path, pinned_hashes, and status. Define the SkillRegistryStore interface and its Ent adapter implementation with CRUD, pinned hash management, and list operations. Embed in the composite store. * skill-bank M5c: add skill registry CRUD handlers Add admin-only HTTP handlers for skill registry CRUD operations: create, list, get, update, delete, and pin hash. Register routes at /api/v1/skill-registries. Enforce HTTPS-only endpoints, validate registry names, and never expose auth tokens in API responses. * skill-bank M5c: add federation proxy and trust enforcement Add federateResolve to proxy skill resolution requests to external registries. The resolve endpoint now detects non-scion registry URIs and delegates to the configured external registry instead of local resolution. Supports trusted (pass-through) and pinned (hash verification) trust levels. * skill-bank M5c: add hub client and CLI for skill registries Add SkillRegistryService to the hub client with List, Get, Create, Update, Delete, and Pin operations. Add CLI subcommands under 'scion skills registries' for list, add, show, update, remove, and pin operations with table and JSON output support. * skill-bank M5c: add federation and registry tests Add 16 tests covering federation proxy (trusted/pinned happy paths, hash mismatch, missing pin, unknown/disabled/wrong-type registry, external registry down, auth token forwarding, custom resolve path) and registry CRUD (lifecycle, duplicate name rejection, HTTPS-only enforcement, non-admin rejection, auth token not in responses, pin). * skill-bank M5c: fix federation security issues (H1, H3, L1) H1: Add 10MB body size limit on federation success path to prevent OOM. H3: Disable redirect following on federation HTTP client to prevent credential leakage via Authorization header on cross-origin redirects. L1: Create federation HTTP client once on Server struct instead of per-call, enabling connection pooling and proper test injection. * skill-bank M5d: add gcp-skill:// URI parser and tests Add ParseGCPSkillURI which extracts alias, skill ID, and optional version from gcp-skill://alias/SKILL_ID[@Version] URIs. This is the first building block for the GCP Vertex AI skill resolver. * skill-bank M5d: add GCPSkillResolver with Vertex AI API integration Implements gcp-skill:// resolution via GCP Vertex AI Skills API. The resolver uses ADC for authentication, looks up registry aliases via an injected RegistryLookup function, fetches skill metadata and files from the GCP API, and computes content hashes for verification. * skill-bank M5d: wire GCP resolver at broker and add tests Register the GCPSkillResolver in the broker's skill resolver chain. The registry alias lookup uses the Hub API (which accepts name-based lookups). Add comprehensive tests covering happy path, error cases (unknown alias, disabled registry, wrong type, GCP 404/403, ADC failure, empty files), and alias forwarding. * skill-bank M5d: fix version validation, SSRF defense, and response size limit F1: Validate that if a version is requested via @Version in the URI, the GCP API response version must match — reject with a clear error otherwise. F2: Validate file download URLs before fetching: must use HTTPS (except localhost), must share the same host as the registry endpoint, and must not target link-local (169.254.x.x) or RFC 1918 addresses. F6: Wrap fetchSkillMetadata response body with io.LimitReader (1MB) to prevent OOM from oversized API responses. * skill-bank M5b: add gh:// URI parser and tests * skill-bank M5b: add GitHubSkillResolver with Contents API integration * skill-bank M5b: add full GitHub URL parser * skill-bank M5b: wire GitHubSkillResolver at CLI and broker * skill-bank M5b: add GitHub resolver integration tests * skill-bank M5b: fix input sanitization and response size limit * skill-bank M5: fix PR review findings (nil checks, SSRF IPv6, resolvePath, ADC caching) * skill-bank M5: fix SSRF redirect bypass, ADC context, and PinSkillHash race * skill-bank M5: fix federation URI translation, CLI GCP wiring, and path escaping * skill-bank M5: fix CI — gofmt and missing mock method --------- Co-authored-by: Scion Agent (skill-bank-m5a-dev3) <agent@scion.dev>
* fix: error contracts, integration feedback, outbound errors, and wake audit
Stream B — Non-existent agent error contract:
- Move agent lookup before message persistence in deliverToAgent() to
prevent orphan message rows for deleted agents
- Add DELIVERY_FAILED notification type dispatched to agent senders
when broker-path delivery targets a non-existent agent
- Enhance Hub API 404 responses with agent slug and project context
- Mark scheduled events targeting deleted agents with status=failed
Stream I — Outbound agent-to-user error feedback:
- Persistence failure returns 500 (was silent 200 OK)
- Missing recipient returns 400 (removed silent creator fallback)
- Broker dispatch failure returns 502 with clear message
- Successful sends return message_id, status, recipient, recipient_id
Stream K — Wake audit and test coverage:
- Add TestHandleAgentMessage_WakeSuspended (primary use case was untested)
- Add wake failure scenario tests (start fails, delivery fails)
- Add test for messaging suspended agent without --wake
- Bump wake timeout from 15s to 30s to match broker retry deadline
- Add distinct error for wake-success-delivery-failure
- Reject messages to suspended agents without --wake with clear error
Stream C — Integration error feedback:
- Add ActionAttach permission check for user: senders in handleBrokerInbound
- Validate default agents against agent cache before routing in Telegram
- Report Hub delivery errors back to originating Telegram chat
- Add error cooldown (max 1 per 5 min per chat+thread+error-type)
- Include remediation suggestions in error responses
* fix: address review findings M1, M2, L1, L2
M1: Fix misleading "Message persisted but delivery failed" error message
to "Message delivery failed" — the broker path doesn't persist before
dispatch, so the old message was incorrect.
M2: Add lazy eviction to errorCooldown map in shouldSuppressError() when
map exceeds 1000 entries, preventing unbounded growth in long-running
Telegram plugin instances.
L1: Fix gofmt alignment on ErrCodeAgentNotFound and ErrCodeDeliveryFailed
constants.
L2: Inline responseStatus and deliveryStatus variables that were never
reassigned — every error path returns early, so the scaffolding added
no value.
* feat(messaging): broadcast partial-failure reporting and CLI sender feedback
Stream H — CLI Sender Feedback Improvements:
- Add agent phase pre-check in handleAgentMessage: non-running agents
return 409 Conflict with guidance (suspended: use --wake, stopped/error:
use scion start, other: wait for running state).
- Extend 200 OK response with message_id, status, agent, agent_phase.
- Update hubclient SendStructuredMessage to return *MessageResponse.
- CLI differentiates "delivered" (200) from "deferred" (202) output.
Stream G — Broadcast Partial-Failure Reporting:
- Broadcasts return 202 Accepted with targeting info: total agents,
targeted (running) count, skipped count with phase breakdown.
- broadcastDirect publishes DELIVERY_FAILED notifications for per-agent
delivery failures.
- Message broker fan-out publishes DELIVERY_FAILED on dispatch failures.
- CLI grove-scoped broadcast uses Hub broadcast endpoint and prints
acceptance summary with targeted/skipped breakdown.
- Update hubclient BroadcastMessage to return *BroadcastResponse.
* fix: address review findings M1, M2, M3, M4
M1: Eliminate double ListAgents TOCTOU in direct-broadcast path by
passing pre-classified running agents from the handler's single
query to broadcastDirect.
M2: Add TODO noting --all path needs P3 upgrade when a global
broadcast endpoint is added.
M3: Restore zero-targeted guard — print "No running agents" when
targeted count is 0 instead of misleading acceptance message.
M4: Sort skipped breakdown phases alphabetically for deterministic
CLI output.
* style: fix gofmt formatting in broker_v2.go and agents.go
* feat: channel validation, group[] rename, and scheduled event cleanup (#213)
Stream A — Channel/flag validation:
- Validate --channel names against registered channels at send time
in CLI (sendMessageViaHub, sendOutboundMessageViaHub) and Hub
(handleAgentOutboundMessage).
- Return actionable error naming available channels.
Stream F — set[] to group[] rename:
- Accept both group[ and set[ prefixes in IsGroupRecipient/ParseGroupRecipient
for backward compatibility.
- FormatGroupRecipients now emits group[...] as the canonical syntax.
- CLI help text updated to show group[...] as primary syntax.
- Deprecation warning logged when set[...] is used.
Stream J — Scheduled event cleanup on agent deletion:
- Cancel all pending scheduled events targeting a deleted agent in
performAgentDelete, before the agent record is removed.
- Match events by parsing payload for agent ID/name/slug.
- Mark cancelled events with status "cancelled" and reason
"target agent deleted".
- Cancel corresponding in-memory scheduler timers.
* feat(messaging): no-queuing delivery policy with synchronous broker retry
Replace implicit fire-and-forget queuing with synchronous-or-reject
semantics. Messages are now retried against the broker for up to 30s
with exponential backoff before failing with 502 (non-transient error)
or 504 (timeout). Messages are persisted with dispatch_state=dispatched
optimistically and marked as failed on delivery failure.
- Add dispatchWithBrokerRetry() helper with exponential backoff
- Add ErrBrokerTimeout sentinel and broker_timeout error code
- Add MarkMessageFailed() to store interface
- Update all 7 dispatch call sites to use sync retry
- Remove signalDeferredMessage, pending message scan in reconcileBroker
- Remove signalDeferred wiring from MessageBrokerProxy, NotificationDispatcher
- Remove dead "deferred" branch from CLI message output
* fix(messaging): address Phase 4 review findings
F1: MarkMessageFailed now persists the failure reason via a new
dispatch_failure_reason column, and removes redundant control flow.
F2: Update stale ErrMessageDeferred comment to reflect retry semantics.
F3: broadcastDirect persists before dispatch, matching other handlers.
F4: Document sequential retry O(N×30s) risk in handleGroupMessage.
F5: Note shared 30s context in deliverToAgent.
F6: Document that post-Phase-4 pending rows indicate a bug.
* fix(messaging): address Phase 1 review findings
- M1: Use FormatGroupRecipients (not deprecated FormatSetRecipients) in handleGroupMessage
- M2: Fail closed when broker proxy is nil during channel validation in outbound handler
- M4: Add unit tests for eventTargetsAgent (6 tests) and validateChannel (3 tests)
- L1: Fix FormatGroupRecipients docstring (set[...] -> group[...])
- Fix ListChannels using CheckResponse which closes body before decode; use DecodeResponse instead
* fix: resolve CI lint and gofmt failures
- Fix gofmt trailing newline in reconcile.go
- Fix errcheck: check CancelEvent return value in handlers.go
- Fix errcheck: discard json.Encode return in handlers.go and test files
- Fix staticcheck: use tagged switch in message_channel_test.go
* fix(messaging): address PR GoogleCloudPlatform#409 review comments
- Add deliveryErr parameter to publishDeliveryFailed for accurate error messages
- Distinguish ErrNotFound from transient errors in agent lookup (messagebroker.go)
- Distinguish ErrNotFound from transient errors in broker inbound handler (403 vs 500)
- Throttle errorCooldown map cleanup to every 100 calls instead of every call
---------
Co-authored-by: Scion Agent (message-improvements-p2) <agent@scion.dev>
gofmt struct alignment in broker_v2.go, errcheck fixes for unchecked json.Encode/Decode calls in handlers.go and handlers_message_delivery_test.go.
There was a problem hiding this comment.
Code Review
This pull request introduces external skill registries with new GCP and GitHub skill resolvers, adds a container image build command, transitions the legacy 'set[]' message syntax to 'group[]', and implements synchronous broker retries for message delivery. It also adds support for '--no-auth' agent startup with capture-auth staging, automatically cancels pending scheduled events upon agent deletion, and integrates OTel metrics for GCP tokens and broker authentication. The review feedback suggests parallelizing group message delivery to prevent timeouts, using reflection to dynamically set auth config fields, performing database updates asynchronously during agent deletion to reduce latency, and properly handling ignored errors when updating scheduled event statuses.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| for i, recip := range recipients { | ||
| recipStr := recip.String() |
There was a problem hiding this comment.
Delivering group messages sequentially with a 30-second timeout per recipient can easily exceed the HTTP request timeout of the client or intermediate proxies (typically 60 seconds) if multiple recipients are offline or unreachable. This will cause a 504 Gateway Timeout for the entire request, failing delivery for subsequent recipients.
Since each recipient writes to a unique index in the results slice, there are no data races on the slice itself. Consider performing the dispatch in parallel using goroutines or errgroup to ensure that one offline agent does not block or timeout the entire group message request.
| func setAuthConfigField(auth *api.AuthConfig, field, value string) { | ||
| switch field { | ||
| case "GoogleAppCredentials": | ||
| auth.GoogleAppCredentials = value | ||
| case "OAuthCreds": | ||
| auth.OAuthCreds = value | ||
| case "CodexAuthFile": | ||
| auth.CodexAuthFile = value | ||
| case "OpenCodeAuthFile": | ||
| auth.OpenCodeAuthFile = value | ||
| case "ClaudeAuthFile": | ||
| auth.ClaudeAuthFile = value | ||
| } | ||
| } |
There was a problem hiding this comment.
The setAuthConfigField function uses a hardcoded switch statement to map string field names to api.AuthConfig struct fields. This introduces a maintenance burden whenever new auth fields are added. We can elegantly refactor this using Go's reflect package to dynamically set the fields by name, making the implementation completely generic and future-proof.
Note: You will need to import the "reflect" package in this file.
func setAuthConfigField(auth *api.AuthConfig, fieldName, value string) {
v := reflect.ValueOf(auth).Elem()
f := v.FieldByName(fieldName)
if f.IsValid() && f.CanSet() && f.Kind() == reflect.String {
f.SetString(value)
}
}| var cancelled int | ||
| for _, evt := range result.Items { | ||
| if !eventTargetsAgent(evt, agent) { | ||
| continue | ||
| } | ||
| if err := s.store.UpdateScheduledEventStatus(ctx, evt.ID, | ||
| store.ScheduledEventCancelled, nil, "target agent deleted"); err != nil { | ||
| s.agentLifecycleLog.Warn("Failed to cancel scheduled event", | ||
| "event_id", evt.ID, "agent_id", agent.ID, "error", err) | ||
| continue |
There was a problem hiding this comment.
Running up to 1000 synchronous database updates in a loop inside the HTTP request path (performAgentDelete) can significantly increase latency and potentially cause gateway timeouts (e.g., 504 Gateway Timeout on Cloud Run/GCP). Consider performing these updates asynchronously in a background goroutine using a detached context (like context.WithoutCancel(ctx)), or utilizing a bulk update query if supported by the store.
| "message", payload.Message) | ||
| return fmt.Errorf("target agent %q no longer exists", targetName) | ||
| now := time.Now() | ||
| _ = s.store.UpdateScheduledEventStatus(ctx, evt.ID, store.ScheduledEventFailed, &now, "target agent deleted") |
There was a problem hiding this comment.
The error returned by UpdateScheduledEventStatus is silently ignored using _ = .... If the database update fails, the scheduled event might remain in a pending state or fail silently, making debugging extremely difficult. It is highly recommended to check the error and log it if it fails.
if err := s.store.UpdateScheduledEventStatus(ctx, evt.ID, store.ScheduledEventFailed, &now, "target agent deleted"); err != nil {
slog.Error("Scheduler: failed to update scheduled event status", "eventID", evt.ID, "error", err)
}
Summary
broker_v2.goafter new field additionjson.NewEncoder(w).Encode()return inhandlers.gojson.NewDecoder(rec.Body).Decode()returns inhandlers_message_delivery_test.go(3 instances)These lint fixes were in commit
0962ca8on the message-improvements branch but were missed by the squash merge of PR GoogleCloudPlatform#409 (which used526cdf4as head).Test plan