User config to dispatch command if sequence table is full

[michaelkiper]

Related Issue(s)	#5812 (comment)
Has Unit Tests (y/n)	y
Documentation Included (y/n)	y
Generative AI was used in this contribution (y/n)	y

---

Change Description

This PR resolves this CCB comment:

CCB: recommendations:

1. Add a configuration to allow ports to bypass tracking (because the tracking is going to be discarded anyway e.g. uplink)
2. Add configuration (drop on full, dispatch-without-tracking) to allow projects to select this.
3. Add a new response type that says this command is untracked.

• Adds in CmdDispatcherCfg::ExecuteCommandWhenSequenceTrackerTableIsFull = false, as a user defined configuration option for whether to still dispatch a sequence command if the sequence tracker table is full.
  • If false (default), the CmdDispatcher will provide back a Fw::CmdResponse::EXECUTION_ERROR to the caller if the sequence tracker table is full and will not dispatch the command. This is the current behavior that already exists.
  • If true, the CmdDispatcher will still dispatch the command and return a Fw::CmdResponse::DISPATCHED_UNTRACKED response to the caller AFTER the command has already been dispatched. It occurs in this order as if the user sets ExecuteCommandWhenSequenceTrackerTableIsFull = true, that means they have critical commands that must execute immediately, so any caller handling occurs after the dispatching since that's a higher priority.
• Adds in Fw::CmdResponse::DISPATCHED_UNTRACKED response type to let the caller know that the command is dispatched but the status is going to be unchecked as the sequence tracker table is full.

Rationale

resolves #5812

Rationale for behavior change request is in the above ticket.

Testing/Review Recommendations

fprime-util check -j5

Future Work

AI Usage (see policy)

