feat(go): add persistent_memory and user_id support

[sawradip]

Summary

• Add UserId and PersistentMemory fields to Config struct
• Add RUNAGENT_USER_ID and RUNAGENT_PERSISTENT_MEMORY environment variable support with config precedence (explicit > env > default)
• Conditionally include user_id and persistent_memory in both REST and WebSocket request payloads
• Add UserId() and PersistentMemory() getter methods on RunAgentClient

Test plan

• Verify UserId() returns configured user ID (explicit config or env var)
• Verify PersistentMemory() returns configured flag
• Verify user_id and persistent_memory appear in REST POST body when set
• Verify user_id and persistent_memory appear in WebSocket payload when set
• Verify fields are omitted when not set (no user_id: "" in payload)
• Run go build ./... and go vet ./... to confirm no regressions

🤖 Generated with Claude Code

[coderabbitai]

Warning

Rate limit exceeded

@sawradip has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 52 minutes and 50 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 81adc5c7-dad9-4a61-8632-972f4554c16d

📥 Commits

Reviewing files that changed from the base of the PR and between 2c0de38 and f8f8019.

📒 Files selected for processing (3)

• runagent-go/client.go
• runagent-go/internal/constants/constants.go
• runagent-go/types.go

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/go-persistent-memory

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Review rate limit: 0/1 reviews remaining, refill in 52 minutes and 50 seconds.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[sawradip]

Thanks for a well-structured PR. The overall approach — separate env fields, *bool for tri-state, firstNonEmpty for precedence — is solid. A few real issues need addressing before merge.

---

Issues requiring changes

1. persistent_memory=false is silently dropped (logic bug)

In both Run and RunStream:

if c.persistentMemory {
    bodyMap["persistent_memory"] = c.persistentMemory
}

This only injects the field when it is true. If a caller sets Config{PersistentMemory: Bool(false)} to explicitly disable persistent memory, or sets RUNAGENT_PERSISTENT_MEMORY=false in the environment, the field is never sent and the server sees no signal at all. Depending on the server's default, this can mean the feature stays on when the user wanted it off.

The construction path correctly resolves the tri-state (nil → env → default), so c.persistentMemory is already the definitive value. The injection logic must distinguish between "the caller never set anything" (nil in the original config/env) and "the caller explicitly set false".

Fix: Store persistentMemory as a *bool on RunAgentClient as well, and only inject when non-nil:

if c.persistentMemory != nil {
    bodyMap["persistent_memory"] = *c.persistentMemory
}

This requires the client field to change from bool to *bool, which is a small but necessary change for correctness.

---

2. Non-idiomatic Go naming (UserId → UserID)

The Go spec and golint/staticcheck require acronyms to be all-caps: ID, not Id. The existing codebase is clean on this. The PR introduces:

• Config.UserId → should be Config.UserID
• RunAgentClient.userId → should be userID
• envConfig.userId → should be userID
• (c *RunAgentClient) UserId() → should be UserID()
• constants.EnvUserId → should be EnvUserID

golint and staticcheck will flag every one of these.

---

3. JSON struct tags on Config are dead code and inconsistent

UserId           string `json:"user_id,omitempty"`
PersistentMemory *bool  `json:"persistent_memory,omitempty"`

Config is a pure constructor-argument struct — it is never marshalled or unmarshalled anywhere in the codebase. All other Config fields (AgentID, APIKey, HTTPClient, etc.) have no json tags. These tags serve no purpose, add confusion (callers might think they can json.Unmarshal into Config), and are inconsistent. Please remove them.

---

4. Double marshal/unmarshal is fragile and unnecessary

Both Run and RunStream use:

raw, _ := json.Marshal(payload)          // struct → JSON bytes
json.Unmarshal(raw, &bodyMap)            // JSON bytes → map
bodyMap["user_id"] = ...                 // inject
body, _ = json.Marshal(bodyMap)          // map → JSON bytes again

This round-trip through map[string]interface{} can silently corrupt numeric types (JSON numbers become float64, losing integer precision) and is an unnecessary allocation. The correct approach is to add the optional fields directly to apiRunRequest in types.go:

type apiRunRequest struct {
    EntrypointTag    string                 `json:"entrypoint_tag"`
    InputArgs        []interface{}          `json:"input_args"`
    InputKwargs      map[string]interface{} `json:"input_kwargs"`
    TimeoutSeconds   int                    `json:"timeout_seconds"`
    AsyncExecution   bool                   `json:"async_execution,omitempty"`
    UserID           string                 `json:"user_id,omitempty"`
    PersistentMemory *bool                  `json:"persistent_memory,omitempty"`
}

Then populate them before json.Marshal. This is a single allocation, type-safe, and uses the existing omitempty mechanism correctly — which also fixes issue #1 naturally (nil *bool is omitted; non-nil is included regardless of value).

---

5. Inconsistent alignment in constants.go (gofmt violation)

EnvTimeout          = "RUNAGENT_TIMEOUT"
EnvUserId           = "RUNAGENT_USER_ID"
EnvPersistentMemory = "RUNAGENT_PERSISTENT_MEMORY"

The new constants break the alignment of the existing block. Please run gofmt -w over the file to fix this before merging.

---

Minor / non-blocking observations

• The comment on UserId() says "user ID for persistent memory" — it might just be "the configured user ID" since user IDs have uses beyond persistent memory.
• No tests were added. Even a table-driven unit test covering env resolution precedence (explicit > env > default) and the payload injection conditions would significantly reduce regression risk for issue #1.

---

Summary

The two correctness issues (#1 and #4) need fixes before this merges — specifically that persistent_memory=false is never sent to the server, and the double-marshal approach is fragile. Issues #2–#3 are naming/idiom fixes that should also be addressed given the rest of the codebase is clean. Issue #5 is a gofmt fix.