Fix/qa notify wait verify - #36
Conversation
After POST, poll /v2/runs until post_test_verify and fail the notify workflow when releasable is false. Also fail fast when k6 is missing on the agent. Co-authored-by: Cursor <cursoragent@cursor.com>
- Added a timeout for Kafka message publishing to prevent blocking the HTTP read path. - Enhanced error handling for Kafka publishing failures, including logging warnings for timeouts and general exceptions. - Updated application configuration to set a maximum block time for Kafka producers, ensuring responsiveness during Kafka downtime.
- Introduced a new method `withAuth` to centralize authentication logic for trade-related API calls. - Replaced direct API call error handling with the new `withAuth` method to improve code readability and maintainability. - Updated constructor to include `TradeAuthTokenSupplier` for dynamic token management. - Adjusted `TradeSdkConfig` to initialize `AmTradeSdk` with a blank API key, binding the live token during each call. - Improved error handling and logging consistency across trade methods.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe PR adds QA notification tracking and verification polling, runtime trade authentication, centralized MCP identity resolution, Keycloak launcher authentication, bounded Kafka publication, and Maven publishing updates. ChangesQA notification verification
Runtime trade authentication
MCP user identity resolution
Bounded Kafka trigger publication
Build and package publishing configuration
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
.github/workflows/qa-agent-notify.yml (1)
133-133: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd
persist-credentials: falseto both new checkout steps.Both new jobs (
notifyandwait-verify) check out the repository withactions/checkout@v4but do not disable credential persistence. Neither job needs git write access after checkout; both only run Python scripts. Persisting credentials unnecessarily increases exposure if the runner or a downstream step is compromised.
.github/workflows/qa-agent-notify.yml#L133-L133: addwith: persist-credentials: falseto thenotifyjob's checkout step..github/workflows/qa-agent-notify.yml#L158-L158: addwith: persist-credentials: falseto thewait-verifyjob's checkout step.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/qa-agent-notify.yml at line 133, Disable persisted credentials on both checkout steps in .github/workflows/qa-agent-notify.yml at lines 133-133 and 158-158 by adding the checkout action’s with configuration with persist-credentials set to false for the notify and wait-verify jobs.Source: Linters/SAST tools
scripts/qa_agent_notify_ci.py (1)
224-227: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winFail fast on authorization errors instead of retrying for the full timeout.
Any HTTP status of 400 or more, including 401 and 403, is retried until
timeout_secelapses (up to 1200 seconds by default). Authorization failures do not self-resolve. Retrying them wastes CI time and delays the failure signal.Return immediately on 401 or 403 and keep the retry loop for other 4xx/5xx statuses that can be transient.
♻️ Proposed fix
if code >= 400: + if code in (401, 403): + print(f"::error::authorization failed (HTTP {code}); check QA_AGENT_GATEWAY_TOKEN", file=sys.stderr) + return 1 print(f"attempt {attempt}: http={code} body={body[:500]}") time.sleep(poll) continue🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/qa_agent_notify_ci.py` around lines 224 - 227, Update the HTTP status handling in the retry loop to return immediately for authorization failures with status 401 or 403, while preserving the existing logging and retry behavior for other statuses at or above 400.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@libraries/am-trade-client-lib/src/main/java/com/am/trade/client/service/TradeClientService.java`:
- Around line 106-120: Refactor withAuth so outbound action.get() calls do not
synchronize on or mutate the singleton tradeSdk configuration. Use
request-scoped or SDK-client-scoped Authorization if supported by
am-trade-sdk-core; otherwise create/pass a per-call SDK/client instance
configured with the token, preserving unauthenticated behavior when no
TradeAuthTokenSupplier is available.
In `@scripts/qa_agent_notify_ci.py`:
- Around line 185-192: Validate the tracking_id obtained in cmd_notify before
writing it to GITHUB_OUTPUT, rejecting or safely handling embedded newlines and
unexpected characters so it cannot inject additional outputs. Apply the same
validation to the tracking_id used to construct the polling URL in
cmd_wait_verify, while preserving valid IDs and the existing behavior for
missing values.
In
`@services/am-analysis/src/main/java/com/am/analysis/service/bootstrap/TriggerCalculationPublisher.java`:
- Around line 69-72: Update the exception handling around the publishing
operation in TriggerCalculationPublisher to catch InterruptedException before
the generic Exception catch, restore the thread’s interrupted status in that
handler, and retain the existing logging and flowLogger.fail behavior. Leave
non-interruption failures handled by the generic catch.
- Around line 56-58: Enforce one end-to-end publication deadline across
TriggerCalculationPublisher’s KafkaTemplate.send phase and Future.get wait, so
the bootstrap HTTP read path cannot exceed the configured budget; alternatively
dispatch publication off the request path. In application.yml, cap max.block.ms
from the same timeout budget and prevent KAFKA_PRODUCER_MAX_BLOCK_MS from
overriding that cap. Apply the changes at
services/am-analysis/src/main/java/com/am/analysis/service/bootstrap/TriggerCalculationPublisher.java:56-58
and services/am-analysis/src/main/resources/application.yml:20-23.
- Around line 63-72: Update TriggerCalculationPublisher.publish to return an
explicit publication result instead of void, distinguishing successful sends,
confirmed failures, and timeouts with unknown outcomes unless the pending
CompletableFuture is cancelled. Update
PortfolioBootstrapTrigger.requestBootstrap to record the request, advance
debounce state, and return true only for successful publication; propagate
non-success outcomes so failed or unknown sends do not suppress retries.
In `@services/am-mcp-server/scripts/mcp_remote_launch.py`:
- Around line 135-176: Update the KEYCLOAK_TOKEN_URL default in main to use the
HTTPS scheme for the existing Keycloak token endpoint, while preserving the
environment-variable override and URL path unchanged.
In `@services/am-mcp-server/src/main/java/com/am/mcp/util/UserIdResolver.java`:
- Around line 14-22: Update UserIdResolver.resolve to prioritize and return the
authenticated ID from UserContext.getUserId() before considering the
caller-supplied userId; only permit caller overrides through an explicitly
authorized trusted path, while retaining the configured default when no
authenticated ID is available.
---
Nitpick comments:
In @.github/workflows/qa-agent-notify.yml:
- Line 133: Disable persisted credentials on both checkout steps in
.github/workflows/qa-agent-notify.yml at lines 133-133 and 158-158 by adding the
checkout action’s with configuration with persist-credentials set to false for
the notify and wait-verify jobs.
In `@scripts/qa_agent_notify_ci.py`:
- Around line 224-227: Update the HTTP status handling in the retry loop to
return immediately for authorization failures with status 401 or 403, while
preserving the existing logging and retry behavior for other statuses at or
above 400.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b04557c4-0d84-44e7-a1bc-ff17c6a85fc5
📒 Files selected for processing (17)
.github/workflows/qa-agent-notify.ymllibraries/am-trade-client-lib/src/main/java/com/am/trade/client/auth/TradeAuthTokenSupplier.javalibraries/am-trade-client-lib/src/main/java/com/am/trade/client/service/TradeClientService.javascripts/qa_agent_notify_ci.pyservices/am-analysis/src/main/java/com/am/analysis/service/bootstrap/TriggerCalculationPublisher.javaservices/am-analysis/src/main/resources/application.ymlservices/am-mcp-server/scripts/mcp_remote_launch.pyservices/am-mcp-server/src/main/java/com/am/mcp/auth/AuthTokenProvider.javaservices/am-mcp-server/src/main/java/com/am/mcp/config/TradeSdkConfig.javaservices/am-mcp-server/src/main/java/com/am/mcp/tools/AiAgentTools.javaservices/am-mcp-server/src/main/java/com/am/mcp/tools/AnalysisTools.javaservices/am-mcp-server/src/main/java/com/am/mcp/tools/BasketTools.javaservices/am-mcp-server/src/main/java/com/am/mcp/tools/PortfolioTools.javaservices/am-mcp-server/src/main/java/com/am/mcp/tools/TradeTools.javaservices/am-mcp-server/src/main/java/com/am/mcp/util/UserIdResolver.javaservices/am-mcp-server/src/test/java/com/am/mcp/auth/AuthTokenProviderTest.javaservices/am-mcp-server/src/test/java/com/am/mcp/util/UserIdResolverTest.java
| tid = data.get("tracking_id") or "" | ||
| print(f"tracking_id={tid}") | ||
| gh_out = os.environ.get("GITHUB_OUTPUT") | ||
| if gh_out and tid: | ||
| with open(gh_out, "a", encoding="utf-8") as f: | ||
| f.write(f"tracking_id={tid}\n") | ||
| print(f"qa-agent activated for {os.environ['SERVICE']} on {os.environ['BRANCH']}") | ||
| return 0 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
GitHub Actions GITHUB_OUTPUT multiline value injection risk
💡 Result:
Injecting untrusted, multiline data into the $GITHUB_OUTPUT file in GitHub Actions creates a significant security risk known as output injection or output clobbering [1][2]. Because the output format relies on key=value pairs, an attacker who can influence the value can inject newlines to prematurely terminate the intended output and introduce new, malicious output variables that downstream steps may consume as trusted data [1][2]. When writing multiline values to $GITHUB_OUTPUT, the primary defense is to use the documented heredoc syntax with a random, unpredictable delimiter [1][3]. Core Risks: 1. Output Clobbering: An attacker can inject newlines and new keys to overwrite previous output values, potentially bypassing security checks or modifying workflow logic (e.g., changing an 'approved=false' status to 'true') [1][2]. 2. Downstream Injection: If an attacker injects a new key-value pair that a subsequent, privileged step uses—for example, by passing an output directly into a shell command—it can facilitate command injection, allowing the attacker to execute arbitrary code on the runner [1][3][4]. Mitigation Strategies: - Use Random Heredoc Delimiters: Never use static delimiters like EOF for multiline strings [3][5]. Generate a random, unique delimiter for every write operation [3][6]. This ensures that even if user-controlled input contains strings that look like your delimiter, they will not be interpreted as the end of the data block [1][6]. - Avoid Raw Interpolation: Never directly echo untrusted context (e.g., ${{ github.event.issue.title }}) into $GITHUB_OUTPUT [1][7]. - Move to Environment Variables: Before processing untrusted input, assign it to a step-level environment variable (env:) [1][2][8]. This separates the untrusted data from the workflow's command-line context during expression expansion [8][7]. - Sanitize Inputs: If possible, strip newlines and carriage returns from user-provided data before writing it to an output file [2]. - Use Defense-in-Depth: Treat all workflow outputs as potentially tainted, especially if they originated from external events like issue comments or pull request metadata [1][8][7]. Validate downstream usage of step outputs by properly quoting variables and checking against expected formats [1].
Citations:
- 1: https://actsense.dev/vulnerabilities/github_output_injection/
- 2: https://sisaku-security.github.io/lint/docs/rules/outputclobbering/
- 3: Security: fixed EOF heredoc delimiter in action.yml enables step output injection google-github-actions/run-gemini-cli#526
- 4: GHSA-r4fj-r33x-8v88
- 5: fix(workflows): randomize $GITHUB_OUTPUT heredoc delimiter (defense in depth) Uniswap/hooklist#545
- 6: fix: randomize heredoc delimiter in GITHUB_OUTPUT writes google-github-actions/run-gemini-cli#527
- 7: https://docs.semgrep.dev/learn/vulnerabilities/command-injection/github-actions-injection
- 8: https://github.blog/security/supply-chain-security/four-tips-to-keep-your-github-actions-workflows-secure/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the target script around the relevant symbols/functions.
fd -a 'qa_agent_notify_ci.py' . | sed 's#^\./##'
TARGET="$(fd 'qa_agent_notify_ci.py' . | head -n 1)"
if [ -n "$TARGET" ]; then
echo "--- outline ---"
ast-grep outline "$TARGET" --view compact || true
echo "--- target lines 150-225 ---"
sed -n '150,225p' "$TARGET" | nl -ba -v150
echo "--- usages of tracking_id/TRACKING_ID/cmd_wait_verify/GITHUB_OUTPUT ---"
rg -n 'tracking_id|TRACKING_ID|cmd_wait_verify|GITHUB_OUTPUT' "$TARGET"
fi
echo "--- workflow references ---"
rg -n 'qa_agent_notify_ci|tracking_id|TRACKING_ID|qa-agent|notify|api/.*qa' .github scripts -S || trueRepository: AM-Portfolio/am-core-services
Length of output: 453
🏁 Script executed:
#!/bin/bash
set -euo pipefail
TARGET="scripts/qa_agent_notify_ci.py"
echo "--- outline ---"
ast-grep outline "$TARGET" --view names || true
echo "--- target lines 150-225 ---"
python3 - <<'PY' "$TARGET" "$TARGET"
from pathlib import Path
import sys
path = Path(sys.argv[1])
lines = path.read_text(encoding="utf-8").splitlines()
for i in range(149, min(226, len(lines))):
print(f"{i+1:5d} {lines[i]}")
PY
echo "--- usages of tracking_id/TRACKING_ID/cmd_wait_verify/GITHUB_OUTPUT ---"
python3 - <<'PY' "$TARGET" "$TARGET"
from pathlib import Path
import re, sys
path = Path(sys.argv[1])
text = path.read_text(encoding="utf-8")
for pat in ['tracking_id', 'TRACKING_ID', 'cmd_wait_verify', 'GITHUB_OUTPUT']:
for m in re.finditer(re.escape(pat), text):
line = text.count("\n", 0, m.start()) + 1
start = text.rfind("\n", 0, m.start()-100) + 1
end = text.find("\n", m.start()+600)
print(f"--- {pat} line {line} ---")
print(text[start:end] if end != -1 else text[start:])
PY
echo "--- workflow references ---"
python3 - <<'PY'
from pathlib import Path
import re
for path in Path(".").rglob("*"):
if path.is_file() and (path.name.startswith(".") or path.parts[0] in {"git", "node_modules", ".git"}) : continue
try:
text = path.read_text(encoding="utf-8", errors="ignore")
except Exception:
continue
hits = ['qa_agent_notify_ci', 'tracking_id', 'TRACKING_ID', 'qa-agent', 'notify', 'api']
if any(h in text for h in hits):
lines = text.splitlines()
print(f"--- {path} ---")
for n,l in enumerate(lines, 1):
if any(h in l for h in hits):
s=max(1,n-3); e=min(len(lines),n+3)
for i in range(s,e+1):
print(f"{i:5d} {lines[i-1]}")
print()
PYRepository: AM-Portfolio/am-core-services
Length of output: 50388
🏁 Script executed:
#!/bin/bash
set -euo pipefail
TARGET="scripts/qa_agent_notify_ci.py"
echo "--- target lines 1-240 ---"
python3 - <<'PY'
from pathlib import Path
lines = Path("scripts/qa_agent_notify_ci.py").read_text(encoding="utf-8").splitlines()
for i in range(min(240, len(lines))):
print(f"{i+1:5d} {lines[i]}")
PY
echo "--- workflow references: only tracked workflows/scripts ---"
python3 - <<'PY'
from pathlib import Path
terms = ['qa_agent_notify_ci', 'tracking_id', 'TRACKING_ID', 'GITHUB_OUTPUT']
for path in Path(".").rglob("*"):
if not path.is_file():
continue
if path parts[0] in {"node_modules", ".git"} or path.name.startswith("."):
continue
try:
text = path.read_text(encoding="utf-8", errors="ignore")
except Exception:
continue
if any(t in text for t in terms):
lines = text.splitlines()
print(f"--- {path} ---")
for n,l in enumerate(lines, 1):
if any(t in l for t in terms):
s=max(1,n-4); e=min(len(lines),n+4)
for i in range(s,e+1):
print(f"{i:5d} {lines[i-1]}")
print()
PY
echo "--- deterministic multiline injection probe for the exported value ---"
python3 - <<'PY'
import io
def write_tracking_id(value):
gh = io.StringIO()
value = value or ""
if gh and value:
gh.write("tracking_id=" + value + "\n")
gh.seek(0)
result = {}
for line in gh.read().splitlines():
key, _, rv = line.partition("=")
result.setdefault(key, []).append(rv)
return gh.getvalue(), result
for payload in [
"ok",
"evil\nmalicious=true",
"evil\ntracking_id=overridden\ntrusted=yes",
]:
raw, parsed = write_tracking_id(payload)
print(f"payload={payload!r}")
print("raw=" + raw.rstrip().replace("\n", "\\n\n"))
print("parsed_tracking_id=" + repr(parsed.get("tracking_id")))
print("all keys=" + repr(list(parsed.keys())))
print()
PYRepository: AM-Portfolio/am-core-services
Length of output: 11129
Validate tracking_id before exporting it to GITHUB_OUTPUT.
cmd_notify reads tracking_id from the qa-agent JSON response and writes it verbatim to $GITHUB_OUTPUT. If this value contains an embedded newline or unexpected characters, it can inject extra keys or overwrite GitHub Actions outputs. Also apply the same validation before using it in the polling URL in cmd_wait_verify.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 188-188: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(gh_out, "a", encoding="utf-8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/qa_agent_notify_ci.py` around lines 185 - 192, Validate the
tracking_id obtained in cmd_notify before writing it to GITHUB_OUTPUT, rejecting
or safely handling embedded newlines and unexpected characters so it cannot
inject additional outputs. Apply the same validation to the tracking_id used to
construct the polling URL in cmd_wait_verify, while preserving valid IDs and the
existing behavior for missing values.
| // Never block HTTP read path for ~60s when Kafka metadata is unavailable. | ||
| kafkaTemplate.send(KafkaTopics.TRIGGER_CALCULATION, key, payload) | ||
| .get(PUBLISH_TIMEOUT_SECONDS, TimeUnit.SECONDS); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 4 \
'PUBLISH_TIMEOUT_SECONDS|max\.block\.ms|KAFKA_PRODUCER_MAX_BLOCK_MS|triggerCalculationPublisher\.publish' \
--glob '*.java' --glob '*.yml' --glob '*.yaml'Repository: AM-Portfolio/am-core-services
Length of output: 167
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "Repository files relevant to target names:"
git ls-files | rg 'TriggerCalculationPublisher\.java|application\.ya?ml|$' | sort
echo
echo "Files matching trigger calculation publisher:"
fd -i 'TriggerCalculationPublisher|application\.ya?ml' . || true
echo
echo "Search all tracked files for relevant strings:"
rg -n -C 3 -i \
'TriggerCalculationPublisher|publishCalculation|trigger.*calculation|PUBLISH_TIMEOUT|max\.block\.ms|KAFKA_PRODUCER_MAX_BLOCK_MS|kafkaTemplate\.send\(|kafka\.template\.send' \
|| trueRepository: AM-Portfolio/am-core-services
Length of output: 49793
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== TriggerCalculationPublisher =="
cat -n services/am-analysis/src/main/java/com/am/analysis/service/bootstrap/TriggerCalculationPublisher.java
echo
echo "== application.yml kafka bootstrap section =="
cat -n services/am-analysis/src/main/resources/application.yml | sed -n '1,80p'
echo
echo "== publish timeout references =="
rg -n -C 4 'PUBLISH_TIMEOUT|TIMEOUT|triggerCalculationPublisher|kafkaTemplate|send\(\)|max\.block\.ms|KAFKA_PRODUCER_MAX_BLOCK_MS|Bootstrap' services/am-analysis/src/main/java services/am-analysis/src/main/resources application.yml || trueRepository: AM-Portfolio/am-core-services
Length of output: 50385
🌐 Web query:
Apache Kafka ProducerConfig max.block.ms ProducerRecord blocking documentation
💡 Result:
The max.block.ms configuration in Apache Kafka controls the maximum duration that certain KafkaProducer methods—most notably send—will block [1][2]. When calling KafkaProducer.send, the producer may block for two primary reasons [1][2]: 1. Metadata Fetching: Waiting to retrieve cluster metadata if it is unavailable (e.g., when the producer first starts or needs to discover a new topic/partition) [1][2]. 2. Buffer Allocation: Waiting for space to become available in the producer's internal buffer (controlled by the buffer.memory configuration) [1][2]. If the producer's buffer is full (i.e., records are being produced faster than they can be sent to the broker), the send method will block for up to max.block.ms [1][2]. If space does not become available within this timeframe, the producer will stop waiting and throw an exception (typically a TimeoutException or BufferExhaustedException) [1][3][4]. Key details regarding this configuration include: - Default Value: 60,000 milliseconds (1 minute) [1][3]. - Scope: It limits the time spent waiting for metadata and buffer memory [1][2]. It does not count time spent executing user-supplied serializers or partitioners [1][2]. - Other affected methods: Beyond send, this timeout also applies to partitionsFor, initTransactions, sendOffsetsToTransaction, commitTransaction, and abortTransaction [1][2]. Tuning: - Reducing max.block.ms (e.g., to 1000ms) can allow applications to fail fast and implement custom handling (like shedding load or circuit breaking) when the producer's buffer is exhausted, rather than stalling the application thread for a long period [4]. - Maintaining the default (60s) is often appropriate for durability, as it allows the producer to withstand transient spikes or broker unavailability without immediately failing the produce request [4].
Citations:
- 1: https://kafka.apache.org/43/configuration/producer-configs/
- 2: https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java
- 3: https://kafka.apache.org/38/generated/producer_config.html
- 4: https://kafka-options-explorer.conduktor.io/config/max-block-ms/
🌐 Web query:
Spring Kafka KafkaTemplate send Future get max.block.ms producer send blocks
💡 Result:
In Spring Kafka, the interaction between KafkaTemplate.send() and max.block.ms involves two distinct types of blocking behavior: the internal blocking of the underlying Kafka producer and the explicit blocking of the calling thread when using Future.get() [1][2][3]. 1. Internal Producer Blocking (max.block.ms) The max.block.ms property (default 60 seconds) is a configuration of the underlying Apache Kafka KafkaProducer [4][5]. It determines the maximum time the send() method will block the calling thread internally to wait for metadata retrieval or buffer availability [4][3]. If the producer's buffer is full (due to buffer.memory limits) or if it is waiting to fetch cluster metadata (e.g., partition information), the KafkaProducer.send() call itself may block up to this duration before even returning a Future [2][3][5]. Consequently, your application thread can stall here before it has a chance to interact with the returned CompletableFuture [2][3]. 2. Explicit Future Blocking (Future.get) When you call KafkaTemplate.send(...).get(timeout, unit), you are performing an explicit, synchronous wait for the completion of the asynchronous send operation [1][3][6]. This is separate from max.block.ms [2]. - This get() call blocks your thread waiting for the Kafka broker to acknowledge the message (as determined by the acks producer setting) [1][7]. - Using a timeout in get(timeout, unit) is highly recommended to avoid indefinite blocking [1][6]. - If you use autoFlush=true or call flush() on the template, you are further forcing a block to ensure pending batches are sent to the broker immediately, which can also influence perceived latency [1][7][8]. In summary, max.block.ms limits the time the producer spends preparing the record for transmission [4][5], while get(timeout, unit) limits how long you are willing to wait for the final acknowledgement from the Kafka broker [1][6]. It is possible to experience blocking from either source depending on the state of your buffer, network, and broker [3]. To tune this, adjust max.block.ms to control internal producer stalls [3][5] and use explicit timeouts in your get() calls to protect your application threads from downstream latency [1][6].
Citations:
- 1: https://docs.spring.io/spring-kafka/reference/kafka/sending-messages.html
- 2: future's get timeout is not working on KafkaTemplate spring-projects/spring-kafka#2366
- 3: https://codemia.io/knowledge-hub/path/spring_kafka_asynchronous_send_calls_block
- 4: https://kafka.apache.org/41/configuration/producer-configs/
- 5: https://kafka-options-explorer.conduktor.io/config/max-block-ms/
- 6: https://docs.spring.io/spring-kafka/reference/4.1/kafka/sending-messages.html
- 7: https://docs.spring.io/spring-kafka/docs/4.0.4/api/org/springframework/kafka/core/KafkaTemplate.html
- 8: https://github.com/spring-projects/spring-kafka/blob/main/spring-kafka/src/main/java/org/springframework/kafka/core/KafkaTemplate.java
Enforce one end-to-end Kafka publication budget.
KafkaTemplate.send() can block for metadata/buffer allocation before Future.get() starts, and max.block.ms is still freely overridable via KAFKA_PRODUCER_MAX_BLOCK_MS. The bootstrap HTTP read path can still exceed the intended timeout during Kafka outages. Move the publish off the request path, or enforce a single deadline that includes the producer send() phase and cap this property from the same timeout budget.
📍 Affects 2 files
services/am-analysis/src/main/java/com/am/analysis/service/bootstrap/TriggerCalculationPublisher.java#L56-L58(this comment)services/am-analysis/src/main/resources/application.yml#L20-L23
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@services/am-analysis/src/main/java/com/am/analysis/service/bootstrap/TriggerCalculationPublisher.java`
around lines 56 - 58, Enforce one end-to-end publication deadline across
TriggerCalculationPublisher’s KafkaTemplate.send phase and Future.get wait, so
the bootstrap HTTP read path cannot exceed the configured budget; alternatively
dispatch publication off the request path. In application.yml, cap max.block.ms
from the same timeout budget and prevent KAFKA_PRODUCER_MAX_BLOCK_MS from
overriding that cap. Apply the changes at
services/am-analysis/src/main/java/com/am/analysis/service/bootstrap/TriggerCalculationPublisher.java:56-58
and services/am-analysis/src/main/resources/application.yml:20-23.
| } catch (TimeoutException e) { | ||
| log.warn("[Bootstrap] Kafka trigger publish timed out ({}s) for portfolioId={} — returning without blocking HTTP", | ||
| PUBLISH_TIMEOUT_SECONDS, debounceKey); | ||
| flowLogger.fail(span, e); | ||
| } catch (JsonProcessingException e) { | ||
| flowLogger.fail(span, e); | ||
| } catch (Exception e) { | ||
| log.warn("[Bootstrap] Kafka trigger publish failed for portfolioId={}: {}", | ||
| debounceKey, e.getMessage()); | ||
| flowLogger.fail(span, e); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 6 \
'requestBootstrap|triggerCalculationPublisher\.publish|lastBootstrapTrigger|analysis\.bootstrap\.requested' \
--glob '*.java'Repository: AM-Portfolio/am-core-services
Length of output: 167
🏁 Script executed:
#!/bin/sh
set -eu
printf 'Tracked Java files around Target filenames/classes:\n'
git ls-files '*.java' | rg '(TriggerCalculationPublisher|PortfolioBootstrapTrigger)|bootstrap' || true
printf '\nSearch tracked Java text:\n'
rg -n -C 5 'class TriggerCalculationPublisher|class PortfolioBootstrapTrigger|requestBootstrap|triggerCalculationPublisher\.publish|lastBootstrapTrigger|analysis\.bootstrap\.requested|PUBLISH_TIMEOUT_SECONDS|KafkaTemplate|get\(' --glob '*.java' || true
printf '\nGit status/stat:\n'
git status --short
git diff --stat || trueRepository: AM-Portfolio/am-core-services
Length of output: 448
🏁 Script executed:
#!/bin/sh
set -eu
for f in \
services/am-analysis/src/main/java/com/am/analysis/service/bootstrap/TriggerCalculationPublisher.java \
services/am-analysis/src/main/java/com/am/analysis/service/bootstrap/PortfolioBootstrapTrigger.java
do
echo "===== $f ====="
wc -l "$f"
ast-grep outline "$f" || true
echo
cat -n "$f"
echo
done
echo "===== all bootstrap files ====="
git ls-files 'services/am-analysis/src/main/java/com/am/analysis/service/bootstrap/*' | sort
echo "===== related text ====="
rg -n -C 4 'requestBootstrap|publish\(|record\(\s*"analysis\.bootstrap\.requested"|lastBootstrapTrigger|PUBLISH_TIMEOUT_SECONDS|KafkaTemplate|void ' --glob '*.java' . || trueRepository: AM-Portfolio/am-core-services
Length of output: 50386
🏁 Script executed:
#!/bin/sh
set -eu
for f in \
services/am-analysis/src/main/java/com/am/analysis/service/bootstrap/TriggerCalculationPublisher.java \
services/am-analysis/src/main/java/com/am/analysis/service/bootstrap/PortfolioBootstrapTrigger.java
do
echo "===== $f ====="
wc -l "$f" 2>/dev/null || true
ast-grep outline "$f" 2>/dev/null || true
sed -n '1,140p' "$f"
echo
done
echo "===== focused symbols ====="
rg -n -C 4 'class TriggerCalculationPublisher|public .*publish|class PortfolioBootstrapTrigger|requestBootstrap|triggerCalculationPublisher\.publish|lastBootstrapTrigger|analysis\.bootstrap\.requested|KafkaTemplate<P>|KafkaTemplate<|get\(|PUBLISH_TIMEOUT_SECONDS|void ' \
services/am-analysis/src/main/java/com/am/analysis/service/bootstrap \
services/am-analysis/src/main/java 2>/dev/null || trueRepository: AM-Portfolio/am-core-services
Length of output: 50386
🌐 Web query:
Spring Kafka KafkaTemplate send get future TimeoutException send returns ListenableFuture can future be cancelled SpringDoc send future
💡 Result:
In Spring for Apache Kafka, KafkaTemplate send methods have returned CompletableFuture<SendResult<K, V>> since version 3.0 [1][2]. They previously returned ListenableFuture, but this was replaced in the 3.0 release to align with modern Java concurrency standards [1][3]. Regarding the ability to cancel the future: Because the return type is now a standard Java CompletableFuture, you can technically invoke the cancel(boolean mayInterruptIfRunning) method [4][5]. However, in the context of a Kafka producer send operation, calling cancel on the returned future does not abort the actual network request or the background producer operation [4][5]. Once the message has been handed off to the Kafka producer's internal buffer (RecordAccumulator), the underlying Kafka client manages the sending process [6]. The future will simply reflect that it was cancelled, but the message may still be sent to the broker [5]. Regarding TimeoutException: A TimeoutException (typically wrapped in an ExecutionException when using.get or handled within the CompletableFuture's exception chain) is often encountered when the producer cannot complete the send operation within the configured time limits (e.g., delivery.timeout.ms, request.timeout.ms, or metadata fetching timeouts) [7][8][6]. If you are blocking the sending thread to wait for the result, it is best practice to use the.get(timeout, unit) method [8][9]. If you encounter a TimeoutException, it indicates that the producer was unable to receive an acknowledgment from the Kafka broker within the allocated time [6]. Troubleshooting this often involves adjusting producer configuration properties such as linger.ms, request.timeout.ms, or delivery.timeout.ms to better match your infrastructure's latency [6].
Citations:
- 1: https://docs.spring.io/spring-kafka/reference/3.2/kafka/sending-messages.html
- 2: https://docs.spring.io/spring-kafka/docs/3.0.15/reference/html/
- 3: https://docs.spring.io/spring-kafka/docs/3.1.x/reference/kafka/sending-messages.html
- 4: https://docs.spring.io/spring-kafka/reference/kafka/sending-messages.html
- 5: https://kafka.apache.org/38/javadoc/org/apache/kafka/common/KafkaFuture.html
- 6: https://stackoverflow.com/questions/58448747/getting-timeoutexception-for-some-messages-while-sending-to-kafka-topic
- 7: Metadata timeout on KafkaTemplate::send when using .completable().await() the second time spring-projects/spring-kafka#2199
- 8: https://docs.spring.io/spring-kafka/reference/4.1/kafka/sending-messages.html
- 9: https://docs.spring.io/spring-kafka/reference/3.3/kafka/sending-messages.html
Propagate publication failures from publish.
TriggerCalculationPublisher.publish hides Kafka timeouts and send failures by returning void, so callers can treat failed Kafka publication as success. PortfolioBootstrapTrigger.requestBootstrap will still record analysis.bootstrap.requested, update the debounce timestamp, and return true, which violates its documented contract and suppresses retries. Return an explicit result from publish, update debounce state based only on successful sends, and treat TimeoutException as an unknown outcome unless the pending CompletableFuture can be cancelled.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@services/am-analysis/src/main/java/com/am/analysis/service/bootstrap/TriggerCalculationPublisher.java`
around lines 63 - 72, Update TriggerCalculationPublisher.publish to return an
explicit publication result instead of void, distinguishing successful sends,
confirmed failures, and timeouts with unknown outcomes unless the pending
CompletableFuture is cancelled. Update
PortfolioBootstrapTrigger.requestBootstrap to record the request, advance
debounce state, and return true only for successful publication; propagate
non-success outcomes so failed or unknown sends do not suppress retries.
| } catch (Exception e) { | ||
| log.warn("[Bootstrap] Kafka trigger publish failed for portfolioId={}: {}", | ||
| debounceKey, e.getMessage()); | ||
| flowLogger.fail(span, e); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
git ls-files | rg 'TriggerCalculationPublisher.java|services/am-analysis/src/main/java/com/am/analysis/service/bootstrap/TriggerCalculationPublisher.java' || true
echo "== file excerpt =="
if [ -f services/am-analysis/src/main/java/com/am/analysis/service/bootstrap/TriggerCalculationPublisher.java ]; then
nl -ba services/am-analysis/src/main/java/com/am/analysis/service/bootstrap/TriggerCalculationPublisher.java | sed -n '1,140p'
fi
echo "== future.get calls/usages nearby =="
rg -n "Future\.get|get\(" services/am-analysis/src/main/java/com/am/analysis/service/bootstrap/TriggerCalculationPublisher.java || true
echo "== imports/exception checks =="
rg -n "InterruptedException|JsonProcessingException|Future" services/am-analysis/src/main/java/com/am/analysis/service/bootstrap/TriggerCalculationPublisher.java || trueRepository: AM-Portfolio/am-core-services
Length of output: 346
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file excerpt =="
awk '{ printf "%6d\t%s\n", NR, $0 }' services/am-analysis/src/main/java/com/sm/analysis/service/bootstrap/TriggerCalculationPublisher.java | sed -n '1,140p' || true
echo "== interrupt/future exception searches =="
grep -nE "InterruptedException|JsonProcessingException|Future\.get|\.get\(" services/am-analysis/src/main/java/com/am/analysis/service/bootstrap/TriggerCalculationPublisher.java || trueRepository: AM-Portfolio/am-core-services
Length of output: 590
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file excerpt =="
awk '{ printf "%6d\t%s\n", NR, $0 }' services/am-analysis/src/main/java/com/am/analysis/service/bootstrap/TriggerCalculationPublisher.java | sed -n '50,85p'
echo "== exception handles in file =="
grep -nE "catch \(|InterruptedException|Future\.get|\.get\(" services/am-analysis/src/main/java/com/am/analysis/service/bootstrap/TriggerCalculationPublisher.java || trueRepository: AM-Portfolio/am-core-services
Length of output: 1955
Preserve the interrupt status when publishing fails.
Future.get(...) can throw InterruptedException, but the generic catch (Exception) clears it before flowLogger.fail(span, e). Add a dedicated InterruptedException catch before the generic catch and restore the interrupt status there.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@services/am-analysis/src/main/java/com/am/analysis/service/bootstrap/TriggerCalculationPublisher.java`
around lines 69 - 72, Update the exception handling around the publishing
operation in TriggerCalculationPublisher to catch InterruptedException before
the generic Exception catch, restore the thread’s interrupted status in that
handler, and retain the existing logging and flowLogger.fail behavior. Leave
non-interruption failures handled by the generic catch.
| def _resolve_token( | ||
| *, | ||
| token_url: str, | ||
| client_id: str, | ||
| client_secret: str, | ||
| login_url: str, | ||
| user: str, | ||
| password: str, | ||
| identity_port: int, | ||
| ) -> str: | ||
| if client_id and client_secret: | ||
| try: | ||
| print("auth: Keycloak client_credentials", file=sys.stderr) | ||
| return _client_credentials(token_url, client_id, client_secret) | ||
| except Exception as exc: # noqa: BLE001 | ||
| print(f"client_credentials failed: {exc}", file=sys.stderr) | ||
| if not (user and password): | ||
| raise | ||
| print("auth: falling back to identity user login", file=sys.stderr) | ||
| if user and password: | ||
| return _login_with_fallback(login_url, user, password, identity_port) | ||
| raise RuntimeError( | ||
| "Set AM_MCP_CLIENT_ID+AM_MCP_CLIENT_SECRET (preferred) " | ||
| "or AM_AUTH_USER+AM_AUTH_PASS" | ||
| ) | ||
|
|
||
|
|
||
| def main() -> int: | ||
| local_port = int(os.environ.get("AM_MCP_PF_PORT", "18080")) | ||
| identity_port = int(os.environ.get("AM_IDENTITY_PF_PORT", "18001")) | ||
| sse_url = os.environ.get("AM_MCP_SSE_URL", f"http://127.0.0.1:{local_port}/sse") | ||
| sse_url = os.environ.get("AM_MCP_SSE_URL", "https://am-dev.asrax.in/mcp/sse") | ||
| token_url = os.environ.get( | ||
| "KEYCLOAK_TOKEN_URL", | ||
| "http://auth.munish.org/auth/realms/am-dev-realm/protocol/openid-connect/token", | ||
| ) | ||
| login_url = os.environ.get( | ||
| "AM_AUTH_LOGIN_URL", "https://am-dev.asrax.in/identity/auth/login" | ||
| ) | ||
| user = os.environ.get("AM_AUTH_USER", "") | ||
| password = os.environ.get("AM_AUTH_PASS", "") | ||
| if not user or not password: | ||
| print("AM_AUTH_USER and AM_AUTH_PASS are required", file=sys.stderr) | ||
| return 2 | ||
| client_id = os.environ.get("AM_MCP_CLIENT_ID", "").strip() | ||
| client_secret = os.environ.get("AM_MCP_CLIENT_SECRET", "").strip() | ||
| user = os.environ.get("AM_AUTH_USER", "").strip() | ||
| password = os.environ.get("AM_AUTH_PASS", "").strip() |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does auth.munish.org support HTTPS for its Keycloak token endpoint?
💡 Result:
There is no publicly accessible or authoritative information to confirm the existence, configuration, or security protocols (such as HTTPS support) for a service located at auth.munish.org [1][2][3][4][5]. The domain "munish.org" does not appear to host a public Keycloak instance, and research indicates that "Munish" is a common personal name associated with various independent professionals and developers, none of whom are linked to a public authentication infrastructure of this nature [1][2][3][4][5]. Keycloak itself, as a platform, natively supports and strongly recommends the use of HTTPS for all endpoints, including the token endpoint, to ensure secure communication [6][7][8]. If you are attempting to configure or troubleshoot an integration with a private Keycloak instance, you should verify its configuration directly through the server's administration console or its OpenID Connect discovery document (typically found at.../realms/{realm-name}/.well-known/openid-configuration) [9][10][11][12][13].
Citations:
- 1: https://linkedin.com/in/munish-salesforce
- 2: https://linkedin.com/in/munish-sharma-435a8a43
- 3: https://codewithmishu.in/
- 4: https://linkedin.com/in/munish-kumar-99303156
- 5: https://builtbymunish.com/
- 6: https://www.keycloak.org/
- 7: https://www.keycloak.org/docs/latest/server%5Fadmin/
- 8: https://www.keycloak.org/docs/26.2.5/server_admin/index.html
- 9: https://jisccti.github.io/misp-docker/configuration/keycloak/
- 10: Support: setting up MISP with SSO (keycloak) MISP/MISP#10276
- 11: https://github.com/MISP/misp-docker/blob/master/docs/keycloak-integration-guide.md
- 12: Support: Infinite Redirect loop during Oauth flow with OIDC plugin + KeyCloak MISP/MISP#10295
- 13: https://wlcg-soc-wg-doc.web.cern.ch/threat_intelligence/misp/oidc.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
git ls-files | rg 'services/am-mcp-server/scripts/mcp_remote_launch.py' || true
echo "== relevant file excerpt =="
if [ -f services/am-mcp-server/scripts/mcp_remote_launch.py ]; then
sed -n '1,220p' services/am-mcp-server/scripts/mcp_remote_launch.py | cat -n
fi
echo "== references to token_url/client_credentials =="
rg -n "KEYCLOAK_TOKEN_URL|_client_credentials|client_credentials|AM_MCP_CLIENT_SECRET|http://auth\\.munish\\.org|https://auth\\.munish\\.org" .Repository: AM-Portfolio/am-core-services
Length of output: 11018
🌐 Web query:
https://auth.munish.org/auth/realms/am-dev-realm/protocol/openid-connect/token HTTP status
💡 Result:
The URL you provided is a standard endpoint for an OpenID Connect (OIDC) token exchange in a Keycloak identity management environment [1][2]. The HTTP status code for this endpoint is not fixed; it depends entirely on the outcome of the request you send [1]. Based on the OpenID Connect specification and standard Keycloak behavior: 1. Successful Request: If the request contains valid credentials (such as an authorization code, client ID, and client secret) and meets all other requirements, the server will return an HTTP 200 OK status, providing the access, ID, and refresh tokens in the JSON response body [3][1]. 2. Client/Authentication Errors: If the request is missing parameters, uses invalid credentials, or fails validation (e.g., mismatched redirect URI, invalid grant type), the server will typically return an HTTP 400 Bad Request or HTTP 401 Unauthorized status [4][5]. 3. Method Not Allowed: If you send the request using a method other than POST (such as GET), the server will return an HTTP 405 Method Not Allowed status, as the token endpoint is strictly defined to accept only POST requests [6][7]. To diagnose the status you are seeing, you should inspect the response body, which generally includes an error code and a descriptive message (e.g., invalid_client, invalid_request) [5]. If you are experiencing unexpected results, ensure that your client configuration (client_id, client_secret) and your authentication method (e.g., client_secret_basic vs. client_secret_post) match what is configured in your Keycloak realm [6][7][1].
Citations:
- 1: regarding keycloak sso for single realm with different clients in multi services thomasdarimont/keycloak-docker-demo#10
- 2: https://forum.keycloak.org/t/keycloak-not-securing-angular-and-not-showing-login-page/12233
- 3: Authorization code flow with OIDCProviderTokenEndpointAuth client_secret_basic not working OpenIDC/mod_auth_openidc#1034
- 4: /oidc/callback/ throws exception on 403 from /oauth/token/ mozilla/mozilla-django-oidc#154
- 5: Public OIDC (token_endpoint_auth_method=none) sends unintended Basic Auth on token request and misses code_verifier openpubkey/opkssh#372
- 6: issue: Open WebUI: OIDC Token Exchange with Authentik Returns 405 Method Not Allowed open-webui/open-webui#16221
- 7: https://docs.optare.one/guides/oidc-configuration
Use HTTPS by default for the Keycloak token URL.
_client_credentials() posts client_id and client_secret to token_url, but the KEYCLOAK_TOKEN_URL default uses http://auth.munish.org/..., so the secret is sent unencrypted unless the caller overrides it. Change the default to https:// if that Keycloak endpoint supports it.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 167-167: Do not make http calls without encryption
Context: "http://auth.munish.org/auth/realms/am-dev-realm/protocol/openid-connect/token"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/am-mcp-server/scripts/mcp_remote_launch.py` around lines 135 - 176,
Update the KEYCLOAK_TOKEN_URL default in main to use the HTTPS scheme for the
existing Keycloak token endpoint, while preserving the environment-variable
override and URL path unchanged.
Source: Linters/SAST tools
| public static String resolve(String userId, AmMcpProperties props) { | ||
| if (userId != null && !userId.isBlank()) { | ||
| return userId; | ||
| } | ||
| String fromJwt = UserContext.getUserId(); | ||
| if (fromJwt != null && !fromJwt.isBlank()) { | ||
| return fromJwt; | ||
| } | ||
| return props.getDefaults().getUserId(); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map MCP tool and security configuration before inspecting the authorization path.
ast-grep outline services/am-mcp-server/src/main/java --items all --type class,method
# Inspect explicit user-ID inputs, resolver call sites, and owner-scoped repository access.
rg -n -C 5 '`@ToolParam`\(description = "User ID|UserIdResolver\.resolve|findByOwnerIdAndType' \
services/am-mcp-server/src/main/javaRepository: AM-Portfolio/am-core-services
Length of output: 18148
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the resolver, tool interceptor, and security config relevant to MCP authorization.
sed -n '1,220p' services/am-mcp-server/src/main/java/com/am/mcp/util/UserIdResolver.java
printf '\n--- ToolExecutionInterceptor ---\n'
sed -n '1,220p' services/am-mcp-server/src/main/java/com/am/mcp/aspect/ToolExecutionInterceptor.java
printf '\n--- Security/Auth configs ---\n'
rg -n -C 4 'SecurityFilterChain|hasRole|hasAuthority|PreAuthorize|Secured|AuthorizeHttpRequests|JWT|UserContext|`@Tool`|`@Mcp`' services/am-mcp-server/src/main/javaRepository: AM-Portfolio/am-core-services
Length of output: 50385
Use the authenticated user ID before caller-supplied values.
Callers can pass @ToolParam description = "User ID." into analysis and trade tools, and UserIdResolver.resolve returns that value before extracting the JWT subject. Since these tools pass the resolved ID into owner-scoped queries, normalize to UserContext.getUserId() for normal caller paths and reserve overrides for an authorized trusted path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/am-mcp-server/src/main/java/com/am/mcp/util/UserIdResolver.java`
around lines 14 - 22, Update UserIdResolver.resolve to prioritize and return the
authenticated ID from UserContext.getUserId() before considering the
caller-supplied userId; only permit caller overrides through an explicitly
authorized trusted path, while retaining the configured default when no
authenticated ID is available.
…JWT outbound Use x-access-token plus GHCR/PACKAGES token fallbacks for package reads; clarify TradeAuthTokenSupplier and TradeSdkConfig for per-request user JWT on outbound trade calls. Co-authored-by: Cursor <cursoragent@cursor.com>
Align with reactor TradeAuthTokenSupplier so CI can compile against the in-repo client lib via central-build-publish -am. Co-authored-by: Cursor <cursoragent@cursor.com>
Publish TradeAuthTokenSupplier to GitHub Packages before mcp-server can resolve the new client API remotely. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Match published GitHub Packages; ${project.version} resolved to
1.1.5-SNAPSHOT which is not published and broke SDK publish.
Co-authored-by: Cursor <cursoragent@cursor.com>
….1.5-SNAPSHOT Co-authored-by: Cursor <cursoragent@cursor.com>
Replace phantom 1.1.4-SNAPSHOT pins with latest published artifacts so MCP CI can resolve after am-trade-client-lib 1.1.5-SNAPSHOT. Co-authored-by: Cursor <cursoragent@cursor.com>
GitHub Actions
Waiting
GitHub Actions
Waiting
GitHub Actions
Waiting
Updated the version of am.api-core-lib to 1.0.0-SNAPSHOT to align with dependency requirements and ensure compatibility with other library versions.
GitHub Actions
Waiting
GitHub Actions
Waiting
GitHub Actions
Inactive
2 participants
Summary by CodeRabbit
New Features
Bug Fixes