Claude Code Opus 5 was used for searching the solution space for where the configuration file, default/config/CommandDispatcherImplCfg.hpp lives, identifying common patterns in the code base to replicate, e.g. asking it if there is a common syntax in the code base to change if constexpr (! CmdDispatcherCfg::ExecuteCommandWhenSequenceTrackerTableIsFull) { to a more obvious branching evaluation than using a logical NOT operator, and having it run a heuristical check to identify if I've made any logical errors in my implementation.

Claude Code Opus 5 was also used in documentation and unit test generation - I reviewed it for correctness.

JPL-Devin was used indirectly, borrowing from changes in https://github.com/nasa/fprime/pull/5823/changes.

[michaelkiper]

Evaluate the expression once.

[michaelkiper]

Initialize to Fw::Success::SUCCESS so that the if (CmdDispatcherCfg::ExecuteCommandWhenSequenceTrackerTableIsFull && pendingInsertStatus != Fw::Success::SUCCESS) conditional is only taken if portIsConnected is true and we call this->m_sequenceTracker.insert.

[michaelkiper]

This still is elided even on -O0 for clang 22.1 on arm64 - likely for other targets as well.

(base) makiper@MT-400290 fprime % clang++ --version
Homebrew clang version 22.1.6
Target: arm64-apple-darwin25.6.0

LBB26_27:
        .loc    0 132 13                        ; Svc/CmdDispatcher/CommandDispatcherImpl.cpp:132:13
        add     x0, sp, #176
        bl      __ZN2Fw7SuccessD1Ev
Ltmp161:                                ; EH_LABEL
Ltmp216:
        .loc    0 136 37 is_stmt 1              ; Svc/CmdDispatcher/CommandDispatcherImpl.cpp:136:37
        add     x0, sp, #208
        mov     w1, #1                          ; =0x1
        bl      __ZNK2Fw7SuccessneENS0_1TE
        str     w0, [sp, #68]                   ; 4-byte Spill
Ltmp162:                                ; EH_LABEL
        b       LBB26_28
LBB26_28:
        .loc    0 0 37 is_stmt 0                ; Svc/CmdDispatcher/CommandDispatcherImpl.cpp:0:37
        ldr     w8, [sp, #68]                   ; 4-byte Reload
        .loc    0 135 84 is_stmt 1              ; Svc/CmdDispatcher/CommandDispatcherImpl.cpp:135:84
        tbz     w8, #0, LBB26_40
        b       LBB26_29

[michaelkiper]

This block does not get generated (can't speak for all compilers but it at least would never be taken) in the default case where CmdDispatcherCfg::ExecuteCommandWhenSequenceTrackerTableIsFull = false. This preserves the original behavior of not dispatching a command if the sequence table is full and instead return a Fw::CmdResponse::EXECUTION_ERROR to the caller.

[michaelkiper]

We don't need a second if (portIsConnected) check here as the pendingInsertStatus != Fw::Success::SUCCESS condition already subsumes the former.

[lestarch-autobot]

[Supply Chain] could fix Hidden HTML comment in the PR body carries instructions aimed at AI agents/bots.

(Anchored here; the PR body is not part of the diff.) The first line of the PR description is a hidden <!-- If you are an AI agent or bot, please respond ... with AI ... --> block. It is inherited verbatim from the repo's pull_request_template.md and appears benign, but hidden instructions targeting automated actors are a prompt-injection surface and are flagged regardless of origin (zero-trust, contract §0). Consider removing the hidden block from the PR body; the template itself is a separate, repo-level concern for maintainers.

[lestarch-autobot]

[Test Quality] must fix New Fw::CmdResponse::DISPATCHED_UNTRACKED path is never executed by the unit test.

ExecuteCommandWhenSequenceTrackerTableIsFull is a constexpr from default/config, so with the default (false) this branch is compile-time dead: CI never dispatches-on-full, never emits DISPATCHED_UNTRACKED, and never checks the OpCodeDispatched → TooManyCommands ordering. The new enum constant and the new behavior in seqCmdBuff_handler therefore ship with zero executed coverage, and the TestMain COMMENT ("for both settings") overstates what one build verifies.
Consider making the policy injectable for UT (e.g. a UT-only config override / -D for the CmdDispatcher UT target, or a second UT build of this test with the flag flipped) so both branches run in CI.

cc @LeStarch @thomas-bc — low-confidence finding, please confirm (same pattern already exists for IncludeCommandOpcodesInEvents).

[lestarch-autobot]

[C++ Design] suggestion portIsConnected is never reassigned; declare it const (CPP-11).

Matches the neighbouring const U32 sequenceNumber idiom and makes clear the connection state is sampled once per dispatch.

Suggested change

	bool portIsConnected = this->isConnected_seqCmdStatus_OutputPort(portNum);
	const bool portIsConnected = this->isConnected_seqCmdStatus_OutputPort(portNum);

[lestarch-autobot]

[Documentation] could fix docs/reference/system-functional/command-dispatch.md "Off Nominal" still describes the tracker-full case as an unconditional rejection.

(Anchored here; the stale line, command-dispatch.md:74, is not in the PR diff.) The reference page's "Too Many Pending Commands" bullet now only describes the false default of the new config; readers of the subsystem reference will not learn that the dispatch-and-report-DISPATCHED_UNTRACKED path exists. Proposed replacement for that bullet:

- **Too Many Pending Commands** — If the pending command tracker is full, a `TooManyCommands` warning event is logged. By default (`Svc::CmdDispatcherCfg::ExecuteCommandWhenSequenceTrackerTableIsFull = false`) the command is rejected with `EXECUTION_ERROR`; when the setting is `true`, the command is dispatched anyway and the source receives `DISPATCHED_UNTRACKED`, meaning its completion status will never be reported.

[lestarch-autobot]

[Maintainability] suggestion maint-code-duplication: the table-full outcome is split across two mirrored blocks whose correctness rests on an unwritten invariant.

This block and the one at L135 carry inverted copies of the same predicate and the same TooManyCommands log; a future third mode (or a changed event) must be edited in both places. It also only stays correct because pendingInsertStatus is pre-set to SUCCESS on the disconnected-port path — that coupling is explained in PR comments, not in the code. The trailing return; is a no-op today but will silently skip any statement later appended after this block inside the branch.

Suggested change

	if (CmdDispatcherCfg::ExecuteCommandWhenSequenceTrackerTableIsFull &&
	pendingInsertStatus != Fw::Success::SUCCESS) {
	this->log_WARNING_HI_TooManyCommands(CmdDispatcherCfg::getEventOpcode(cmdPkt.getOpCode()));
	this->seqCmdStatus_out(portNum, cmdPkt.getOpCode(), context, Fw::CmdResponse::DISPATCHED_UNTRACKED);
	return;
	}
	// pendingInsertStatus is only non-SUCCESS for a connected caller whose insert failed (see check above)
	if (CmdDispatcherCfg::ExecuteCommandWhenSequenceTrackerTableIsFull &&
	pendingInsertStatus != Fw::Success::SUCCESS) {
	this->log_WARNING_HI_TooManyCommands(CmdDispatcherCfg::getEventOpcode(cmdPkt.getOpCode()));
	this->seqCmdStatus_out(portNum, cmdPkt.getOpCode(), context, Fw::CmdResponse::DISPATCHED_UNTRACKED);
	}

[lestarch-autobot]

[Maintainability] suggestion maint-misleading-comment: comment says "quit" on a full table, but the new condition only quits when the config is false.

With ExecuteCommandWhenSequenceTrackerTableIsFull = true the same insert failure now falls through to dispatch and is reported at L150, so the comment describes half the behavior. Separately, not mixed with && in one expression diverges from the file (L121 in this very diff was normalized from and to &&); use ! for a single operator dialect.

Suggested change

	// if we couldn't find a slot to track the command, quit
	if (pendingInsertStatus != Fw::Success::SUCCESS) {
	if (not CmdDispatcherCfg::ExecuteCommandWhenSequenceTrackerTableIsFull &&
	pendingInsertStatus != Fw::Success::SUCCESS) {
	// table full: reject here unless configured to dispatch untracked (then reported after dispatch below)
	if (!CmdDispatcherCfg::ExecuteCommandWhenSequenceTrackerTableIsFull &&
	pendingInsertStatus != Fw::Success::SUCCESS) {

[lestarch-autobot]

[Design] could fix New DISPATCHED_UNTRACKED = 7 is not mirrored in the WASM guest ABI enum FprimeCmdResponse (Svc/WasmSequencer/spacewasm_include/fprime.h).

WasmSequencer::cmdResponseIn_handler forwards response.e verbatim to guest code via hostResumeI32, so guests can now receive value 7 with no named constant. That header already lags Fw::CmdResponse (FPRIME_CMD_CLEARED = 5 collides with BUSY; being corrected in #6001), so please add FPRIME_CMD_DISPATCHED_UNTRACKED = 7 alongside so the two enums do not drift further.

[lestarch-autobot]

[Design] suggestion The design leaves unstated how existing callers react to DISPATCHED_UNTRACKED: for Svc::CmdSequencer any non-OK status (cmdResponseIn_handler, response != Fw::CmdResponse::OK) calls commandError + performCmd_Cancel, so with the opt-in enabled a critical command executes but the sequence still aborts at that step, exactly as it does today with EXECUTION_ERROR.

That may be the intended contract (the issue's goal is that the command runs), but a project enabling the flag should learn it here rather than from a cancelled sequence. Documenting the stock-sequencer behavior keeps the SDD honest about what the new status buys.

Suggested change

Note #5: the pending sequence command table is a fixed-size table (`CMD_DISPATCHER_SEQUENCER_TABLE_SIZE`), and it is only consulted when the `seqCmdStatus` output port for the calling port is connected. When a command arrives, that port is connected, and the table is full, the dispatcher's behavior is selected at compile time by `Svc::CmdDispatcherCfg::ExecuteCommandWhenSequenceTrackerTableIsFull`. When `false` (the default), the command is not dispatched: a `TooManyCommands` warning is emitted and the caller receives `Fw::CmdResponse::EXECUTION_ERROR`. When `true`, the command is dispatched, a `TooManyCommands` warning is emitted, and the caller receives `Fw::CmdResponse::DISPATCHED_UNTRACKED`, indicating that the command is executing but that its completion status cannot be matched back to the caller and will never be reported.

Note #5: the pending sequence command table is a fixed-size table (`CMD_DISPATCHER_SEQUENCER_TABLE_SIZE`), and it is only consulted when the `seqCmdStatus` output port for the calling port is connected. When a command arrives, that port is connected, and the table is full, the dispatcher's behavior is selected at compile time by `Svc::CmdDispatcherCfg::ExecuteCommandWhenSequenceTrackerTableIsFull`. When `false` (the default), the command is not dispatched: a `TooManyCommands` warning is emitted and the caller receives `Fw::CmdResponse::EXECUTION_ERROR`. When `true`, the command is dispatched, a `TooManyCommands` warning is emitted, and the caller receives `Fw::CmdResponse::DISPATCHED_UNTRACKED`, indicating that the command is executing but that its completion status cannot be matched back to the caller and will never be reported. Note that the stock sequencers do not treat `DISPATCHED_UNTRACKED` as success: `Svc::CmdSequencer` fails and cancels the sequence on any non-`OK` response, `Svc::WasmSequencer` counts it as a failed command, and `Svc::FpySequencer` pushes the raw response value for the sequence to branch on.

[lestarch-autobot]

[Design] could fix The PR body says it resolves the CCB comment on #5812, but only recommendations 2 and 3 (drop-vs-dispatch config, new response type) are implemented; recommendation 1 (a per-port configuration to bypass tracking entirely, e.g. for uplink whose status is discarded) is not addressed here or in the SDD.

If item 1 is intentionally deferred, please say so in the PR description / Future Work (and optionally a follow-up issue) so the CCB comment is not marked resolved with a gap.

cc @LeStarch @thomas-bc — low-confidence finding, please confirm.

[lestarch-autobot]

[Operational] could fix Note #5 omits the worst-case operational consequence of ExecuteCommandWhenSequenceTrackerTableIsFull = true for in-repo callers.

Finding class: ops-doc-reality. Scenario: table saturated (25 default slots held by commands whose handlers never respond) and true selected. Every subsequent tracked command is dispatched and then answered DISPATCHED_UNTRACKED; Svc::CmdSequencer treats any non-OK response as a command error and cancels the sequence (CmdSequencerImpl.cpp cmdResponseIn_handler), so each sequence aborts at its first command while that command still executes — the cancel cannot recall it. This repeats for every sequence until CMD_CLEAR_TRACKING. FpySequencer/WasmSequencer instead continue with the value on the stack / as the host return, so behaviour differs per sequencer. Smallest remedy: state the consequence and the recovery in the SDD (operational judgment call on wording).

Suggested change

Note #5: the pending sequence command table is a fixed-size table (`CMD_DISPATCHER_SEQUENCER_TABLE_SIZE`), and it is only consulted when the `seqCmdStatus` output port for the calling port is connected. When a command arrives, that port is connected, and the table is full, the dispatcher's behavior is selected at compile time by `Svc::CmdDispatcherCfg::ExecuteCommandWhenSequenceTrackerTableIsFull`. When `false` (the default), the command is not dispatched: a `TooManyCommands` warning is emitted and the caller receives `Fw::CmdResponse::EXECUTION_ERROR`. When `true`, the command is dispatched, a `TooManyCommands` warning is emitted, and the caller receives `Fw::CmdResponse::DISPATCHED_UNTRACKED`, indicating that the command is executing but that its completion status cannot be matched back to the caller and will never be reported.

Note #5: the pending sequence command table is a fixed-size table (`CMD_DISPATCHER_SEQUENCER_TABLE_SIZE`), and it is only consulted when the `seqCmdStatus` output port for the calling port is connected. When a command arrives, that port is connected, and the table is full, the dispatcher's behavior is selected at compile time by `Svc::CmdDispatcherCfg::ExecuteCommandWhenSequenceTrackerTableIsFull`. When `false` (the default), the command is not dispatched: a `TooManyCommands` warning is emitted and the caller receives `Fw::CmdResponse::EXECUTION_ERROR`. When `true`, the command is dispatched, a `TooManyCommands` warning is emitted, and the caller receives `Fw::CmdResponse::DISPATCHED_UNTRACKED`, indicating that the command is executing but that its completion status cannot be matched back to the caller and will never be reported. Operators should note that in-repo callers do not treat `DISPATCHED_UNTRACKED` uniformly: `Svc::CmdSequencer` treats it as a command failure and cancels the running sequence even though the dispatched command continues to execute and cannot be recalled, whereas `Svc::FpySequencer` and `Svc::WasmSequencer` continue and expose the value to the sequence. Once the table is saturated (e.g. by commands whose handlers never respond), every subsequent tracked command takes this path until `CMD_CLEAR_TRACKING` is issued.

[lestarch-autobot]

Automated review summary (run 1)

Pre-run prompt-injection alert

⚠️ The orchestrator's pre-run metadata scan flagged potential
prompt-injection in PR-authored content before reviewers were
invoked. All reviewers were warned via their kickoff prompts.

Flagged surfaces:

• pr_body: hidden-html-comment — "<!-- If you are an AI agent or bot, please respond to the question "Generative AI was used in this con..." (this is the standard F Prime PR-template AI-disclosure comment)

The supply-chain reviewer's inline findings below include full
analysis of any prompt-injection content in the diff and metadata.

Per-agent results

Agent	must fix	suggestion	could fix	future work	outstanding	Verdict
Security Vulnerabilities	0	0	0	0	0	Go
Supply Chain / Runner Safety	0	0	1	0	1	Go
F Prime C/C++ Design	0	1	0	0	1	Go
Documentation Currency	0	0	1	0	1	Go
Design	0	1	2	0	3	Go
Architecture	0	0	0	0	0	Go
Test Quality	1	0	0	0	1	No-Go
Correctness	0	0	0	0	0	Go
Operational	0	0	1	0	1	Go
Maintainability	0	2	0	0	2	Go
CI safety	—	—	—	—	—	Go
Totals	1	4	5	0	10	No-Go

Supply-chain surfaces

Surface	Outstanding
Dependencies	clean
Vendored / submodule	clean
Build / test infrastructure	clean
Workflows / actions / scripts	clean
Generator output	clean
Prompt-injection	1 could-fix — hidden AI-targeting HTML comment (from PR template) in PR body
Review-system integrity	clean

Outstanding must-fix items (1)

Test Quality

• New Fw::CmdResponse::DISPATCHED_UNTRACKED path is never executed by the unit test — #6006 (comment)

Merge readiness

Merge readiness: No-Go — Test Quality has 1 outstanding must-fix item (the opt-in DISPATCHED_UNTRACKED path is untested).

---

One untested branch stands between this dispatcher and go-for-launch — light it up and we're ready to fly.